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

Commit 0199b205 authored by Melody Hsu's avatar Melody Hsu
Browse files

Cleaner ScreenshotArg struct init, function parameter cleanup 

Uses a more readable syntax for ScreenshotArgs struct initialization
and places more screenshot args in the struct, reducing the length
of the parameter list for screenshot functions.

Bug: b/377758217
Test: atest SurfaceFlinger_test
Test: atest CompositionTest
Test: atest RegionSamplingtest
Flag: EXEMPT, refactor
Change-Id: I0d3c09d4ccfbe877b2d95065031efa5d64701acd
parent b2d8f7e2

services/surfaceflinger/RegionSamplingThread.cpp

+14 −15
Original line number Diff line number Diff line
@@ -361,24 +361,23 @@ void RegionSamplingThread::captureSample() { 
                                                             WRITEABLE);

    }



    constexpr bool kRegionSampling = true;

    constexpr bool kGrayscale = false;

    constexpr bool kIsProtected = false;



    SurfaceFlinger::ScreenshotArgs screenshotArgs;

    screenshotArgs.captureTypeVariant = displayWeak;

    screenshotArgs.displayIdVariant = std::nullopt;

    screenshotArgs.sourceCrop = sampledBounds.isEmpty() ? layerStackSpaceRect : sampledBounds;

    screenshotArgs.reqSize = sampledBounds.getSize();

    screenshotArgs.dataspace = ui::Dataspace::V0_SRGB;

    screenshotArgs.isSecure = true;

    screenshotArgs.seamlessTransition = false;

    SurfaceFlinger::ScreenshotArgs screenshotArgs{.captureTypeVariant = displayWeak,

                                                  .displayIdVariant = std::nullopt,

                                                  .sourceCrop = sampledBounds.isEmpty()

                                                          ? layerStackSpaceRect

                                                          : sampledBounds,

                                                  .size = sampledBounds.getSize(),

                                                  .dataspace = ui::Dataspace::V0_SRGB,

                                                  .disableBlur = true,

                                                  .isGrayscale = false,

                                                  .isSecure = true,

                                                  .seamlessTransition = false,

                                                  .debugName = "RegionSampling"};



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

    mFlinger.getSnapshotsFromMainThread(screenshotArgs, getLayerSnapshotsFn, layers);

    FenceResult fenceResult = mFlinger.captureScreenshot(screenshotArgs, buffer, kRegionSampling,

                                                         kGrayscale, kIsProtected, nullptr, layers)

                                      .get();

    FenceResult fenceResult =

            mFlinger.captureScreenshot(screenshotArgs, buffer, nullptr, layers).get();

    if (fenceResult.ok()) {

        fenceResult.value()->waitForever(LOG_TAG);

    }

services/surfaceflinger/ScreenCaptureOutput.cpp

