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

Commit 335ef3e2 authored by Melody Hsu's avatar Melody Hsu
Browse files

Refactor obtaining display in SF main thread 

Getting the display requires the SF state lock, and this can
cause deadlocks if this work is done on a binder thread. The
eventual goal is to place all the work in renderScreenImpl on the
binder thread, so reshuffling where we get the display to an area
where we already go onto the main thread will reduce lock contention.

Additional cleanups to remove overloaded renderScreenImpl for
testable SF suite.

Bug: b/294936197
Test: atest SurfaceFlinger_test
Test: atest CompositionTest
Test: presubmit
Change-Id: I95697a2508a9d936ac9689430d9060c49ed3f039
parent 4a9f8e3e

services/surfaceflinger/SurfaceFlinger.cpp

+74 −76
Original line number Diff line number Diff line
@@ -8220,35 +8220,65 @@ void SurfaceFlinger::captureScreenCommon(RenderAreaBuilderVariant renderAreaBuil 
    futureFence.get();

}



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

        RenderAreaBuilderVariant renderAreaBuilder, GetLayerSnapshotsFunction getLayerSnapshots,

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

        bool grayscale, bool isProtected, const sp<IScreenCaptureListener>& captureListener) {

    ATRACE_CALL();



    auto takeScreenshotFn = [=, this, renderAreaBuilder = std::move(renderAreaBuilder)]() REQUIRES(

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

        // LayerSnapshots must be obtained from the main thread.

        auto layers = getLayerSnapshots();



const sp<const DisplayDevice> SurfaceFlinger::getRenderAreaDisplay(

        RenderAreaBuilderVariant& renderAreaBuilder, OutputCompositionState& state) {

    sp<const DisplayDevice> display = nullptr;

    {

        Mutex::Autolock lock(mStateLock);

        if (auto* layerRenderAreaBuilder =

                    std::get_if<LayerRenderAreaBuilder>(&renderAreaBuilder)) {

            // LayerSnapshotBuilder should only be accessed from the main thread.

            frontend::LayerSnapshot* snapshot =

            const frontend::LayerSnapshot* snapshot =

                    mLayerSnapshotBuilder.getSnapshot(layerRenderAreaBuilder->layer->getSequence());

            if (!snapshot) {

                ALOGW("Couldn't find layer snapshot for %d",

                      layerRenderAreaBuilder->layer->getSequence());

            } else {

                layerRenderAreaBuilder->setLayerSnapshot(*snapshot);

                display = findDisplay(

                        [layerStack = snapshot->outputFilter.layerStack](const auto& display) {

                            return display.getLayerStack() == layerStack;

                        });

            }

        } else if (auto* displayRenderAreaBuilder =

                           std::get_if<DisplayRenderAreaBuilder>(&renderAreaBuilder)) {

            display = displayRenderAreaBuilder->displayWeak.promote();

        }



        if (display == nullptr) {

            display = getDefaultDisplayDeviceLocked();

        }



        if (display != nullptr) {

            state = display->getCompositionDisplay()->getState();

        }

    }

    return display;

}



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

SurfaceFlinger::getLayerSnapshotsFromMainThread(GetLayerSnapshotsFunction getLayerSnapshotsFn) {

    auto layers = getLayerSnapshotsFn();

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

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

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

        }

    }

    return layers;

}



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

        RenderAreaBuilderVariant renderAreaBuilder, GetLayerSnapshotsFunction getLayerSnapshotsFn,

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

        bool grayscale, bool isProtected, const sp<IScreenCaptureListener>& captureListener) {

    ATRACE_CALL();



    auto takeScreenshotFn = [=, this, renderAreaBuilder = std::move(renderAreaBuilder)]() REQUIRES(

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

        auto layers = getLayerSnapshotsFromMainThread(getLayerSnapshotsFn);



        OutputCompositionState state;

        const auto display = getRenderAreaDisplay(renderAreaBuilder, state);



        ScreenCaptureResults captureResults;

        std::unique_ptr<const RenderArea> renderArea =
@@ -8266,7 +8296,7 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::captureScreenshot( 


        ftl::SharedFuture<FenceResult> renderFuture =

                renderScreenImpl(std::move(renderArea), buffer, regionSampling, grayscale,

                                 isProtected, captureResults, layers);

                                 isProtected, captureResults, display, state, layers);



        if (captureListener) {

            // Defer blocking on renderFuture back to the Binder thread.
@@ -8295,19 +8325,11 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::captureScreenshot( 
    return chain.share();

}



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

        std::unique_ptr<const RenderArea> renderArea, GetLayerSnapshotsFunction getLayerSnapshots,

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

        bool grayscale, bool isProtected, ScreenCaptureResults& captureResults) {

    auto layers = getLayerSnapshots();

    return renderScreenImpl(std::move(renderArea), buffer, regionSampling, grayscale, isProtected,

                            captureResults, layers);

}



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

        std::unique_ptr<const RenderArea> renderArea,

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

        bool grayscale, bool isProtected, ScreenCaptureResults& captureResults,

        const sp<const DisplayDevice> display, const OutputCompositionState& state,

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

    ATRACE_CALL();
@@ -8334,32 +8356,10 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::renderScreenImpl( 
    const bool enableLocalTonemapping = FlagManager::getInstance().local_tonemap_screenshots() &&

            !renderArea->getHintForSeamlessTransition();



    {

        Mutex::Autolock lock(mStateLock);

        const DisplayDevice* display = nullptr;

        if (parent) {

            const frontend::LayerSnapshot* snapshot =

                    mLayerSnapshotBuilder.getSnapshot(parent->sequence);

            if (snapshot) {

                display = findDisplay([layerStack = snapshot->outputFilter.layerStack](

                                              const auto& display) {

                              return display.getLayerStack() == layerStack;

                          }).get();

            }

        }



        if (display == nullptr) {

            display = renderArea->getDisplayDevice().get();

        }



        if (display == nullptr) {

            display = getDefaultDisplayDeviceLocked().get();

        }



    if (display != nullptr) {

            const auto& state = display->getCompositionDisplay()->getState();

        captureResults.capturedDataspace =

                    pickBestDataspace(requestedDataspace, display, captureResults.capturedHdrLayers,

                pickBestDataspace(requestedDataspace, display.get(),

                                  captureResults.capturedHdrLayers,

                                  renderArea->getHintForSeamlessTransition());

        sdrWhitePointNits = state.sdrWhitePointNits;
@@ -8367,7 +8367,6 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::renderScreenImpl( 
            displayBrightnessNits = sdrWhitePointNits;

        } else {

            displayBrightnessNits = state.displayBrightnessNits;



            if (!enableLocalTonemapping) {

                // Only clamp the display brightness if this is not a seamless transition.

                // Otherwise for seamless transitions it's important to match the current
@@ -8389,7 +8388,6 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::renderScreenImpl( 
            renderIntent = state.renderIntent;

        }

    }

    }



    captureResults.buffer = capturedBuffer->getBuffer();

services/surfaceflinger/SurfaceFlinger.h

+11 −10
Original line number Diff line number Diff line
@@ -898,25 +898,26 @@ private: 
                             ui::Size bufferSize, ui::PixelFormat, bool allowProtected,

                             bool grayscale, const sp<IScreenCaptureListener>&);



    using OutputCompositionState = compositionengine::impl::OutputCompositionState;



    const sp<const DisplayDevice> getRenderAreaDisplay(RenderAreaBuilderVariant& renderAreaBuilder,

                                                       OutputCompositionState& state)

            REQUIRES(kMainThreadContext);



    std::vector<std::pair<Layer*, sp<android::LayerFE>>> getLayerSnapshotsFromMainThread(

            GetLayerSnapshotsFunction getLayerSnapshotsFn) REQUIRES(kMainThreadContext);



    ftl::SharedFuture<FenceResult> captureScreenshot(

            RenderAreaBuilderVariant, GetLayerSnapshotsFunction,

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

            bool grayscale, bool isProtected, const sp<IScreenCaptureListener>&);



    // Overloaded version of renderScreenImpl that is used when layer snapshots have

    // not yet been captured, and thus cannot yet be passed in as a parameter.

    // Needed for TestableSurfaceFlinger.

    ftl::SharedFuture<FenceResult> renderScreenImpl(

            std::unique_ptr<const RenderArea>, GetLayerSnapshotsFunction,

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

            bool grayscale, bool isProtected, ScreenCaptureResults&) EXCLUDES(mStateLock)

            REQUIRES(kMainThreadContext);



    ftl::SharedFuture<FenceResult> renderScreenImpl(

            std::unique_ptr<const RenderArea>,

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

            bool grayscale, bool isProtected, ScreenCaptureResults&,

            std::vector<std::pair<Layer*, sp<android::LayerFE>>>& layers) EXCLUDES(mStateLock)

            const sp<const DisplayDevice> display, const OutputCompositionState& state,

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

            REQUIRES(kMainThreadContext);



    // If the uid provided is not UNSET_UID, the traverse will skip any layers that don't have a

services/surfaceflinger/tests/unittests/CompositionTest.cpp

+1 −1
Original line number Diff line number Diff line
@@ -215,7 +215,7 @@ void CompositionTest::captureScreenComposition() { 
                                                                      HAL_PIXEL_FORMAT_RGBA_8888, 1,

                                                                      usage);



    auto future = mFlinger.renderScreenImpl(std::move(renderArea), getLayerSnapshots,

    auto future = mFlinger.renderScreenImpl(mDisplay, std::move(renderArea), getLayerSnapshots,

                                            mCaptureScreenBuffer, regionSampling);

    ASSERT_TRUE(future.valid());

    const auto fenceResult = future.get();

services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h

+11 −6
Original line number Diff line number Diff line
@@ -485,16 +485,21 @@ public: 
        return mFlinger->setPowerModeInternal(display, mode);

    }



    auto renderScreenImpl(std::unique_ptr<const RenderArea> renderArea,

    auto renderScreenImpl(const sp<DisplayDevice> display,

                          std::unique_ptr<const RenderArea> renderArea,

                          SurfaceFlinger::GetLayerSnapshotsFunction traverseLayers,

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

                          bool regionSampling) {

        Mutex::Autolock lock(mFlinger->mStateLock);

        ftl::FakeGuard guard(kMainThreadContext);



        ScreenCaptureResults captureResults;

        return FTL_FAKE_GUARD(kMainThreadContext,

                              mFlinger->renderScreenImpl(std::move(renderArea), traverseLayers,

                                                         buffer, regionSampling,

                                                         false /* grayscale */,

                                                         false /* isProtected */, captureResults));

        SurfaceFlinger::OutputCompositionState state = display->getCompositionDisplay()->getState();

        auto layers = mFlinger->getLayerSnapshotsFromMainThread(traverseLayers);



        return mFlinger->renderScreenImpl(std::move(renderArea), buffer, regionSampling,

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

                                          captureResults, display, state, layers);

    }



    auto traverseLayersInLayerStack(ui::LayerStack layerStack, int32_t uid,