Merge changes I7901b017,I0f38f678
* changes:
Remove RenderEngine::flush from latchBuffer()
Flip native fence sync check for tex binding.
diff --git a/services/surfaceflinger/BufferLayer.cpp b/services/surfaceflinger/BufferLayer.cpp
index 1a73ff0..cafe26b 100644
--- a/services/surfaceflinger/BufferLayer.cpp
+++ b/services/surfaceflinger/BufferLayer.cpp
@@ -320,7 +320,8 @@
return true;
}
-Region BufferLayer::latchBuffer(bool& recomputeVisibleRegions, nsecs_t latchTime) {
+Region BufferLayer::latchBuffer(bool& recomputeVisibleRegions, nsecs_t latchTime,
+ const sp<Fence>& releaseFence) {
ATRACE_CALL();
std::optional<Region> sidebandStreamDirtyRegion = latchSidebandStream(recomputeVisibleRegions);
@@ -361,7 +362,7 @@
return dirtyRegion;
}
- status_t err = updateTexImage(recomputeVisibleRegions, latchTime);
+ status_t err = updateTexImage(recomputeVisibleRegions, latchTime, releaseFence);
if (err != NO_ERROR) {
return dirtyRegion;
}
diff --git a/services/surfaceflinger/BufferLayer.h b/services/surfaceflinger/BufferLayer.h
index 92bf132..a294793 100644
--- a/services/surfaceflinger/BufferLayer.h
+++ b/services/surfaceflinger/BufferLayer.h
@@ -91,7 +91,11 @@
// the visible regions need to be recomputed (this is a fairly heavy
// operation, so this should be set only if needed). Typically this is used
// to figure out if the content or size of a surface has changed.
- Region latchBuffer(bool& recomputeVisibleRegions, nsecs_t latchTime) override;
+ // If there was a GL composition step rendering the previous frame, then
+ // releaseFence will be populated with a native fence that fires when
+ // composition has completed.
+ Region latchBuffer(bool& recomputeVisibleRegions, nsecs_t latchTime,
+ const sp<Fence>& releaseFence) override;
bool isBufferLatched() const override { return mRefreshPending; }
@@ -135,7 +139,8 @@
virtual void setFilteringEnabled(bool enabled) = 0;
virtual status_t bindTextureImage() const = 0;
- virtual status_t updateTexImage(bool& recomputeVisibleRegions, nsecs_t latchTime) = 0;
+ virtual status_t updateTexImage(bool& recomputeVisibleRegions, nsecs_t latchTime,
+ const sp<Fence>& flushFence) = 0;
virtual status_t updateActiveBuffer() = 0;
virtual status_t updateFrameNumber(nsecs_t latchTime) = 0;
diff --git a/services/surfaceflinger/BufferLayerConsumer.cpp b/services/surfaceflinger/BufferLayerConsumer.cpp
index fa181ad..f499f06 100644
--- a/services/surfaceflinger/BufferLayerConsumer.cpp
+++ b/services/surfaceflinger/BufferLayerConsumer.cpp
@@ -147,7 +147,8 @@
status_t BufferLayerConsumer::updateTexImage(BufferRejecter* rejecter, const DispSync& dispSync,
bool* autoRefresh, bool* queuedBuffer,
- uint64_t maxFrameNumber) {
+ uint64_t maxFrameNumber,
+ const sp<Fence>& releaseFence) {
ATRACE_CALL();
BLC_LOGV("updateTexImage");
Mutex::Autolock lock(mMutex);
@@ -198,12 +199,12 @@
}
// Release the previous buffer.
- err = updateAndReleaseLocked(item, &mPendingRelease);
+ err = updateAndReleaseLocked(item, &mPendingRelease, releaseFence);
if (err != NO_ERROR) {
return err;
}
- if (mRE.useNativeFenceSync()) {
+ if (!mRE.useNativeFenceSync()) {
// Bind the new buffer to the GL texture.
//
// Older devices require the "implicit" synchronization provided
@@ -278,14 +279,15 @@
}
status_t BufferLayerConsumer::updateAndReleaseLocked(const BufferItem& item,
- PendingRelease* pendingRelease) {
+ PendingRelease* pendingRelease,
+ const sp<Fence>& releaseFence) {
status_t err = NO_ERROR;
int slot = item.mSlot;
// Do whatever sync ops we need to do before releasing the old slot.
if (slot != mCurrentTexture) {
- err = syncForReleaseLocked();
+ err = syncForReleaseLocked(releaseFence);
if (err != NO_ERROR) {
// Release the buffer we just acquired. It's not safe to
// release the old buffer, so instead we just drop the new frame.
@@ -367,19 +369,19 @@
return doFenceWaitLocked();
}
-status_t BufferLayerConsumer::syncForReleaseLocked() {
+status_t BufferLayerConsumer::syncForReleaseLocked(const sp<Fence>& releaseFence) {
BLC_LOGV("syncForReleaseLocked");
if (mCurrentTexture != BufferQueue::INVALID_BUFFER_SLOT) {
- if (mRE.useNativeFenceSync()) {
- base::unique_fd fenceFd = mRE.flush();
- if (fenceFd == -1) {
+ if (mRE.useNativeFenceSync() && releaseFence != Fence::NO_FENCE) {
+ // TODO(alecmouri): fail further upstream if the fence is invalid
+ if (!releaseFence->isValid()) {
BLC_LOGE("syncForReleaseLocked: failed to flush RenderEngine");
return UNKNOWN_ERROR;
}
- sp<Fence> fence(new Fence(std::move(fenceFd)));
- status_t err = addReleaseFenceLocked(mCurrentTexture,
- mCurrentTextureImage->graphicBuffer(), fence);
+ status_t err =
+ addReleaseFenceLocked(mCurrentTexture, mCurrentTextureImage->graphicBuffer(),
+ releaseFence);
if (err != OK) {
BLC_LOGE("syncForReleaseLocked: error adding release fence: "
"%s (%d)",
diff --git a/services/surfaceflinger/BufferLayerConsumer.h b/services/surfaceflinger/BufferLayerConsumer.h
index 257a4e5..dc00487 100644
--- a/services/surfaceflinger/BufferLayerConsumer.h
+++ b/services/surfaceflinger/BufferLayerConsumer.h
@@ -94,7 +94,8 @@
// used to reject the newly acquired buffer. It also does not bind the
// RenderEngine texture until bindTextureImage is called.
status_t updateTexImage(BufferRejecter* rejecter, const DispSync& dispSync, bool* autoRefresh,
- bool* queuedBuffer, uint64_t maxFrameNumber);
+ bool* queuedBuffer, uint64_t maxFrameNumber,
+ const sp<Fence>& releaseFence);
// See BufferLayerConsumer::bindTextureImageLocked().
status_t bindTextureImage();
@@ -208,7 +209,8 @@
// completion of the method will instead be returned to the caller, so that
// it may call releaseBufferLocked itself later.
status_t updateAndReleaseLocked(const BufferItem& item,
- PendingRelease* pendingRelease = nullptr);
+ PendingRelease* pendingRelease = nullptr,
+ const sp<Fence>& releaseFence = Fence::NO_FENCE);
// Binds mTexName and the current buffer to TEXTURE_EXTERNAL target. Uses
// mCurrentTexture if it's set, mCurrentTextureImage if not. If the
@@ -282,7 +284,7 @@
// current slot from RenderEngine. If needed it will set the current
// slot's fence to guard against a producer accessing the buffer before
// the outstanding accesses have completed.
- status_t syncForReleaseLocked();
+ status_t syncForReleaseLocked(const sp<Fence>& releaseFence);
// The default consumer usage flags that BufferLayerConsumer always sets on its
// BufferQueue instance; these will be OR:d with any additional flags passed
diff --git a/services/surfaceflinger/BufferQueueLayer.cpp b/services/surfaceflinger/BufferQueueLayer.cpp
index 17706b5..6822298 100644
--- a/services/surfaceflinger/BufferQueueLayer.cpp
+++ b/services/surfaceflinger/BufferQueueLayer.cpp
@@ -221,7 +221,8 @@
return mConsumer->bindTextureImage();
}
-status_t BufferQueueLayer::updateTexImage(bool& recomputeVisibleRegions, nsecs_t latchTime) {
+status_t BufferQueueLayer::updateTexImage(bool& recomputeVisibleRegions, nsecs_t latchTime,
+ const sp<Fence>& releaseFence) {
// This boolean is used to make sure that SurfaceFlinger's shadow copy
// of the buffer queue isn't modified when the buffer queue is returning
// BufferItem's that weren't actually queued. This can happen in shared
@@ -232,7 +233,7 @@
getTransformToDisplayInverse(), mFreezeGeometryUpdates);
status_t updateResult =
mConsumer->updateTexImage(&r, *mFlinger->mPrimaryDispSync, &mAutoRefresh, &queuedBuffer,
- mLastFrameNumberReceived);
+ mLastFrameNumberReceived, releaseFence);
if (updateResult == BufferQueue::PRESENT_LATER) {
// Producer doesn't want buffer to be displayed yet. Signal a
// layer update so we check again at the next opportunity.
diff --git a/services/surfaceflinger/BufferQueueLayer.h b/services/surfaceflinger/BufferQueueLayer.h
index 051b15d..dcf39ee 100644
--- a/services/surfaceflinger/BufferQueueLayer.h
+++ b/services/surfaceflinger/BufferQueueLayer.h
@@ -90,7 +90,8 @@
void setFilteringEnabled(bool enabled) override;
status_t bindTextureImage() const override;
- status_t updateTexImage(bool& recomputeVisibleRegions, nsecs_t latchTime) override;
+ status_t updateTexImage(bool& recomputeVisibleRegions, nsecs_t latchTime,
+ const sp<Fence>& releaseFence) override;
status_t updateActiveBuffer() override;
status_t updateFrameNumber(nsecs_t latchTime) override;
diff --git a/services/surfaceflinger/BufferStateLayer.cpp b/services/surfaceflinger/BufferStateLayer.cpp
index 9c10868..2ecf71c 100644
--- a/services/surfaceflinger/BufferStateLayer.cpp
+++ b/services/surfaceflinger/BufferStateLayer.cpp
@@ -377,7 +377,8 @@
return NO_ERROR;
}
-status_t BufferStateLayer::updateTexImage(bool& /*recomputeVisibleRegions*/, nsecs_t latchTime) {
+status_t BufferStateLayer::updateTexImage(bool& /*recomputeVisibleRegions*/, nsecs_t latchTime,
+ const sp<Fence>& releaseFence) {
const State& s(getDrawingState());
if (!s.buffer) {
@@ -419,16 +420,13 @@
}
// Handle sync fences
- if (SyncFeatures::getInstance().useNativeFenceSync()) {
- base::unique_fd fenceFd = engine.flush();
- if (fenceFd == -1) {
- ALOGE("failed to flush RenderEngine");
+ if (SyncFeatures::getInstance().useNativeFenceSync() && releaseFence != Fence::NO_FENCE) {
+ // TODO(alecmouri): Fail somewhere upstream if the fence is invalid.
+ if (!releaseFence->isValid()) {
mTimeStats.clearLayerRecord(getName().c_str());
return UNKNOWN_ERROR;
}
- sp<Fence> fence(new Fence(std::move(fenceFd)));
-
// Check status of fences first because merging is expensive.
// Merging an invalid fence with any other fence results in an
// invalid fence.
@@ -439,10 +437,10 @@
return BAD_VALUE;
}
- auto incomingStatus = fence->getStatus();
+ auto incomingStatus = releaseFence->getStatus();
if (incomingStatus == Fence::Status::Invalid) {
ALOGE("New fence has invalid state");
- mDrawingState.acquireFence = fence;
+ mDrawingState.acquireFence = releaseFence;
mTimeStats.clearLayerRecord(getName().c_str());
return BAD_VALUE;
}
@@ -452,12 +450,13 @@
if (currentStatus == incomingStatus) {
char fenceName[32] = {};
snprintf(fenceName, 32, "%.28s:%d", mName.string(), mFrameNumber);
- sp<Fence> mergedFence = Fence::merge(fenceName, mDrawingState.acquireFence, fence);
+ sp<Fence> mergedFence =
+ Fence::merge(fenceName, mDrawingState.acquireFence, releaseFence);
if (!mergedFence.get()) {
ALOGE("failed to merge release fences");
// synchronization is broken, the best we can do is hope fences
// signal in order so the new fence will act like a union
- mDrawingState.acquireFence = fence;
+ mDrawingState.acquireFence = releaseFence;
mTimeStats.clearLayerRecord(getName().c_str());
return BAD_VALUE;
}
@@ -470,7 +469,7 @@
// by this point, they will have both signaled and only the timestamp
// will be slightly off; any dependencies after this point will
// already have been met.
- mDrawingState.acquireFence = fence;
+ mDrawingState.acquireFence = releaseFence;
}
} else {
// Bind the new buffer to the GL texture.
diff --git a/services/surfaceflinger/BufferStateLayer.h b/services/surfaceflinger/BufferStateLayer.h
index b662b96..db11110 100644
--- a/services/surfaceflinger/BufferStateLayer.h
+++ b/services/surfaceflinger/BufferStateLayer.h
@@ -115,7 +115,8 @@
void setFilteringEnabled(bool enabled) override;
status_t bindTextureImage() const override;
- status_t updateTexImage(bool& recomputeVisibleRegions, nsecs_t latchTime) override;
+ status_t updateTexImage(bool& recomputeVisibleRegions, nsecs_t latchTime,
+ const sp<Fence>& releaseFence) override;
status_t updateActiveBuffer() override;
status_t updateFrameNumber(nsecs_t latchTime) override;
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index 4890fa6..3eb8327 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -461,7 +461,8 @@
* operation, so this should be set only if needed). Typically this is used
* to figure out if the content or size of a surface has changed.
*/
- virtual Region latchBuffer(bool& /*recomputeVisibleRegions*/, nsecs_t /*latchTime*/) {
+ virtual Region latchBuffer(bool& /*recomputeVisibleRegions*/, nsecs_t /*latchTime*/,
+ const sp<Fence>& /*releaseFence*/) {
return {};
}
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index c37b3b1..37b32ed 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -1528,6 +1528,18 @@
getBE().mHwc->hasClientComposition(display->getId());
}
+ // Setup RenderEngine sync fences if native sync is supported.
+ if (getBE().mRenderEngine->useNativeFenceSync()) {
+ if (mHadClientComposition) {
+ base::unique_fd flushFence(getRenderEngine().flush());
+ ALOGE_IF(flushFence < 0, "Failed to flush RenderEngine!");
+ getBE().flushFence = new Fence(std::move(flushFence));
+ } else {
+ // Cleanup for hygiene.
+ getBE().flushFence = Fence::NO_FENCE;
+ }
+ }
+
mVsyncModulator.onRefreshed(mHadClientComposition);
getBE().mEndOfFrameCompositionInfo = std::move(getBE().mCompositionInfo);
@@ -3038,7 +3050,7 @@
});
for (auto& layer : mLayersWithQueuedFrames) {
- const Region dirty(layer->latchBuffer(visibleRegions, latchTime));
+ const Region dirty(layer->latchBuffer(visibleRegions, latchTime, getBE().flushFence));
layer->useSurfaceDamage();
invalidateLayerStack(layer, dirty);
if (layer->isBufferLatched()) {
@@ -3046,6 +3058,11 @@
}
}
+ // Clear the renderengine fence here...
+ // downstream code assumes that a cleared fence == NO_FENCE, so reassign to
+ // clear instead of sp::clear.
+ getBE().flushFence = Fence::NO_FENCE;
+
mVisibleRegionsDirty |= visibleRegions;
// If we will need to wake up at some time in the future to deal with a
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 3f3086b..a0f7c75 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -191,6 +191,9 @@
nsecs_t mTotalTime;
std::atomic<nsecs_t> mLastSwapTime;
+ // Synchronization fence from a GL composition.
+ sp<Fence> flushFence = Fence::NO_FENCE;
+
// Double- vs. triple-buffering stats
struct BufferingStats {
BufferingStats()
diff --git a/services/surfaceflinger/tests/unittests/CompositionTest.cpp b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
index 3caf1f6..7b79fbd 100644
--- a/services/surfaceflinger/tests/unittests/CompositionTest.cpp
+++ b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
@@ -478,11 +478,8 @@
EXPECT_CALL(*test->mRenderEngine, useNativeFenceSync()).WillRepeatedly(Return(true));
EXPECT_CALL(*test->mRenderEngine, createImage())
.WillOnce(Return(ByMove(std::unique_ptr<renderengine::Image>(test->mReImage))));
- EXPECT_CALL(*test->mRenderEngine, bindExternalTextureImage(DEFAULT_TEXTURE_ID, _)).Times(1);
- EXPECT_CALL(*test->mRenderEngine, checkErrors()).Times(1);
- EXPECT_CALL(*test->mReImage, setNativeWindowBuffer(_, false)).WillOnce(Return(true));
bool ignoredRecomputeVisibleRegions;
- layer->latchBuffer(ignoredRecomputeVisibleRegions, 0);
+ layer->latchBuffer(ignoredRecomputeVisibleRegions, 0, Fence::NO_FENCE);
Mock::VerifyAndClear(test->mRenderEngine);
Mock::VerifyAndClear(test->mReImage);
}