+9 −18
Original line number Diff line number Diff line
@@ -33,13 +33,12 @@ std::shared_ptr<ScreenCaptureOutput> createScreenCaptureOutput(ScreenCaptureOutp 
            /* sourceCrop */ const Rect, ftl::Optional<DisplayIdVariant>,

            const compositionengine::Output::ColorProfile&,

            /* layerAlpha */ float,

            /* regionSampling */ bool>(args.compositionEngine, args.sourceCrop,

                                       args.displayIdVariant, args.colorProfile, args.layerAlpha,

                                       args.regionSampling,

            /* disableBlur */ bool>(args.compositionEngine, args.sourceCrop, args.displayIdVariant,

                                    args.colorProfile, args.layerAlpha, args.disableBlur,

                                    args.dimInGammaSpaceForEnhancedScreenshots,

                                    args.enableLocalTonemapping);

    output->editState().isSecure = args.isSecure;

    output->editState().isProtected = args.isProtected;

    output->editState().isProtected = args.buffer->getUsage() & GRALLOC_USAGE_PROTECTED;

    output->setCompositionEnabled(true);

    output->setLayerFilter({args.layerStack});

    output->setRenderSurface(std::make_unique<ScreenCaptureRenderSurface>(std::move(args.buffer)));
@@ -57,27 +56,19 @@ std::shared_ptr<ScreenCaptureOutput> createScreenCaptureOutput(ScreenCaptureOutp 
    output->setDisplaySize({sourceCrop.getWidth(), sourceCrop.getHeight()});

    output->setProjection(orientation, sourceCrop,

                          {args.reqBufferSize.width, args.reqBufferSize.height});



    {

        std::string name = args.regionSampling ? "RegionSampling" : "ScreenCaptureOutput";

        if (const auto id = args.displayIdVariant.and_then(asDisplayIdOfType<DisplayId>)) {

            base::StringAppendF(&name, " for %" PRIu64, id->value);

        }

        output->setName(name);

    }

    output->setName(args.debugName);

    return output;

}



ScreenCaptureOutput::ScreenCaptureOutput(

        const Rect sourceCrop, ftl::Optional<DisplayIdVariant> displayIdVariant,

        const compositionengine::Output::ColorProfile& colorProfile, float layerAlpha,

        bool regionSampling, bool dimInGammaSpaceForEnhancedScreenshots,

        bool enableLocalTonemapping)

        bool disableBlur, bool dimInGammaSpaceForEnhancedScreenshots, bool enableLocalTonemapping)

      : mSourceCrop(sourceCrop),

        mDisplayIdVariant(displayIdVariant),

        mColorProfile(colorProfile),

        mLayerAlpha(layerAlpha),

        mRegionSampling(regionSampling),

        mDisableBlur(disableBlur),

        mDimInGammaSpaceForEnhancedScreenshots(dimInGammaSpaceForEnhancedScreenshots),

        mEnableLocalTonemapping(enableLocalTonemapping) {}
@@ -189,7 +180,7 @@ ScreenCaptureOutput::generateClientCompositionRequests( 
        }

    }



    if (mRegionSampling) {

    if (mDisableBlur) {

        for (auto& layer : clientCompositionLayers) {

            layer.backgroundBlurRadius = 0;

            layer.blurRegions.clear();

services/surfaceflinger/ScreenCaptureOutput.h

+4 −4
Original line number Diff line number Diff line
@@ -37,12 +37,12 @@ struct ScreenCaptureOutputArgs { 
    // Counterintuitively, when targetBrightness > 1.0 then dim the scene.

    float targetBrightness;

    float layerAlpha;

    bool regionSampling;

    bool disableBlur;

    bool treat170mAsSrgb;

    bool dimInGammaSpaceForEnhancedScreenshots;

    bool isSecure = false;

    bool isProtected = false;

    bool enableLocalTonemapping = false;

    std::string debugName;

};



// ScreenCaptureOutput is used to compose a set of layers into a preallocated buffer.
@@ -53,7 +53,7 @@ class ScreenCaptureOutput : public compositionengine::impl::Output { 
public:

    ScreenCaptureOutput(const Rect sourceCrop, ftl::Optional<DisplayIdVariant> displayIdVariant,

                        const compositionengine::Output::ColorProfile& colorProfile,

                        float layerAlpha, bool regionSampling,

                        float layerAlpha, bool disableBlur,

                        bool dimInGammaSpaceForEnhancedScreenshots, bool enableLocalTonemapping);



    void updateColorProfile(const compositionengine::CompositionRefreshArgs&) override;
@@ -73,7 +73,7 @@ private: 
    const ftl::Optional<DisplayIdVariant> mDisplayIdVariant;

    const compositionengine::Output::ColorProfile& mColorProfile;

    const float mLayerAlpha;

    const bool mRegionSampling;

    const bool mDisableBlur;

    const bool mDimInGammaSpaceForEnhancedScreenshots;

    const bool mEnableLocalTonemapping;

};

services/surfaceflinger/SurfaceFlinger.cpp

+59 −57
Original line number Diff line number Diff line
@@ -7481,21 +7481,24 @@ void SurfaceFlinger::captureDisplay(const DisplayCaptureArgs& args, 
            getLayerSnapshotsForScreenshots(layerStack, captureArgs.uid,

                                            std::move(excludeLayerIds));



    ScreenshotArgs screenshotArgs;

    screenshotArgs.captureTypeVariant = displayWeak;

    screenshotArgs.displayIdVariant = displayIdVariantOpt;

    screenshotArgs.sourceCrop = gui::aidl_utils::fromARect(captureArgs.sourceCrop);

    ScreenshotArgs screenshotArgs{.captureTypeVariant = displayWeak,

                                  .displayIdVariant = displayIdVariantOpt,

                                  .sourceCrop = gui::aidl_utils::fromARect(captureArgs.sourceCrop),

                                  .size = reqSize,

                                  .dataspace = static_cast<ui::Dataspace>(captureArgs.dataspace),

                                  .disableBlur = false,

                                  .isGrayscale = captureArgs.grayscale,

                                  .isSecure = captureArgs.captureSecureLayers && displayIsSecure,

                                  .includeProtected = captureArgs.allowProtected,

                                  .seamlessTransition = captureArgs.hintForSeamlessTransition,

                                  .debugName = "ScreenCapture"};



    if (screenshotArgs.sourceCrop.isEmpty()) {

        screenshotArgs.sourceCrop = layerStackSpaceRect;

    }

    screenshotArgs.reqSize = reqSize;

    screenshotArgs.dataspace = static_cast<ui::Dataspace>(captureArgs.dataspace);

    screenshotArgs.isSecure = captureArgs.captureSecureLayers && displayIsSecure;

    screenshotArgs.seamlessTransition = captureArgs.hintForSeamlessTransition;



    captureScreenCommon(screenshotArgs, getLayerSnapshotsFn, reqSize,

                        static_cast<ui::PixelFormat>(captureArgs.pixelFormat),

                        captureArgs.allowProtected, captureArgs.grayscale, captureListener);

    captureScreenCommon(screenshotArgs, getLayerSnapshotsFn,

                        static_cast<ui::PixelFormat>(captureArgs.pixelFormat), captureListener);

}



void SurfaceFlinger::captureDisplay(DisplayId displayId, const CaptureArgs& args,
@@ -7548,21 +7551,20 @@ void SurfaceFlinger::captureDisplay(DisplayId displayId, const CaptureArgs& args 
        return;

    }



    constexpr bool kAllowProtected = false;

    constexpr bool kGrayscale = false;



    ScreenshotArgs screenshotArgs;

    screenshotArgs.captureTypeVariant = displayWeak;

    screenshotArgs.displayIdVariant = displayIdVariantOpt;

    screenshotArgs.sourceCrop = layerStackSpaceRect;

    screenshotArgs.reqSize = size;

    screenshotArgs.dataspace = static_cast<ui::Dataspace>(args.dataspace);

    screenshotArgs.isSecure = args.captureSecureLayers && displayIsSecure;

    screenshotArgs.seamlessTransition = args.hintForSeamlessTransition;

    ScreenshotArgs screenshotArgs{.captureTypeVariant = displayWeak,

                                  .displayIdVariant = displayIdVariantOpt,

                                  .sourceCrop = layerStackSpaceRect,

                                  .size = size,

                                  .dataspace = static_cast<ui::Dataspace>(args.dataspace),

                                  .disableBlur = false,

                                  .isGrayscale = false,

                                  .isSecure = args.captureSecureLayers && displayIsSecure,

                                  .includeProtected = false,

                                  .seamlessTransition = args.hintForSeamlessTransition,

                                  .debugName = "ScreenCapture"};



    captureScreenCommon(screenshotArgs, getLayerSnapshotsFn, size,

                        static_cast<ui::PixelFormat>(args.pixelFormat), kAllowProtected, kGrayscale,

                        captureListener);

    captureScreenCommon(screenshotArgs, getLayerSnapshotsFn,

                        static_cast<ui::PixelFormat>(args.pixelFormat), captureListener);

}



ScreenCaptureResults SurfaceFlinger::captureLayersSync(const LayerCaptureArgs& args) {
@@ -7664,18 +7666,20 @@ void SurfaceFlinger::captureLayers(const LayerCaptureArgs& args, 
        return;

    }



    ScreenshotArgs screenshotArgs;

    screenshotArgs.captureTypeVariant = parent->getSequence();

    screenshotArgs.childrenOnly = args.childrenOnly;

    screenshotArgs.sourceCrop = crop;

    screenshotArgs.reqSize = reqSize;

    screenshotArgs.dataspace = static_cast<ui::Dataspace>(captureArgs.dataspace);

    screenshotArgs.isSecure = captureArgs.captureSecureLayers;

    screenshotArgs.seamlessTransition = captureArgs.hintForSeamlessTransition;

    ScreenshotArgs screenshotArgs{.captureTypeVariant = parent->getSequence(),

                                  .childrenOnly = args.childrenOnly,

                                  .sourceCrop = crop,

                                  .size = reqSize,

                                  .dataspace = static_cast<ui::Dataspace>(captureArgs.dataspace),

                                  .disableBlur = false,

                                  .isGrayscale = captureArgs.grayscale,

                                  .isSecure = captureArgs.captureSecureLayers,

                                  .includeProtected = captureArgs.allowProtected,

                                  .seamlessTransition = captureArgs.hintForSeamlessTransition,

                                  .debugName = "ScreenCapture"};



    captureScreenCommon(screenshotArgs, getLayerSnapshotsFn, reqSize,

                        static_cast<ui::PixelFormat>(captureArgs.pixelFormat),

                        captureArgs.allowProtected, captureArgs.grayscale, captureListener);

    captureScreenCommon(screenshotArgs, getLayerSnapshotsFn,

                        static_cast<ui::PixelFormat>(captureArgs.pixelFormat), captureListener);

}



// Creates a Future release fence for a layer and keeps track of it in a list to
@@ -7734,15 +7738,14 @@ bool SurfaceFlinger::getSnapshotsFromMainThread( 


void SurfaceFlinger::captureScreenCommon(ScreenshotArgs& args,

                                         GetLayerSnapshotsFunction getLayerSnapshotsFn,

                                         ui::Size bufferSize, ui::PixelFormat reqPixelFormat,

                                         bool allowProtected, bool grayscale,

                                         ui::PixelFormat reqPixelFormat,

                                         const sp<IScreenCaptureListener>& captureListener) {

    SFTRACE_CALL();



    if (exceedsMaxRenderTargetSize(bufferSize.getWidth(), bufferSize.getHeight())) {

    if (exceedsMaxRenderTargetSize(args.size.getWidth(), args.size.getHeight())) {

        ALOGE("Attempted to capture screen with size (%" PRId32 ", %" PRId32

              ") that exceeds render target size limit.",

              bufferSize.getWidth(), bufferSize.getHeight());

              args.size.getWidth(), args.size.getHeight());

        invokeScreenCaptureError(BAD_VALUE, captureListener);

        return;

    }
@@ -7760,16 +7763,16 @@ void SurfaceFlinger::captureScreenCommon(ScreenshotArgs& args, 


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

    bool hasProtectedLayer = false;

    if (allowProtected && supportsProtected) {

    if (args.includeProtected && supportsProtected) {

        hasProtectedLayer = layersHasProtectedLayer(layers);

    }

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

    const bool isProtected = hasProtectedLayer && args.includeProtected && supportsProtected;

    const uint32_t usage = GRALLOC_USAGE_HW_COMPOSER | GRALLOC_USAGE_HW_RENDER |

            GRALLOC_USAGE_HW_TEXTURE |

            (isProtected ? GRALLOC_USAGE_PROTECTED

                         : GRALLOC_USAGE_SW_READ_OFTEN | GRALLOC_USAGE_SW_WRITE_OFTEN);

    sp<GraphicBuffer> buffer =

            getFactory().createGraphicBuffer(bufferSize.getWidth(), bufferSize.getHeight(),

            getFactory().createGraphicBuffer(args.size.getWidth(), args.size.getHeight(),

                                             static_cast<android_pixel_format>(reqPixelFormat),

                                             1 /* layerCount */, usage, "screenshot");
@@ -7825,8 +7828,7 @@ void SurfaceFlinger::captureScreenCommon(ScreenshotArgs& args, 
    }



    auto futureFence =

            captureScreenshot(args, texture, false /* regionSampling */, grayscale, isProtected,

                              captureListener, layers, hdrTexture, gainmapTexture);

            captureScreenshot(args, texture, captureListener, layers, hdrTexture, gainmapTexture);

    futureFence.get();

}
@@ -7854,6 +7856,7 @@ bool SurfaceFlinger::getDisplayStateOnMainThread(ScreenshotArgs& args) { 
                        [layerStack = snapshot->outputFilter.layerStack](const auto& display) {

                            return display.getLayerStack() == layerStack;

                        });

                args.debugName.append(", ").append(snapshot->debugName);

            }



            // Screenshot initiated through captureDisplay
@@ -7872,6 +7875,8 @@ bool SurfaceFlinger::getDisplayStateOnMainThread(ScreenshotArgs& args) { 
            args.sdrWhitePointNits = state.sdrWhitePointNits;

            args.renderIntent = state.renderIntent;

            args.colorMode = state.colorMode;

            args.debugName.append(", ").append(display->getDisplayName());

            args.debugName.append(" (").append(to_string(display->getId())).append(")");

            return true;

        }

    }
@@ -7880,7 +7885,6 @@ bool SurfaceFlinger::getDisplayStateOnMainThread(ScreenshotArgs& args) { 


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

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

        bool regionSampling, bool grayscale, bool isProtected,

        const sp<IScreenCaptureListener>& captureListener,

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

        const std::shared_ptr<renderengine::ExternalTexture>& hdrBuffer,
@@ -7894,8 +7898,7 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::captureScreenshot( 


    if (hdrBuffer && gainmapBuffer) {

        ftl::SharedFuture<FenceResult> hdrRenderFuture =

                renderScreenImpl(args, hdrBuffer, regionSampling, grayscale, isProtected,

                                 captureResults, layers);

                renderScreenImpl(args, hdrBuffer, captureResults, layers);

        captureResults.buffer = buffer->getBuffer();

        captureResults.optionalGainMap = gainmapBuffer->getBuffer();
@@ -7926,8 +7929,7 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::captureScreenshot( 
                        })

                        .share();

    } else {

        renderFuture = renderScreenImpl(args, buffer, regionSampling, grayscale, isProtected,

                                        captureResults, layers);

        renderFuture = renderScreenImpl(args, buffer, captureResults, layers);

    }



    if (captureListener) {
@@ -7947,7 +7949,7 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::captureScreenshot( 


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

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

        bool regionSampling, bool grayscale, bool isProtected, ScreenCaptureResults& captureResults,

        ScreenCaptureResults& captureResults,

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

    SFTRACE_CALL();
@@ -8002,7 +8004,7 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::renderScreenImpl( 
    }



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

                    grayscale, isProtected, layers, layerStack, regionSampling, args, renderIntent,

                    layers, layerStack, args, renderIntent,

                    enableLocalTonemapping]() -> FenceResult {

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

                mFactory.createCompositionEngine();
@@ -8056,20 +8058,20 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::renderScreenImpl( 
                                        .sourceCrop = args.sourceCrop,

                                        .buffer = std::move(buffer),

                                        .displayIdVariant = args.displayIdVariant,

                                        .reqBufferSize = args.reqSize,

                                        .reqBufferSize = args.size,

                                        .sdrWhitePointNits = args.sdrWhitePointNits,

                                        .displayBrightnessNits = args.displayBrightnessNits,

                                        .targetBrightness = targetBrightness,

                                        .layerAlpha = layerAlpha,

                                        .regionSampling = regionSampling,

                                        .disableBlur = args.disableBlur,

                                        .treat170mAsSrgb = mTreat170mAsSrgb,

                                        .dimInGammaSpaceForEnhancedScreenshots =

                                                dimInGammaSpaceForEnhancedScreenshots,

                                        .isSecure = args.isSecure,

                                        .isProtected = isProtected,

                                        .enableLocalTonemapping = enableLocalTonemapping});

                                        .enableLocalTonemapping = enableLocalTonemapping,

                                        .debugName = args.debugName});



        const float colorSaturation = grayscale ? 0 : 1;

        const float colorSaturation = args.isGrayscale ? 0 : 1;

        compositionengine::CompositionRefreshArgs refreshArgs{

                .outputs = {output},

                .layers = std::move(layerFEs),

services/surfaceflinger/SurfaceFlinger.h

+15 −6
Original line number Diff line number Diff line
@@ -902,15 +902,25 @@ private: 
        ui::Transform transform;



        // Size of the physical render area

        ui::Size reqSize;

        ui::Size size;



        // Composition dataspace of the render area

        ui::Dataspace dataspace;



        // Whether blur should be disabled, such as for region sampling

        bool disableBlur{false};



        // If true, screenshot is captured in grayscale

        bool isGrayscale{false};



        // If false, the secure layer is blacked out or skipped

        // when rendered to an insecure render area

        bool isSecure{false};



        // If true, includes protected layers which, if screenshotted, results

        // in a protected output buffer

        bool includeProtected{false};



        // If true, the render result may be used for system animations

        // that must preserve the exact colors of the display

        bool seamlessTransition{false};
@@ -926,21 +936,21 @@ private: 


        // Current active render intent of the output composition state

        ui::RenderIntent renderIntent{ui::RenderIntent::COLORIMETRIC};



        std::string debugName;

    };



    bool getSnapshotsFromMainThread(ScreenshotArgs& args,

                                    GetLayerSnapshotsFunction getLayerSnapshotsFn,

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



    void captureScreenCommon(ScreenshotArgs& args, GetLayerSnapshotsFunction, ui::Size bufferSize,

                             ui::PixelFormat, bool allowProtected, bool grayscale,

    void captureScreenCommon(ScreenshotArgs& args, GetLayerSnapshotsFunction, ui::PixelFormat,

                             const sp<IScreenCaptureListener>&);



    bool getDisplayStateOnMainThread(ScreenshotArgs& args) REQUIRES(kMainThreadContext);



    ftl::SharedFuture<FenceResult> captureScreenshot(

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

            bool regionSampling, bool grayscale, bool isProtected,

            const sp<IScreenCaptureListener>& captureListener,

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

            const std::shared_ptr<renderengine::ExternalTexture>& hdrBuffer = nullptr,
@@ -948,8 +958,7 @@ private: 


    ftl::SharedFuture<FenceResult> renderScreenImpl(

            ScreenshotArgs& args, const std::shared_ptr<renderengine::ExternalTexture>&,

            bool regionSampling, bool grayscale, bool isProtected, ScreenCaptureResults&,

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

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



    void readPersistentProperties();