From c2edb476e76d96dc48ed0f5b46367d1b0137e8b7 Mon Sep 17 00:00:00 2001 From: Peter Cai Date: Sun, 16 Mar 2025 13:24:30 -0400 Subject: [PATCH 6/7] Revert "Remove release fence flags" This reverts commit 0077fde3aba6f2bde4e878f88c0dd466350fc1b1. Change-Id: I04eb5f0a45cc63e55b6b74f2327d182cd9f15098 --- .../src/CompositionEngine.cpp | 32 +++-- .../CompositionEngine/src/Output.cpp | 15 +- .../tests/CompositionEngineTest.cpp | 5 + .../CompositionEngine/tests/OutputTest.cpp | 134 ++++++++++++++++++ services/surfaceflinger/Layer.cpp | 59 +++++++- services/surfaceflinger/Layer.h | 16 +++ services/surfaceflinger/LayerFE.cpp | 4 +- .../surfaceflinger/RegionSamplingThread.cpp | 2 +- services/surfaceflinger/SurfaceFlinger.cpp | 123 +++++++++++----- .../TransactionCallbackInvoker.cpp | 11 +- .../TransactionCallbackInvoker.h | 1 + .../surfaceflinger/common/FlagManager.cpp | 4 + .../common/include/common/FlagManager.h | 2 + .../tests/TransactionTestHarnesses.h | 15 +- 14 files changed, 364 insertions(+), 59 deletions(-) diff --git a/services/surfaceflinger/CompositionEngine/src/CompositionEngine.cpp b/services/surfaceflinger/CompositionEngine/src/CompositionEngine.cpp index cfcce473a2..5c5d0cd74d 100644 --- a/services/surfaceflinger/CompositionEngine/src/CompositionEngine.cpp +++ b/services/surfaceflinger/CompositionEngine/src/CompositionEngine.cpp @@ -198,23 +198,25 @@ void CompositionEngine::preComposition(CompositionRefreshArgs& args) { // these buffers and fire a NO_FENCE to release it. This ensures that all // promises for buffer releases are fulfilled at the end of composition. void CompositionEngine::postComposition(CompositionRefreshArgs& args) { - SFTRACE_CALL(); - ALOGV(__FUNCTION__); - - for (auto& layerFE : args.layers) { - if (layerFE->getReleaseFencePromiseStatus() == - LayerFE::ReleaseFencePromiseStatus::INITIALIZED) { - layerFE->setReleaseFence(Fence::NO_FENCE); + if (FlagManager::getInstance().ce_fence_promise()) { + SFTRACE_CALL(); + ALOGV(__FUNCTION__); + + for (auto& layerFE : args.layers) { + if (layerFE->getReleaseFencePromiseStatus() == + LayerFE::ReleaseFencePromiseStatus::INITIALIZED) { + layerFE->setReleaseFence(Fence::NO_FENCE); + } } - } - // List of layersWithQueuedFrames does not necessarily overlap with - // list of layers, so those layersWithQueuedFrames also need any - // unfulfilled promises to be resolved for completeness. - for (auto& layerFE : args.layersWithQueuedFrames) { - if (layerFE->getReleaseFencePromiseStatus() == - LayerFE::ReleaseFencePromiseStatus::INITIALIZED) { - layerFE->setReleaseFence(Fence::NO_FENCE); + // List of layersWithQueuedFrames does not necessarily overlap with + // list of layers, so those layersWithQueuedFrames also need any + // unfulfilled promises to be resolved for completeness. + for (auto& layerFE : args.layersWithQueuedFrames) { + if (layerFE->getReleaseFencePromiseStatus() == + LayerFE::ReleaseFencePromiseStatus::INITIALIZED) { + layerFE->setReleaseFence(Fence::NO_FENCE); + } } } } diff --git a/services/surfaceflinger/CompositionEngine/src/Output.cpp b/services/surfaceflinger/CompositionEngine/src/Output.cpp index f9ed92d1ee..34773de107 100644 --- a/services/surfaceflinger/CompositionEngine/src/Output.cpp +++ b/services/surfaceflinger/CompositionEngine/src/Output.cpp @@ -1664,7 +1664,13 @@ void Output::presentFrameAndReleaseLayers(bool flushEvenWhenDisabled) { releaseFence = Fence::merge("LayerRelease", releaseFence, frame.clientTargetAcquireFence); } - layer->getLayerFE().setReleaseFence(releaseFence); + if (FlagManager::getInstance().ce_fence_promise()) { + layer->getLayerFE().setReleaseFence(releaseFence); + } else { + layer->getLayerFE() + .onLayerDisplayed(ftl::yield(std::move(releaseFence)).share(), + outputState.layerFilter.layerStack); + } } // We've got a list of layers needing fences, that are disjoint with @@ -1672,7 +1678,12 @@ void Output::presentFrameAndReleaseLayers(bool flushEvenWhenDisabled) { // supply them with the present fence. for (auto& weakLayer : mReleasedLayers) { if (const auto layer = weakLayer.promote()) { - layer->setReleaseFence(frame.presentFence); + if (FlagManager::getInstance().ce_fence_promise()) { + layer->setReleaseFence(frame.presentFence); + } else { + layer->onLayerDisplayed(ftl::yield(frame.presentFence).share(), + outputState.layerFilter.layerStack); + } } } diff --git a/services/surfaceflinger/CompositionEngine/tests/CompositionEngineTest.cpp b/services/surfaceflinger/CompositionEngine/tests/CompositionEngineTest.cpp index 3e0c390a5d..deb90deaf7 100644 --- a/services/surfaceflinger/CompositionEngine/tests/CompositionEngineTest.cpp +++ b/services/surfaceflinger/CompositionEngine/tests/CompositionEngineTest.cpp @@ -29,6 +29,8 @@ #include "TimeStats/TimeStats.h" #include "mock/DisplayHardware/MockHWComposer.h" +#include + using namespace com::android::graphics::surfaceflinger; namespace android::compositionengine { @@ -491,6 +493,9 @@ struct CompositionEnginePostCompositionTest : public CompositionEngineTest { }; TEST_F(CompositionEnginePostCompositionTest, postCompositionReleasesAllFences) { + SET_FLAG_FOR_TEST(com::android::graphics::surfaceflinger::flags::ce_fence_promise, true); + ASSERT_TRUE(FlagManager::getInstance().ce_fence_promise()); + EXPECT_CALL(*mLayer1FE, getReleaseFencePromiseStatus) .WillOnce(Return(LayerFE::ReleaseFencePromiseStatus::FULFILLED)); EXPECT_CALL(*mLayer2FE, getReleaseFencePromiseStatus) diff --git a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp index 442b603ca0..eb7f0ddc37 100644 --- a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp +++ b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp @@ -35,6 +35,7 @@ #include #include +#include #include #include @@ -3289,9 +3290,57 @@ TEST_F(OutputPostFramebufferTest, ifEnabledMustFlipThenPresentThenSendPresentCom mOutput.presentFrameAndReleaseLayers(kFlushEvenWhenDisabled); } +TEST_F(OutputPostFramebufferTest, releaseFencesAreSentToLayerFE) { + SET_FLAG_FOR_TEST(com::android::graphics::surfaceflinger::flags::ce_fence_promise, false); + ASSERT_FALSE(FlagManager::getInstance().ce_fence_promise()); + // Simulate getting release fences from each layer, and ensure they are passed to the + // front-end layer interface for each layer correctly. + + mOutput.mState.isEnabled = true; + + // Create three unique fence instances + sp layer1Fence = sp::make(); + sp layer2Fence = sp::make(); + sp layer3Fence = sp::make(); + + Output::FrameFences frameFences; + frameFences.layerFences.emplace(&mLayer1.hwc2Layer, layer1Fence); + frameFences.layerFences.emplace(&mLayer2.hwc2Layer, layer2Fence); + frameFences.layerFences.emplace(&mLayer3.hwc2Layer, layer3Fence); + + EXPECT_CALL(mOutput, presentFrame()).WillOnce(Return(frameFences)); + EXPECT_CALL(*mRenderSurface, onPresentDisplayCompleted()); + + // Compare the pointers values of each fence to make sure the correct ones + // are passed. This happens to work with the current implementation, but + // would not survive certain calls like Fence::merge() which would return a + // new instance. + EXPECT_CALL(*mLayer1.layerFE, onLayerDisplayed(_, _)) + .WillOnce([&layer1Fence](ftl::SharedFuture futureFenceResult, + ui::LayerStack) { + EXPECT_EQ(FenceResult(layer1Fence), futureFenceResult.get()); + }); + EXPECT_CALL(*mLayer2.layerFE, onLayerDisplayed(_, _)) + .WillOnce([&layer2Fence](ftl::SharedFuture futureFenceResult, + ui::LayerStack) { + EXPECT_EQ(FenceResult(layer2Fence), futureFenceResult.get()); + }); + EXPECT_CALL(*mLayer3.layerFE, onLayerDisplayed(_, _)) + .WillOnce([&layer3Fence](ftl::SharedFuture futureFenceResult, + ui::LayerStack) { + EXPECT_EQ(FenceResult(layer3Fence), futureFenceResult.get()); + }); + + constexpr bool kFlushEvenWhenDisabled = false; + mOutput.presentFrameAndReleaseLayers(kFlushEvenWhenDisabled); +} + TEST_F(OutputPostFramebufferTest, releaseFencesAreSetInLayerFE) { + SET_FLAG_FOR_TEST(com::android::graphics::surfaceflinger::flags::ce_fence_promise, true); + ASSERT_TRUE(FlagManager::getInstance().ce_fence_promise()); // Simulate getting release fences from each layer, and ensure they are passed to the // front-end layer interface for each layer correctly. + mOutput.mState.isEnabled = true; // Create three unique fence instances @@ -3328,7 +3377,37 @@ TEST_F(OutputPostFramebufferTest, releaseFencesAreSetInLayerFE) { mOutput.presentFrameAndReleaseLayers(kFlushEvenWhenDisabled); } +TEST_F(OutputPostFramebufferTest, releaseFencesIncludeClientTargetAcquireFence) { + SET_FLAG_FOR_TEST(com::android::graphics::surfaceflinger::flags::ce_fence_promise, false); + ASSERT_FALSE(FlagManager::getInstance().ce_fence_promise()); + + mOutput.mState.isEnabled = true; + mOutput.mState.usesClientComposition = true; + + Output::FrameFences frameFences; + frameFences.clientTargetAcquireFence = sp::make(); + frameFences.layerFences.emplace(&mLayer1.hwc2Layer, sp::make()); + frameFences.layerFences.emplace(&mLayer2.hwc2Layer, sp::make()); + frameFences.layerFences.emplace(&mLayer3.hwc2Layer, sp::make()); + + EXPECT_CALL(mOutput, presentFrame()).WillOnce(Return(frameFences)); + EXPECT_CALL(*mRenderSurface, onPresentDisplayCompleted()); + + // Fence::merge is called, and since none of the fences are actually valid, + // Fence::NO_FENCE is returned and passed to each onLayerDisplayed() call. + // This is the best we can do without creating a real kernel fence object. + EXPECT_CALL(*mLayer1.layerFE, onLayerDisplayed).WillOnce(Return()); + EXPECT_CALL(*mLayer2.layerFE, onLayerDisplayed).WillOnce(Return()); + EXPECT_CALL(*mLayer3.layerFE, onLayerDisplayed).WillOnce(Return()); + + constexpr bool kFlushEvenWhenDisabled = false; + mOutput.presentFrameAndReleaseLayers(kFlushEvenWhenDisabled); +} + TEST_F(OutputPostFramebufferTest, setReleaseFencesIncludeClientTargetAcquireFence) { + SET_FLAG_FOR_TEST(com::android::graphics::surfaceflinger::flags::ce_fence_promise, true); + ASSERT_TRUE(FlagManager::getInstance().ce_fence_promise()); + mOutput.mState.isEnabled = true; mOutput.mState.usesClientComposition = true; @@ -3351,7 +3430,62 @@ TEST_F(OutputPostFramebufferTest, setReleaseFencesIncludeClientTargetAcquireFenc mOutput.presentFrameAndReleaseLayers(kFlushEvenWhenDisabled); } +TEST_F(OutputPostFramebufferTest, releasedLayersSentPresentFence) { + SET_FLAG_FOR_TEST(com::android::graphics::surfaceflinger::flags::ce_fence_promise, false); + ASSERT_FALSE(FlagManager::getInstance().ce_fence_promise()); + + mOutput.mState.isEnabled = true; + mOutput.mState.usesClientComposition = true; + + // This should happen even if there are no (current) output layers. + EXPECT_CALL(mOutput, getOutputLayerCount()).WillOnce(Return(0u)); + + // Load up the released layers with some mock instances + sp> releasedLayer1 = sp>::make(); + sp> releasedLayer2 = sp>::make(); + sp> releasedLayer3 = sp>::make(); + Output::ReleasedLayers layers; + layers.push_back(releasedLayer1); + layers.push_back(releasedLayer2); + layers.push_back(releasedLayer3); + mOutput.setReleasedLayers(std::move(layers)); + + // Set up a fake present fence + sp presentFence = sp::make(); + Output::FrameFences frameFences; + frameFences.presentFence = presentFence; + + EXPECT_CALL(mOutput, presentFrame()).WillOnce(Return(frameFences)); + EXPECT_CALL(*mRenderSurface, onPresentDisplayCompleted()); + + // Each released layer should be given the presentFence. + EXPECT_CALL(*releasedLayer1, onLayerDisplayed(_, _)) + .WillOnce([&presentFence](ftl::SharedFuture futureFenceResult, + ui::LayerStack) { + EXPECT_EQ(FenceResult(presentFence), futureFenceResult.get()); + }); + EXPECT_CALL(*releasedLayer2, onLayerDisplayed(_, _)) + .WillOnce([&presentFence](ftl::SharedFuture futureFenceResult, + ui::LayerStack) { + EXPECT_EQ(FenceResult(presentFence), futureFenceResult.get()); + }); + EXPECT_CALL(*releasedLayer3, onLayerDisplayed(_, _)) + .WillOnce([&presentFence](ftl::SharedFuture futureFenceResult, + ui::LayerStack) { + EXPECT_EQ(FenceResult(presentFence), futureFenceResult.get()); + }); + + constexpr bool kFlushEvenWhenDisabled = false; + mOutput.presentFrameAndReleaseLayers(kFlushEvenWhenDisabled); + + // After the call the list of released layers should have been cleared. + EXPECT_TRUE(mOutput.getReleasedLayersForTest().empty()); +} + TEST_F(OutputPostFramebufferTest, setReleasedLayersSentPresentFence) { + SET_FLAG_FOR_TEST(com::android::graphics::surfaceflinger::flags::ce_fence_promise, true); + ASSERT_TRUE(FlagManager::getInstance().ce_fence_promise()); + mOutput.mState.isEnabled = true; mOutput.mState.usesClientComposition = true; diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 195461f47e..a8e60f1632 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -805,6 +805,54 @@ void Layer::prepareReleaseCallbacks(ftl::Future futureFenceResult, } } +void Layer::onLayerDisplayed(ftl::SharedFuture futureFenceResult, + ui::LayerStack layerStack, + std::function&& continuation) { + sp ch = findCallbackHandle(); + + if (!FlagManager::getInstance().screenshot_fence_preservation() && continuation) { + futureFenceResult = ftl::Future(futureFenceResult).then(std::move(continuation)).share(); + } + + if (ch != nullptr) { + ch->previousReleaseCallbackId = mPreviousReleaseCallbackId; + ch->previousSharedReleaseFences.emplace_back(std::move(futureFenceResult)); + ch->name = mName; + } else if (FlagManager::getInstance().screenshot_fence_preservation()) { + // If we didn't get a release callback yet, e.g. some scenarios when capturing screenshots + // asynchronously, then make sure we don't drop the fence. + mPreviousReleaseFenceAndContinuations.emplace_back(std::move(futureFenceResult), + std::move(continuation)); + std::vector mergedFences; + sp prevFence = nullptr; + // For a layer that's frequently screenshotted, try to merge fences to make sure we don't + // grow unbounded. + for (const auto& futureAndContinuation : mPreviousReleaseFenceAndContinuations) { + auto result = futureAndContinuation.future.wait_for(0s); + if (result != std::future_status::ready) { + mergedFences.emplace_back(futureAndContinuation); + continue; + } + + mergeFence(getDebugName(), + futureAndContinuation.chain().get().value_or(Fence::NO_FENCE), prevFence); + } + if (prevFence != nullptr) { + mergedFences.emplace_back(ftl::yield(FenceResult(std::move(prevFence))).share()); + } + + mPreviousReleaseFenceAndContinuations.swap(mergedFences); + } + + if (mBufferInfo.mBuffer) { + mPreviouslyPresentedLayerStacks.push_back(layerStack); + } + + if (mDrawingState.frameNumber > 0) { + mDrawingState.previousFrameNumber = mDrawingState.frameNumber; + } +} + void Layer::releasePendingBuffer(nsecs_t dequeueReadyTime) { for (const auto& handle : mDrawingState.callbackHandles) { handle->bufferReleaseChannel = mBufferReleaseChannel; @@ -1117,13 +1165,22 @@ bool Layer::setTransactionCompletedListeners(const std::vectoracquireTimeOrFence = mCallbackHandleAcquireTimeOrFence; handle->frameNumber = mDrawingState.frameNumber; handle->previousFrameNumber = mDrawingState.previousFrameNumber; - if (mPreviousReleaseBufferEndpoint == handle->listener) { + if (FlagManager::getInstance().ce_fence_promise() && + mPreviousReleaseBufferEndpoint == handle->listener) { // Add fence from previous screenshot now so that it can be dispatched to the // client. for (auto& [_, future] : mAdditionalPreviousReleaseFences) { handle->previousReleaseFences.emplace_back(std::move(future)); } mAdditionalPreviousReleaseFences.clear(); + } else if (FlagManager::getInstance().screenshot_fence_preservation() && + mPreviousReleaseBufferEndpoint == handle->listener) { + // Add fences from previous screenshots now so that they can be dispatched to the + // client. + for (const auto& futureAndContinution : mPreviousReleaseFenceAndContinuations) { + handle->previousSharedReleaseFences.emplace_back(futureAndContinution.chain()); + } + mPreviousReleaseFenceAndContinuations.clear(); } // Store so latched time and release fence can be set mDrawingState.callbackHandles.push_back(handle); diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index c234a75693..c4ffe1ef2d 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -261,6 +261,8 @@ public: bool fenceHasSignaled() const; void onPreComposition(nsecs_t refreshStartTime); + void onLayerDisplayed(ftl::SharedFuture, ui::LayerStack layerStack, + std::function&& continuation = nullptr); // Tracks mLastClientCompositionFence and gets the callback handle for this layer. sp findCallbackHandle(); @@ -389,6 +391,20 @@ public: // from the layer. std::vector mPreviouslyPresentedLayerStacks; + struct FenceAndContinuation { + ftl::SharedFuture future; + std::function continuation; + + ftl::SharedFuture chain() const { + if (continuation) { + return ftl::Future(future).then(continuation).share(); + } else { + return future; + } + } + }; + std::vector mPreviousReleaseFenceAndContinuations; + // Release fences for buffers that have not yet received a release // callback. A release callback may not be given when capturing // screenshots asynchronously. There may be no buffer update for the diff --git a/services/surfaceflinger/LayerFE.cpp b/services/surfaceflinger/LayerFE.cpp index fea7671af2..de64b271f5 100644 --- a/services/surfaceflinger/LayerFE.cpp +++ b/services/surfaceflinger/LayerFE.cpp @@ -26,6 +26,7 @@ #include "LayerFE.h" #include "SurfaceFlinger.h" +#include "common/FlagManager.h" #include "ui/FenceResult.h" namespace android { @@ -82,7 +83,8 @@ LayerFE::~LayerFE() { // Ensures that no promise is left unfulfilled before the LayerFE is destroyed. // An unfulfilled promise could occur when a screenshot is attempted, but the // render area is invalid and there is no memory for the capture result. - if (mReleaseFencePromiseStatus == ReleaseFencePromiseStatus::INITIALIZED) { + if (FlagManager::getInstance().ce_fence_promise() && + mReleaseFencePromiseStatus == ReleaseFencePromiseStatus::INITIALIZED) { setReleaseFence(Fence::NO_FENCE); } } diff --git a/services/surfaceflinger/RegionSamplingThread.cpp b/services/surfaceflinger/RegionSamplingThread.cpp index 011fd9e20a..06c2f26a6d 100644 --- a/services/surfaceflinger/RegionSamplingThread.cpp +++ b/services/surfaceflinger/RegionSamplingThread.cpp @@ -355,7 +355,7 @@ void RegionSamplingThread::captureSample() { FenceResult fenceResult; if (FlagManager::getInstance().single_hop_screenshot() && - mFlinger.mRenderEngine->isThreaded()) { + FlagManager::getInstance().ce_fence_promise() && mFlinger.mRenderEngine->isThreaded()) { std::vector> layerFEs; auto displayState = mFlinger.getSnapshotsFromMainThread(renderAreaBuilder, getLayerSnapshotsFn, layerFEs); diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 067a8bafe0..47a3c2c2bf 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -2820,6 +2820,16 @@ CompositeResultsPerDisplay SurfaceFlinger::composite( compositionengine::Feature::kSnapshotLayerMetadata); refreshArgs.bufferIdsToUncache = std::move(mBufferIdsToUncache); + + if (!FlagManager::getInstance().ce_fence_promise()) { + refreshArgs.layersWithQueuedFrames.reserve(mLayersWithQueuedFrames.size()); + for (auto& [layer, _] : mLayersWithQueuedFrames) { + if (const auto& layerFE = layer->getCompositionEngineLayerFE( + {static_cast(layer->sequence)})) + refreshArgs.layersWithQueuedFrames.push_back(layerFE); + } + } + refreshArgs.outputColorSetting = mDisplayColorSetting; refreshArgs.forceOutputColorMode = mForceColorMode; @@ -2883,38 +2893,54 @@ CompositeResultsPerDisplay SurfaceFlinger::composite( layer->onPreComposition(refreshArgs.refreshStartTime); } - for (auto& [layer, layerFE] : layers) { - attachReleaseFenceFutureToLayer(layer, layerFE, - layerFE->mSnapshot->outputFilter.layerStack); - } - - refreshArgs.layersWithQueuedFrames.reserve(mLayersWithQueuedFrames.size()); - for (auto& [layer, _] : mLayersWithQueuedFrames) { - if (const auto& layerFE = - layer->getCompositionEngineLayerFE({static_cast(layer->sequence)})) { - refreshArgs.layersWithQueuedFrames.push_back(layerFE); - // Some layers are not displayed and do not yet have a future release fence - if (layerFE->getReleaseFencePromiseStatus() == - LayerFE::ReleaseFencePromiseStatus::UNINITIALIZED || - layerFE->getReleaseFencePromiseStatus() == - LayerFE::ReleaseFencePromiseStatus::FULFILLED) { - // layerStack is invalid because layer is not on a display - attachReleaseFenceFutureToLayer(layer.get(), layerFE.get(), - ui::INVALID_LAYER_STACK); + if (FlagManager::getInstance().ce_fence_promise()) { + for (auto& [layer, layerFE] : layers) { + attachReleaseFenceFutureToLayer(layer, layerFE, + layerFE->mSnapshot->outputFilter.layerStack); + } + + refreshArgs.layersWithQueuedFrames.reserve(mLayersWithQueuedFrames.size()); + for (auto& [layer, _] : mLayersWithQueuedFrames) { + if (const auto& layerFE = layer->getCompositionEngineLayerFE( + {static_cast(layer->sequence)})) { + refreshArgs.layersWithQueuedFrames.push_back(layerFE); + // Some layers are not displayed and do not yet have a future release fence + if (layerFE->getReleaseFencePromiseStatus() == + LayerFE::ReleaseFencePromiseStatus::UNINITIALIZED || + layerFE->getReleaseFencePromiseStatus() == + LayerFE::ReleaseFencePromiseStatus::FULFILLED) { + // layerStack is invalid because layer is not on a display + attachReleaseFenceFutureToLayer(layer.get(), layerFE.get(), + ui::INVALID_LAYER_STACK); + } } } - } - mCompositionEngine->present(refreshArgs); - moveSnapshotsFromCompositionArgs(refreshArgs, layers); + mCompositionEngine->present(refreshArgs); + moveSnapshotsFromCompositionArgs(refreshArgs, layers); - for (auto& [layer, layerFE] : layers) { - CompositionResult compositionResult{layerFE->stealCompositionResult()}; - if (compositionResult.lastClientCompositionFence) { - layer->setWasClientComposed(compositionResult.lastClientCompositionFence); + for (auto& [layer, layerFE] : layers) { + CompositionResult compositionResult{layerFE->stealCompositionResult()}; + if (compositionResult.lastClientCompositionFence) { + layer->setWasClientComposed(compositionResult.lastClientCompositionFence); + } } - if (com_android_graphics_libgui_flags_apply_picture_profiles()) { - mActivePictureUpdater.onLayerComposed(*layer, *layerFE, compositionResult); + + } else { + mCompositionEngine->present(refreshArgs); + moveSnapshotsFromCompositionArgs(refreshArgs, layers); + + for (auto [layer, layerFE] : layers) { + CompositionResult compositionResult{layerFE->stealCompositionResult()}; + for (auto& [releaseFence, layerStack] : compositionResult.releaseFences) { + layer->onLayerDisplayed(std::move(releaseFence), layerStack); + } + if (compositionResult.lastClientCompositionFence) { + layer->setWasClientComposed(compositionResult.lastClientCompositionFence); + } + if (com_android_graphics_libgui_flags_apply_picture_profiles()) { + mActivePictureUpdater.onLayerComposed(*layer, *layerFE, compositionResult); + } } } @@ -3196,8 +3222,13 @@ void SurfaceFlinger::onCompositionPresented(PhysicalDisplayId pacesetterId, auto optDisplay = layerStackToDisplay.get(layerStack); if (optDisplay && !optDisplay->get()->isVirtual()) { auto fence = getHwComposer().getPresentFence(optDisplay->get()->getPhysicalId()); - layer->prepareReleaseCallbacks(ftl::yield(fence), - ui::INVALID_LAYER_STACK); + if (FlagManager::getInstance().ce_fence_promise()) { + layer->prepareReleaseCallbacks(ftl::yield(fence), + ui::INVALID_LAYER_STACK); + } else { + layer->onLayerDisplayed(ftl::yield(fence).share(), + ui::INVALID_LAYER_STACK); + } } } layer->releasePendingBuffer(presentTime.ns()); @@ -7382,7 +7413,8 @@ void SurfaceFlinger::captureScreenCommon(RenderAreaBuilderVariant renderAreaBuil return; } - if (FlagManager::getInstance().single_hop_screenshot() && mRenderEngine->isThreaded()) { + if (FlagManager::getInstance().single_hop_screenshot() && + FlagManager::getInstance().ce_fence_promise() && mRenderEngine->isThreaded()) { std::vector> layerFEs; auto displayState = getSnapshotsFromMainThread(renderAreaBuilder, getLayerSnapshotsFn, layerFEs); @@ -7623,8 +7655,10 @@ ftl::SharedFuture SurfaceFlinger::captureScreenshotLegacy( auto takeScreenshotFn = [=, this, renderAreaBuilder = std::move(renderAreaBuilder)]() REQUIRES( kMainThreadContext) mutable -> ftl::SharedFuture { auto layers = getLayerSnapshotsFn(); - for (auto& [layer, layerFE] : layers) { - attachReleaseFenceFutureToLayer(layer, layerFE.get(), ui::INVALID_LAYER_STACK); + if (FlagManager::getInstance().ce_fence_promise()) { + for (auto& [layer, layerFE] : layers) { + attachReleaseFenceFutureToLayer(layer, layerFE.get(), ui::INVALID_LAYER_STACK); + } } auto displayState = getDisplayStateFromRenderAreaBuilder(renderAreaBuilder); @@ -7821,13 +7855,36 @@ ftl::SharedFuture SurfaceFlinger::renderScreenImpl( // TODO(b/196334700) Once we use RenderEngineThreaded everywhere we can always defer the call // to CompositionEngine::present. ftl::SharedFuture presentFuture; - if (FlagManager::getInstance().single_hop_screenshot() && mRenderEngine->isThreaded()) { + if (FlagManager::getInstance().single_hop_screenshot() && + FlagManager::getInstance().ce_fence_promise() && mRenderEngine->isThreaded()) { presentFuture = ftl::yield(present()).share(); } else { presentFuture = mRenderEngine->isThreaded() ? ftl::defer(std::move(present)).share() : ftl::yield(present()).share(); } + if (!FlagManager::getInstance().ce_fence_promise()) { + for (auto& [layer, layerFE] : layers) { + layer->onLayerDisplayed(presentFuture, ui::INVALID_LAYER_STACK, + [layerFE = std::move(layerFE)](FenceResult) { + if (FlagManager::getInstance() + .screenshot_fence_preservation()) { + const auto compositionResult = + layerFE->stealCompositionResult(); + const auto& fences = compositionResult.releaseFences; + // CompositionEngine may choose to cull layers that + // aren't visible, so pass a non-fence. + return fences.empty() ? Fence::NO_FENCE + : fences.back().first.get(); + } else { + return layerFE->stealCompositionResult() + .releaseFences.back() + .first.get(); + } + }); + } + } + return presentFuture; } diff --git a/services/surfaceflinger/TransactionCallbackInvoker.cpp b/services/surfaceflinger/TransactionCallbackInvoker.cpp index b22ec66819..cc79acbbf5 100644 --- a/services/surfaceflinger/TransactionCallbackInvoker.cpp +++ b/services/surfaceflinger/TransactionCallbackInvoker.cpp @@ -28,6 +28,7 @@ #include "Utils/FenceUtils.h" #include +#include #include #include @@ -126,8 +127,14 @@ status_t TransactionCallbackInvoker::addCallbackHandle(const sp& if (surfaceControl) { sp prevFence = nullptr; - for (auto& future : handle->previousReleaseFences) { - mergeFence(handle->name.c_str(), future.get().value_or(Fence::NO_FENCE), prevFence); + if (FlagManager::getInstance().ce_fence_promise()) { + for (auto& future : handle->previousReleaseFences) { + mergeFence(handle->name.c_str(), future.get().value_or(Fence::NO_FENCE), prevFence); + } + } else { + for (const auto& future : handle->previousSharedReleaseFences) { + mergeFence(handle->name.c_str(), future.get().value_or(Fence::NO_FENCE), prevFence); + } } handle->previousReleaseFence = prevFence; diff --git a/services/surfaceflinger/TransactionCallbackInvoker.h b/services/surfaceflinger/TransactionCallbackInvoker.h index 178ddbbe79..9c7bb0d588 100644 --- a/services/surfaceflinger/TransactionCallbackInvoker.h +++ b/services/surfaceflinger/TransactionCallbackInvoker.h @@ -43,6 +43,7 @@ public: std::string name; sp previousReleaseFence; std::vector> previousReleaseFences; + std::vector> previousSharedReleaseFences; std::variant> acquireTimeOrFence = -1; nsecs_t latchTime = -1; std::optional transformHint = std::nullopt; diff --git a/services/surfaceflinger/common/FlagManager.cpp b/services/surfaceflinger/common/FlagManager.cpp index 5c417ba748..b86e815b57 100644 --- a/services/surfaceflinger/common/FlagManager.cpp +++ b/services/surfaceflinger/common/FlagManager.cpp @@ -167,6 +167,8 @@ void FlagManager::dump(std::string& result) const { DUMP_ACONFIG_FLAG(begone_bright_hlg); DUMP_ACONFIG_FLAG(window_blur_kawase2); DUMP_ACONFIG_FLAG(single_hop_screenshot); + DUMP_ACONFIG_FLAG(screenshot_fence_preservation); + DUMP_ACONFIG_FLAG(ce_fence_promise); #undef DUMP_ACONFIG_FLAG #undef DUMP_LEGACY_SERVER_FLAG @@ -268,6 +270,8 @@ FLAG_MANAGER_ACONFIG_FLAG(deprecate_frame_tracker, ""); FLAG_MANAGER_ACONFIG_FLAG(skip_invisible_windows_in_input, ""); FLAG_MANAGER_ACONFIG_FLAG(begone_bright_hlg, "debug.sf.begone_bright_hlg"); FLAG_MANAGER_ACONFIG_FLAG(window_blur_kawase2, ""); +FLAG_MANAGER_ACONFIG_FLAG(screenshot_fence_preservation, "debug.sf.screenshot_fence_preservation"); +FLAG_MANAGER_ACONFIG_FLAG(ce_fence_promise, ""); /// Trunk stable server (R/W) flags /// FLAG_MANAGER_ACONFIG_FLAG(refresh_rate_overlay_on_external_display, "") diff --git a/services/surfaceflinger/common/include/common/FlagManager.h b/services/surfaceflinger/common/include/common/FlagManager.h index 15284ba0ee..4766f6be38 100644 --- a/services/surfaceflinger/common/include/common/FlagManager.h +++ b/services/surfaceflinger/common/include/common/FlagManager.h @@ -74,6 +74,7 @@ public: bool fp16_client_target() const; bool game_default_frame_rate() const; bool enable_layer_command_batching() const; + bool screenshot_fence_preservation() const; bool vulkan_renderengine() const; bool vrr_bugfix_24q4() const; bool vrr_bugfix_dropped_frame() const; @@ -82,6 +83,7 @@ public: bool dont_skip_on_early_ro() const; bool no_vsyncs_on_screen_off() const; bool protected_if_client() const; + bool ce_fence_promise() const; bool idle_screen_refresh_rate_timeout() const; bool graphite_renderengine() const; bool filter_frames_before_trace_starts() const; diff --git a/services/surfaceflinger/tests/TransactionTestHarnesses.h b/services/surfaceflinger/tests/TransactionTestHarnesses.h index c95c875746..5899c7e8b6 100644 --- a/services/surfaceflinger/tests/TransactionTestHarnesses.h +++ b/services/surfaceflinger/tests/TransactionTestHarnesses.h @@ -17,6 +17,7 @@ #define ANDROID_TRANSACTION_TEST_HARNESSES #include +#include #include #include "LayerTransactionTest.h" @@ -95,8 +96,12 @@ public: #endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ) t.setDisplayProjection(vDisplay, displayState.orientation, Rect(displayState.layerStackSpaceRect), Rect(resolution)); - t.setDisplayLayerStack(vDisplay, layerStack); - t.setLayerStack(mirrorSc, layerStack); + if (FlagManager::getInstance().ce_fence_promise()) { + t.setDisplayLayerStack(vDisplay, layerStack); + t.setLayerStack(mirrorSc, layerStack); + } else { + t.setDisplayLayerStack(vDisplay, ui::DEFAULT_LAYER_STACK); + } t.apply(); SurfaceComposerClient::Transaction().apply(true); @@ -116,8 +121,10 @@ public: // CompositionEngine::present may attempt to be called on the same // display multiple times. The layerStack is set to invalid here so // that the display is ignored if that scenario occurs. - t.setLayerStack(mirrorSc, ui::INVALID_LAYER_STACK); - t.apply(true); + if (FlagManager::getInstance().ce_fence_promise()) { + t.setLayerStack(mirrorSc, ui::INVALID_LAYER_STACK); + t.apply(true); + } SurfaceComposerClient::destroyVirtualDisplay(vDisplay); return sc; } -- 2.48.1