Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 0b80c743 authored by Melody Hsu's avatar Melody Hsu
Browse files

Reorder release fence attachment for non-threaded RE 

Changes when release fences are attached to layers for non-threaded
RenderEngine to ensure that fences are added and fired in the same
hop to the main thread. This removes the dependency on legacy screenshot
code and prevents a deadlock where screenshot requests are blocked on
the main thread while the main thread is blocked on the previous
screenshot request finishing.

Bug: b/351894825
Test: atest SurfaceFlinger_test
Flag: EXEMPT refactor and for flag removal
Change-Id: If9d4134aba1106484f94231c5104a57a605147b8
parent 2a10ab9b

services/surfaceflinger/RegionSamplingThread.cpp

+5 −6
Original line number Diff line number Diff line
@@ -354,14 +354,13 @@ void RegionSamplingThread::captureSample() { 
                              RenderArea::Options::CAPTURE_SECURE_LAYERS);



    FenceResult fenceResult;

    if (FlagManager::getInstance().single_hop_screenshot() &&

        mFlinger.mRenderEngine->isThreaded()) {

        std::vector<sp<LayerFE>> layerFEs;

        auto displayState = mFlinger.getSnapshotsFromMainThread(renderAreaBuilder,

                                                                getLayerSnapshotsFn, layerFEs);

    if (FlagManager::getInstance().single_hop_screenshot()) {

        std::vector<std::pair<Layer*, sp<LayerFE>>> layers;

        auto displayState =

                mFlinger.getSnapshotsFromMainThread(renderAreaBuilder, getLayerSnapshotsFn, layers);

        fenceResult = mFlinger.captureScreenshot(renderAreaBuilder, buffer, kRegionSampling,

                                                 kGrayscale, kIsProtected, kAttachGainmap, nullptr,

                                                 displayState, layerFEs)

                                                 displayState, layers)

                              .get();

    } else {

        fenceResult = mFlinger.captureScreenshotLegacy(renderAreaBuilder, getLayerSnapshotsFn,

services/surfaceflinger/SurfaceFlinger.cpp

+47 −50
Original line number Diff line number Diff line
@@ -7158,9 +7158,10 @@ void SurfaceFlinger::attachReleaseFenceFutureToLayer(Layer* layer, LayerFE* laye 
// typically a layer with DRM contents, or have the GRALLOC_USAGE_PROTECTED set on the buffer.

// A protected layer has no implication on whether it's secure, which is explicitly set by

// application to avoid being screenshot or drawn via unsecure display.

bool SurfaceFlinger::layersHasProtectedLayer(const std::vector<sp<LayerFE>>& layers) const {

bool SurfaceFlinger::layersHasProtectedLayer(

        const std::vector<std::pair<Layer*, sp<LayerFE>>>& layers) const {

    bool protectedLayerFound = false;

    for (auto& layerFE : layers) {

    for (auto& [_, layerFE] : layers) {

        protectedLayerFound |=

                (layerFE->mSnapshot->isVisible && layerFE->mSnapshot->hasProtectedContent);

        if (protectedLayerFound) {
@@ -7176,15 +7177,21 @@ bool SurfaceFlinger::layersHasProtectedLayer(const std::vector<sp<LayerFE>>& lay 
// risk of deadlocks.

std::optional<SurfaceFlinger::OutputCompositionState> SurfaceFlinger::getSnapshotsFromMainThread(

        RenderAreaBuilderVariant& renderAreaBuilder, GetLayerSnapshotsFunction getLayerSnapshotsFn,

        std::vector<sp<LayerFE>>& layerFEs) {

        std::vector<std::pair<Layer*, sp<LayerFE>>>& layers) {

    return mScheduler

            ->schedule([=, this, &renderAreaBuilder, &layerFEs]() REQUIRES(kMainThreadContext) {

            ->schedule([=, this, &renderAreaBuilder, &layers]() REQUIRES(kMainThreadContext) {

                SFTRACE_NAME("getSnapshotsFromMainThread");

                auto layers = getLayerSnapshotsFn();

                layers = getLayerSnapshotsFn();

                // Non-threaded RenderEngine eventually returns to the main thread a 2nd time

                // to complete the screenshot. Release fences should only be added during the 2nd

                // hop to main thread in order to avoid potential deadlocks from waiting for the

                // the future fence to fire.

                if (mRenderEngine->isThreaded()) {

                    for (auto& [layer, layerFE] : layers) {

                    attachReleaseFenceFutureToLayer(layer, layerFE.get(), ui::INVALID_LAYER_STACK);

                        attachReleaseFenceFutureToLayer(layer, layerFE.get(),

                                                        ui::INVALID_LAYER_STACK);

                    }

                }

                layerFEs = extractLayerFEs(layers);

                return getDisplayStateFromRenderAreaBuilder(renderAreaBuilder);

            })

            .get();
@@ -7205,15 +7212,15 @@ void SurfaceFlinger::captureScreenCommon(RenderAreaBuilderVariant renderAreaBuil 
        return;

    }



    if (FlagManager::getInstance().single_hop_screenshot() && mRenderEngine->isThreaded()) {

        std::vector<sp<LayerFE>> layerFEs;

    if (FlagManager::getInstance().single_hop_screenshot()) {

        std::vector<std::pair<Layer*, sp<LayerFE>>> layers;

        auto displayState =

                getSnapshotsFromMainThread(renderAreaBuilder, getLayerSnapshotsFn, layerFEs);

                getSnapshotsFromMainThread(renderAreaBuilder, getLayerSnapshotsFn, layers);



        const bool supportsProtected = getRenderEngine().supportsProtectedContent();

        bool hasProtectedLayer = false;

        if (allowProtected && supportsProtected) {

            hasProtectedLayer = layersHasProtectedLayer(layerFEs);

            hasProtectedLayer = layersHasProtectedLayer(layers);

        }

        const bool isProtected = hasProtectedLayer && allowProtected && supportsProtected;

        const uint32_t usage = GRALLOC_USAGE_HW_COMPOSER | GRALLOC_USAGE_HW_RENDER |
@@ -7240,7 +7247,7 @@ void SurfaceFlinger::captureScreenCommon(RenderAreaBuilderVariant renderAreaBuil 
                                                             WRITEABLE);

        auto futureFence = captureScreenshot(renderAreaBuilder, texture, false /* regionSampling */,

                                             grayscale, isProtected, attachGainmap, captureListener,

                                             displayState, layerFEs);

                                             displayState, layers);

        futureFence.get();



    } else {
@@ -7248,7 +7255,7 @@ void SurfaceFlinger::captureScreenCommon(RenderAreaBuilderVariant renderAreaBuil 
        bool hasProtectedLayer = false;

        if (allowProtected && supportsProtected) {

            auto layers = mScheduler->schedule([=]() { return getLayerSnapshotsFn(); }).get();

            hasProtectedLayer = layersHasProtectedLayer(extractLayerFEs(layers));

            hasProtectedLayer = layersHasProtectedLayer(layers);

        }

        const bool isProtected = hasProtectedLayer && allowProtected && supportsProtected;

        const uint32_t usage = GRALLOC_USAGE_HW_COMPOSER | GRALLOC_USAGE_HW_RENDER |
@@ -7316,22 +7323,13 @@ SurfaceFlinger::getDisplayStateFromRenderAreaBuilder(RenderAreaBuilderVariant& r 
    return std::nullopt;

}



std::vector<sp<LayerFE>> SurfaceFlinger::extractLayerFEs(

        const std::vector<std::pair<Layer*, sp<LayerFE>>>& layers) const {

    std::vector<sp<LayerFE>> layerFEs;

    layerFEs.reserve(layers.size());

    for (const auto& [_, layerFE] : layers) {

        layerFEs.push_back(layerFE);

    }

    return layerFEs;

}



ftl::SharedFuture<FenceResult> SurfaceFlinger::captureScreenshot(

        const RenderAreaBuilderVariant& renderAreaBuilder,

        const std::shared_ptr<renderengine::ExternalTexture>& buffer, bool regionSampling,

        bool grayscale, bool isProtected, bool attachGainmap,

        const sp<IScreenCaptureListener>& captureListener,

        std::optional<OutputCompositionState>& displayState, std::vector<sp<LayerFE>>& layerFEs) {

        std::optional<OutputCompositionState>& displayState,

        std::vector<std::pair<Layer*, sp<LayerFE>>>& layers) {

    SFTRACE_CALL();



    ScreenCaptureResults captureResults;
@@ -7350,11 +7348,9 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::captureScreenshot( 
    float displayBrightnessNits = displayState.value().displayBrightnessNits;

    float sdrWhitePointNits = displayState.value().sdrWhitePointNits;



    // Empty vector needed to pass into renderScreenImpl for legacy path

    std::vector<std::pair<Layer*, sp<android::LayerFE>>> layers;

    ftl::SharedFuture<FenceResult> renderFuture =

            renderScreenImpl(renderArea.get(), buffer, regionSampling, grayscale, isProtected,

                             attachGainmap, captureResults, displayState, layers, layerFEs);

                             attachGainmap, captureResults, displayState, layers);



    if (captureResults.capturedHdrLayers && attachGainmap &&

        FlagManager::getInstance().true_hdr_screenshots()) {
@@ -7390,7 +7386,7 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::captureScreenshot( 
            ftl::SharedFuture<FenceResult> hdrRenderFuture =

                    renderScreenImpl(renderArea.get(), hdrTexture, regionSampling, grayscale,

                                     isProtected, attachGainmap, unusedResults, displayState,

                                     layers, layerFEs);

                                     layers);



            renderFuture =

                    ftl::Future(std::move(renderFuture))
@@ -7446,9 +7442,6 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::captureScreenshotLegacy( 
    auto takeScreenshotFn = [=, this, renderAreaBuilder = std::move(renderAreaBuilder)]() REQUIRES(

                                    kMainThreadContext) mutable -> ftl::SharedFuture<FenceResult> {

        auto layers = getLayerSnapshotsFn();

        for (auto& [layer, layerFE] : layers) {

            attachReleaseFenceFutureToLayer(layer, layerFE.get(), ui::INVALID_LAYER_STACK);

        }

        auto displayState = getDisplayStateFromRenderAreaBuilder(renderAreaBuilder);



        ScreenCaptureResults captureResults;
@@ -7465,10 +7458,9 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::captureScreenshotLegacy( 
            return ftl::yield<FenceResult>(base::unexpected(NO_ERROR)).share();

        }



        auto layerFEs = extractLayerFEs(layers);

        ftl::SharedFuture<FenceResult> renderFuture =

                renderScreenImpl(renderArea.get(), buffer, regionSampling, grayscale, isProtected,

                                 attachGainmap, captureResults, displayState, layers, layerFEs);

                                 attachGainmap, captureResults, displayState, layers);



        if (captureListener) {

            // Defer blocking on renderFuture back to the Binder thread.
@@ -7501,10 +7493,10 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::renderScreenImpl( 
        const RenderArea* renderArea, const std::shared_ptr<renderengine::ExternalTexture>& buffer,

        bool regionSampling, bool grayscale, bool isProtected, bool attachGainmap,

        ScreenCaptureResults& captureResults, std::optional<OutputCompositionState>& displayState,

        std::vector<std::pair<Layer*, sp<LayerFE>>>& layers, std::vector<sp<LayerFE>>& layerFEs) {

        std::vector<std::pair<Layer*, sp<LayerFE>>>& layers) {

    SFTRACE_CALL();



    for (auto& layerFE : layerFEs) {

    for (auto& [_, layerFE] : layers) {

        frontend::LayerSnapshot* snapshot = layerFE->mSnapshot.get();

        captureResults.capturedSecureLayers |= (snapshot->isVisible && snapshot->isSecure);

        captureResults.capturedHdrLayers |= isHdrLayer(*snapshot);
@@ -7563,29 +7555,32 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::renderScreenImpl( 
    captureResults.buffer = capturedBuffer->getBuffer();



    ui::LayerStack layerStack{ui::DEFAULT_LAYER_STACK};

    if (!layerFEs.empty()) {

        const sp<LayerFE>& layerFE = layerFEs.back();

    if (!layers.empty()) {

        const sp<LayerFE>& layerFE = layers.back().second;

        layerStack = layerFE->getCompositionState()->outputFilter.layerStack;

    }



    auto copyLayerFEs = [&layerFEs]() {

        std::vector<sp<compositionengine::LayerFE>> ceLayerFEs;

        ceLayerFEs.reserve(layerFEs.size());

        for (const auto& layerFE : layerFEs) {

            ceLayerFEs.push_back(layerFE);

        }

        return ceLayerFEs;

    };



    auto present = [this, buffer = capturedBuffer, dataspace = captureResults.capturedDataspace,

                    sdrWhitePointNits, displayBrightnessNits, grayscale, isProtected,

                    layerFEs = copyLayerFEs(), layerStack, regionSampling,

                    layers = std::move(layers), layerStack, regionSampling,

                    renderArea = std::move(renderArea), renderIntent,

                    enableLocalTonemapping]() -> FenceResult {

        std::unique_ptr<compositionengine::CompositionEngine> compositionEngine =

                mFactory.createCompositionEngine();

        compositionEngine->setRenderEngine(mRenderEngine.get());



        std::vector<sp<compositionengine::LayerFE>> layerFEs;

        layerFEs.reserve(layers.size());

        for (auto& [layer, layerFE] : layers) {

            // Release fences were not yet added for non-threaded render engine. To avoid

            // deadlocks between main thread and binder threads waiting for the future fence

            // result, fences should be added to layers in the same hop onto the main thread.

            if (!mRenderEngine->isThreaded()) {

                attachReleaseFenceFutureToLayer(layer, layerFE.get(), ui::INVALID_LAYER_STACK);

            }

            layerFEs.push_back(layerFE);

        }



        compositionengine::Output::ColorProfile colorProfile{.dataspace = dataspace,

                                                             .renderIntent = renderIntent};
@@ -7644,8 +7639,10 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::renderScreenImpl( 
    // TODO(b/196334700) Once we use RenderEngineThreaded everywhere we can always defer the call

    // to CompositionEngine::present.

    ftl::SharedFuture<FenceResult> presentFuture;

    if (FlagManager::getInstance().single_hop_screenshot() && mRenderEngine->isThreaded()) {

        presentFuture = ftl::yield(present()).share();

    if (FlagManager::getInstance().single_hop_screenshot()) {

        presentFuture = mRenderEngine->isThreaded()

                ? ftl::yield(present()).share()

                : mScheduler->schedule(std::move(present)).share();

    } else {

        presentFuture = mRenderEngine->isThreaded() ? ftl::defer(std::move(present)).share()

                                                    : ftl::yield(present()).share();

services/surfaceflinger/SurfaceFlinger.h

+5 −11
Original line number Diff line number Diff line
@@ -851,13 +851,14 @@ private: 
    void attachReleaseFenceFutureToLayer(Layer* layer, LayerFE* layerFE, ui::LayerStack layerStack);



    // Checks if a protected layer exists in a list of layers.

    bool layersHasProtectedLayer(const std::vector<sp<LayerFE>>& layers) const;

    bool layersHasProtectedLayer(const std::vector<std::pair<Layer*, sp<LayerFE>>>& layers) const;



    using OutputCompositionState = compositionengine::impl::OutputCompositionState;



    std::optional<OutputCompositionState> getSnapshotsFromMainThread(

            RenderAreaBuilderVariant& renderAreaBuilder,

            GetLayerSnapshotsFunction getLayerSnapshotsFn, std::vector<sp<LayerFE>>& layerFEs);

            GetLayerSnapshotsFunction getLayerSnapshotsFn,

            std::vector<std::pair<Layer*, sp<LayerFE>>>& layers);



    void captureScreenCommon(RenderAreaBuilderVariant, GetLayerSnapshotsFunction,

                             ui::Size bufferSize, ui::PixelFormat, bool allowProtected,
@@ -866,19 +867,13 @@ private: 
    std::optional<OutputCompositionState> getDisplayStateFromRenderAreaBuilder(

            RenderAreaBuilderVariant& renderAreaBuilder) REQUIRES(kMainThreadContext);



    // Legacy layer raw pointer is not safe to access outside the main thread.

    // Creates a new vector consisting only of LayerFEs, which can be safely

    // accessed outside the main thread.

    std::vector<sp<LayerFE>> extractLayerFEs(

            const std::vector<std::pair<Layer*, sp<LayerFE>>>& layers) const;



    ftl::SharedFuture<FenceResult> captureScreenshot(

            const RenderAreaBuilderVariant& renderAreaBuilder,

            const std::shared_ptr<renderengine::ExternalTexture>& buffer, bool regionSampling,

            bool grayscale, bool isProtected, bool attachGainmap,

            const sp<IScreenCaptureListener>& captureListener,

            std::optional<OutputCompositionState>& displayState,

            std::vector<sp<LayerFE>>& layerFEs);

            std::vector<std::pair<Layer*, sp<LayerFE>>>& layers);



    ftl::SharedFuture<FenceResult> captureScreenshotLegacy(

            RenderAreaBuilderVariant, GetLayerSnapshotsFunction,
@@ -890,8 +885,7 @@ private: 
            const RenderArea*, const std::shared_ptr<renderengine::ExternalTexture>&,

            bool regionSampling, bool grayscale, bool isProtected, bool attachGainmap,

            ScreenCaptureResults&, std::optional<OutputCompositionState>& displayState,

            std::vector<std::pair<Layer*, sp<LayerFE>>>& layers,

            std::vector<sp<LayerFE>>& layerFEs);

            std::vector<std::pair<Layer*, sp<LayerFE>>>& layers);



    void readPersistentProperties();

services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h

+1 −2
Original line number Diff line number Diff line
@@ -472,12 +472,11 @@ public: 
        ScreenCaptureResults captureResults;

        auto displayState = std::optional{display->getCompositionDisplay()->getState()};

        auto layers = getLayerSnapshotsFn();

        auto layerFEs = mFlinger->extractLayerFEs(layers);



        return mFlinger->renderScreenImpl(renderArea.get(), buffer, regionSampling,

                                          false /* grayscale */, false /* isProtected */,

                                          false /* attachGainmap */, captureResults, displayState,

                                          layers, layerFEs);

                                          layers);

    }



    auto getLayerSnapshotsForScreenshotsFn(ui::LayerStack layerStack, uint32_t uid) {