From b89cd9e4add465d850fe7e0bab57c6ad6f133c69 Mon Sep 17 00:00:00 2001 From: Peter Cai Date: Sun, 15 Jun 2025 18:57:59 -0400 Subject: [PATCH 6/6] Revert "Remove release fence flags" This reverts commit 0077fde3aba6f2bde4e878f88c0dd466350fc1b1. Change-Id: I339bebac4a7875ccef3b0c5d5111ecfdab35af99 Conflicts: services/surfaceflinger/CompositionEngine/src/Output.cpp services/surfaceflinger/Layer.cpp services/surfaceflinger/SurfaceFlinger.cpp services/surfaceflinger/common/FlagManager.cpp services/surfaceflinger/common/include/common/FlagManager.h services/surfaceflinger/tests/TransactionTestHarnesses.h --- .../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 | 96 +++++++++---- .../TransactionCallbackInvoker.cpp | 10 +- .../TransactionCallbackInvoker.h | 1 + .../surfaceflinger/common/FlagManager.cpp | 4 + .../common/include/common/FlagManager.h | 2 + .../tests/TransactionTestHarnesses.h | 15 +- 14 files changed, 345 insertions(+), 50 deletions(-) diff --git a/services/surfaceflinger/CompositionEngine/src/CompositionEngine.cpp b/services/surfaceflinger/CompositionEngine/src/CompositionEngine.cpp index ab2a03cd60..787bc23389 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 00a61a5ab6..7c7537bbbe 100644 --- a/services/surfaceflinger/CompositionEngine/src/Output.cpp +++ b/services/surfaceflinger/CompositionEngine/src/Output.cpp @@ -1675,7 +1675,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); + } layer->getLayerFE().setReleasedBuffer(layer->getLayerFE().getCompositionState()->buffer); } @@ -1684,7 +1690,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 34c09db6f8..dd012dad75 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 { @@ -504,6 +506,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 590626ace5..38ff11f092 100644 --- a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp +++ b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp @@ -35,6 +35,7 @@ #include #include +#include #include #include @@ -3304,9 +3305,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 @@ -3363,7 +3412,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; @@ -3406,7 +3485,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 2e312827f2..cff489062c 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -812,6 +812,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; @@ -1128,13 +1176,22 @@ bool Layer::setTransactionCompletedListeners(const std::vectorframeNumber = mDrawingState.frameNumber; handle->previousFrameNumber = mDrawingState.previousFrameNumber; handle->previousBuffer = mDrawingState.previousBuffer; - 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 88754f9fa3..16324a426c 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -262,6 +262,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(); @@ -390,6 +392,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 7ae407da1a..21a1ce733c 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" #include "ui/LayerStack.h" @@ -83,7 +84,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 7e2144d6b1..c9f0bfb928 100644 --- a/services/surfaceflinger/RegionSamplingThread.cpp +++ b/services/surfaceflinger/RegionSamplingThread.cpp @@ -357,7 +357,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(screenshotArgs, getLayerSnapshotsFn, layerFEs); diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 235ddfb762..fbae69f30f 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -2918,6 +2918,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; @@ -2981,24 +2991,26 @@ 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); + } } } } @@ -3020,6 +3032,11 @@ CompositeResultsPerDisplay SurfaceFlinger::composite( uint64_t prevOverrideBufferId = 0; for (auto& [layer, layerFE] : layers) { CompositionResult compositionResult{layerFE->stealCompositionResult()}; + if (FlagManager::getInstance().ce_fence_promise()) { + for (auto& [releaseFence, layerStack] : compositionResult.releaseFences) { + layer->onLayerDisplayed(std::move(releaseFence), layerStack); + } + } if (lastLayerStack != layerFE->mSnapshot->outputFilter.layerStack) { if (lastLayerStack != ui::INVALID_LAYER_STACK) { // add a space to separate displays @@ -3345,8 +3362,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()); @@ -7699,7 +7721,8 @@ void SurfaceFlinger::captureScreenCommon(ScreenshotArgs& args, 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; bool hasDisplayState = getSnapshotsFromMainThread(args, getLayerSnapshotsFn, layerFEs); if (!hasDisplayState) { @@ -7948,8 +7971,10 @@ ftl::SharedFuture SurfaceFlinger::captureScreenshotLegacy( 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); + } } ScreenCaptureResults captureResults; @@ -8130,13 +8155,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 2e8c8c1111..e01c494e15 100644 --- a/services/surfaceflinger/TransactionCallbackInvoker.cpp +++ b/services/surfaceflinger/TransactionCallbackInvoker.cpp @@ -127,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 34f6ffc5da..a9d62d9fd1 100644 --- a/services/surfaceflinger/TransactionCallbackInvoker.h +++ b/services/surfaceflinger/TransactionCallbackInvoker.h @@ -44,6 +44,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 6f3a58f6ff..d52957876b 100644 --- a/services/surfaceflinger/common/FlagManager.cpp +++ b/services/surfaceflinger/common/FlagManager.cpp @@ -169,6 +169,8 @@ void FlagManager::dump(std::string& result) const { DUMP_ACONFIG_FLAG(no_vsyncs_on_screen_off); DUMP_ACONFIG_FLAG(override_trusted_overlay); DUMP_ACONFIG_FLAG(protected_if_client); + DUMP_ACONFIG_FLAG(ce_fence_promise); + DUMP_ACONFIG_FLAG(screenshot_fence_preservation); DUMP_ACONFIG_FLAG(reject_dupe_layerstacks); DUMP_ACONFIG_FLAG(renderable_buffer_usage); DUMP_ACONFIG_FLAG(restore_blur_step); @@ -279,6 +281,8 @@ FLAG_MANAGER_ACONFIG_FLAG(no_vsyncs_on_screen_off, "debug.sf.no_vsyncs_on_screen FLAG_MANAGER_ACONFIG_FLAG(protected_if_client, "") FLAG_MANAGER_ACONFIG_FLAG(vrr_bugfix_24q4, ""); FLAG_MANAGER_ACONFIG_FLAG(vrr_bugfix_dropped_frame, "") +FLAG_MANAGER_ACONFIG_FLAG(ce_fence_promise, "") +FLAG_MANAGER_ACONFIG_FLAG(screenshot_fence_preservation, "debug.sf.screenshot_fence_preservation") FLAG_MANAGER_ACONFIG_FLAG(graphite_renderengine, "debug.renderengine.graphite") FLAG_MANAGER_ACONFIG_FLAG(filter_frames_before_trace_starts, "") FLAG_MANAGER_ACONFIG_FLAG(latch_unsignaled_with_auto_refresh_changed, ""); diff --git a/services/surfaceflinger/common/include/common/FlagManager.h b/services/surfaceflinger/common/include/common/FlagManager.h index 23232c381f..91399c1dda 100644 --- a/services/surfaceflinger/common/include/common/FlagManager.h +++ b/services/surfaceflinger/common/include/common/FlagManager.h @@ -71,6 +71,8 @@ public: /// IMPORTANT - please keep alphabetize to reduce merge conflicts bool add_sf_skipped_frames_to_trace() const; bool adpf_fmq_sf() const; + bool ce_fence_promise() const; + bool screenshot_fence_preservation() const; bool allow_n_vsyncs_in_targeter() const; bool arr_setframerate_gte_enum() const; bool begone_bright_hlg() const; diff --git a/services/surfaceflinger/tests/TransactionTestHarnesses.h b/services/surfaceflinger/tests/TransactionTestHarnesses.h index c91f1ea760..186c3a8c5a 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" @@ -99,8 +100,12 @@ public: #endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ) t.setDisplayProjection(vDisplay, ui::Rotation::Rotation0, Rect(resolution), 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); @@ -120,8 +125,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