From 82d524e49b4f0acd9ad1a85599500df84fcfc0d2 Mon Sep 17 00:00:00 2001 From: Melody Hsu Date: Fri, 23 Feb 2024 02:37:38 +0000 Subject: Reland refactor of screenshot code on main thread. Create helper functions to improve readability of what is scheduled on the SurfaceFlinger main thread. This will allow for cleaner changes in reducing the calls on the main thread for screenshots. Changes include some renaming for better clarity. Bug: b/294936197 Test: presubmit Test: atest SurfaceFlinger_test Change-Id: I3643b27b98e20578c51f90f6ab61d1aa2e3458bb --- services/surfaceflinger/RegionSamplingThread.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'services/surfaceflinger/RegionSamplingThread.cpp') diff --git a/services/surfaceflinger/RegionSamplingThread.cpp b/services/surfaceflinger/RegionSamplingThread.cpp index c888ccc8ae..77e045d6f9 100644 --- a/services/surfaceflinger/RegionSamplingThread.cpp +++ b/services/surfaceflinger/RegionSamplingThread.cpp @@ -377,8 +377,8 @@ void RegionSamplingThread::captureSample() { constexpr bool kIsProtected = false; if (const auto fenceResult = - mFlinger.captureScreenCommon(std::move(renderAreaFuture), getLayerSnapshots, buffer, - kRegionSampling, kGrayscale, kIsProtected, nullptr) + mFlinger.captureScreenshot(std::move(renderAreaFuture), getLayerSnapshots, buffer, + kRegionSampling, kGrayscale, kIsProtected, nullptr) .get(); fenceResult.ok()) { fenceResult.value()->waitForever(LOG_TAG); -- cgit v1.2.3-59-g8ed1b From 41ade202bb683797beaa64e3459a69952ce1f0bc Mon Sep 17 00:00:00 2001 From: Melody Hsu Date: Fri, 3 May 2024 01:25:50 +0000 Subject: Wrap RenderArea creation in a builder pattern Use a builder to pass around parameters used for RenderArea creation. This allows more flexibility for when the RenderArea is created, which aids in the overall goal of reducing the number of SF main thread hops during screenshots. Creating a builder will allow the render area to later be passed into captureScreenCommon() without being wrapped in a future. Bug: b/294936197 Test: atest SurfaceFlinger_test Change-Id: I9545e02af42c7e6cd9b0c328e2ecce995811f2d7 --- services/surfaceflinger/RegionSamplingThread.cpp | 8 +- services/surfaceflinger/RenderAreaBuilder.h | 114 +++++++++++++++++++++++ services/surfaceflinger/SurfaceFlinger.cpp | 47 +++++----- 3 files changed, 144 insertions(+), 25 deletions(-) create mode 100644 services/surfaceflinger/RenderAreaBuilder.h (limited to 'services/surfaceflinger/RegionSamplingThread.cpp') diff --git a/services/surfaceflinger/RegionSamplingThread.cpp b/services/surfaceflinger/RegionSamplingThread.cpp index 77e045d6f9..2ec20ad5c2 100644 --- a/services/surfaceflinger/RegionSamplingThread.cpp +++ b/services/surfaceflinger/RegionSamplingThread.cpp @@ -42,6 +42,7 @@ #include "DisplayRenderArea.h" #include "FrontEnd/LayerCreationArgs.h" #include "Layer.h" +#include "RenderAreaBuilder.h" #include "Scheduler/VsyncController.h" #include "SurfaceFlinger.h" @@ -279,8 +280,11 @@ void RegionSamplingThread::captureSample() { constexpr bool kHintForSeamlessTransition = false; SurfaceFlinger::RenderAreaFuture renderAreaFuture = ftl::defer([=] { - return DisplayRenderArea::create(displayWeak, sampledBounds, sampledBounds.getSize(), - ui::Dataspace::V0_SRGB, kHintForSeamlessTransition); + DisplayRenderAreaBuilder displayRenderArea(sampledBounds, sampledBounds.getSize(), + ui::Dataspace::V0_SRGB, + kHintForSeamlessTransition, + true /* captureSecureLayers */, displayWeak); + return displayRenderArea.build(); }); std::unordered_set, SpHash> listeners; diff --git a/services/surfaceflinger/RenderAreaBuilder.h b/services/surfaceflinger/RenderAreaBuilder.h new file mode 100644 index 0000000000..012acd2a67 --- /dev/null +++ b/services/surfaceflinger/RenderAreaBuilder.h @@ -0,0 +1,114 @@ +/* + * Copyright 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include "DisplayDevice.h" +#include "DisplayRenderArea.h" +#include "LayerRenderArea.h" +#include "ui/Size.h" +#include "ui/Transform.h" + +namespace android { +/** + * A parameter object for creating a render area + */ +struct RenderAreaBuilder { + // Source crop of the render area + Rect crop; + + // Size of the physical render area + ui::Size reqSize; + + // Composition data space of the render area + ui::Dataspace reqDataSpace; + + // If true, the secure layer would be blacked out or skipped + // when rendered to an insecure render area + bool allowSecureLayers; + + // If true, the render result may be used for system animations + // that must preserve the exact colors of the display + bool hintForSeamlessTransition; + + virtual std::unique_ptr build() const = 0; + + RenderAreaBuilder(Rect crop, ui::Size reqSize, ui::Dataspace reqDataSpace, + bool allowSecureLayers, bool hintForSeamlessTransition) + : crop(crop), + reqSize(reqSize), + reqDataSpace(reqDataSpace), + allowSecureLayers(allowSecureLayers), + hintForSeamlessTransition(hintForSeamlessTransition) {} + + virtual ~RenderAreaBuilder() = default; +}; + +struct DisplayRenderAreaBuilder : RenderAreaBuilder { + DisplayRenderAreaBuilder(Rect crop, ui::Size reqSize, ui::Dataspace reqDataSpace, + bool allowSecureLayers, bool hintForSeamlessTransition, + wp displayWeak) + : RenderAreaBuilder(crop, reqSize, reqDataSpace, allowSecureLayers, + hintForSeamlessTransition), + displayWeak(displayWeak) {} + + // Display that render area will be on + wp displayWeak; + + std::unique_ptr build() const override { + return DisplayRenderArea::create(displayWeak, crop, reqSize, reqDataSpace, + hintForSeamlessTransition, allowSecureLayers); + } +}; + +struct LayerRenderAreaBuilder : RenderAreaBuilder { + LayerRenderAreaBuilder(Rect crop, ui::Size reqSize, ui::Dataspace reqDataSpace, + bool allowSecureLayers, bool hintForSeamlessTransition, sp layer, + bool childrenOnly) + : RenderAreaBuilder(crop, reqSize, reqDataSpace, allowSecureLayers, + hintForSeamlessTransition), + layer(layer), + childrenOnly(childrenOnly) {} + + // Layer that the render area will be on + sp layer; + + // Transform to be applied on the layers to transform them + // into the logical render area + ui::Transform layerTransform{ui::Transform()}; + + // Buffer bounds + Rect layerBufferSize{Rect()}; + + // If false, transform is inverted from the parent snapshot + bool childrenOnly; + + // Uses parent snapshot to determine layer transform and buffer size + void setLayerInfo(const frontend::LayerSnapshot* parentSnapshot) { + if (!childrenOnly) { + layerTransform = parentSnapshot->localTransform.inverse(); + } + layerBufferSize = parentSnapshot->bufferSize; + } + + std::unique_ptr build() const override { + return std::make_unique(layer, crop, reqSize, reqDataSpace, + allowSecureLayers, layerTransform, layerBufferSize, + hintForSeamlessTransition); + } +}; + +} // namespace android \ No newline at end of file diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index df226c94ff..5263aa80b9 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -148,6 +148,7 @@ #include "MutexUtils.h" #include "NativeWindowSurface.h" #include "RegionSamplingThread.h" +#include "RenderAreaBuilder.h" #include "Scheduler/EventThread.h" #include "Scheduler/LayerHistory.h" #include "Scheduler/Scheduler.h" @@ -7918,8 +7919,10 @@ void SurfaceFlinger::captureDisplay(const DisplayCaptureArgs& args, } RenderAreaFuture renderAreaFuture = ftl::defer([=] { - return DisplayRenderArea::create(displayWeak, args.sourceCrop, reqSize, args.dataspace, - args.hintForSeamlessTransition, args.captureSecureLayers); + DisplayRenderAreaBuilder displayRenderArea(args.sourceCrop, reqSize, args.dataspace, + args.hintForSeamlessTransition, + args.captureSecureLayers, displayWeak); + return displayRenderArea.build(); }); GetLayerSnapshotsFunction getLayerSnapshots; @@ -7972,9 +7975,10 @@ void SurfaceFlinger::captureDisplay(DisplayId displayId, const CaptureArgs& args } RenderAreaFuture renderAreaFuture = ftl::defer([=] { - return DisplayRenderArea::create(displayWeak, Rect(), size, args.dataspace, - args.hintForSeamlessTransition, - false /* captureSecureLayers */); + DisplayRenderAreaBuilder displayRenderArea(Rect(), size, args.dataspace, + args.hintForSeamlessTransition, + false /* captureSecureLayers */, displayWeak); + return displayRenderArea.build(); }); GetLayerSnapshotsFunction getLayerSnapshots; @@ -8079,25 +8083,22 @@ void SurfaceFlinger::captureLayers(const LayerCaptureArgs& args, return; } - RenderAreaFuture renderAreaFuture = ftl::defer([=, this]() FTL_FAKE_GUARD(kMainThreadContext) - -> std::unique_ptr { - ui::Transform layerTransform; - Rect layerBufferSize; - frontend::LayerSnapshot* snapshot = - mLayerSnapshotBuilder.getSnapshot(parent->getSequence()); - if (!snapshot) { - ALOGW("Couldn't find layer snapshot for %d", parent->getSequence()); - } else { - if (!args.childrenOnly) { - layerTransform = snapshot->localTransform.inverse(); - } - layerBufferSize = snapshot->bufferSize; - } + RenderAreaFuture renderAreaFuture = ftl::defer( + [=, this]() FTL_FAKE_GUARD(kMainThreadContext) -> std::unique_ptr { + LayerRenderAreaBuilder layerRenderArea(crop, reqSize, dataspace, + args.captureSecureLayers, + args.hintForSeamlessTransition, parent, + args.childrenOnly); - return std::make_unique(parent, crop, reqSize, dataspace, - args.captureSecureLayers, layerTransform, - layerBufferSize, args.hintForSeamlessTransition); - }); + frontend::LayerSnapshot* snapshot = + mLayerSnapshotBuilder.getSnapshot(parent->getSequence()); + if (!snapshot) { + ALOGW("Couldn't find layer snapshot for %d", parent->getSequence()); + } else { + layerRenderArea.setLayerInfo(snapshot); + } + return layerRenderArea.build(); + }); GetLayerSnapshotsFunction getLayerSnapshots; if (mLayerLifecycleManagerEnabled) { std::optional parentCrop = std::nullopt; -- cgit v1.2.3-59-g8ed1b From 2ffa36b70412a84873e67a6d31123bafdafc9b78 Mon Sep 17 00:00:00 2001 From: Melody Hsu Date: Wed, 8 May 2024 03:51:46 +0000 Subject: Refactor RenderAreaFuture to use RenderAreaBuilder Cleans up renderArea logic and allows all the work that needs to be done on the main thread in the same place. This will aid in the effort to reduce the number of hops to the SF main thread during screenshots. Bug: b/294936197 Test: atest SurfaceFlinger_test Test: presubmit Change-Id: I4234b49638aaecceb8d1fcff7f5cd43698b6c47f --- services/surfaceflinger/RegionSamplingThread.cpp | 18 ++-- services/surfaceflinger/SurfaceFlinger.cpp | 95 +++++++++++----------- services/surfaceflinger/SurfaceFlinger.h | 17 ++-- .../tests/unittests/TestableSurfaceFlinger.h | 2 +- 4 files changed, 65 insertions(+), 67 deletions(-) (limited to 'services/surfaceflinger/RegionSamplingThread.cpp') diff --git a/services/surfaceflinger/RegionSamplingThread.cpp b/services/surfaceflinger/RegionSamplingThread.cpp index 2ec20ad5c2..2b4e234604 100644 --- a/services/surfaceflinger/RegionSamplingThread.cpp +++ b/services/surfaceflinger/RegionSamplingThread.cpp @@ -279,14 +279,6 @@ void RegionSamplingThread::captureSample() { const Rect sampledBounds = sampleRegion.bounds(); constexpr bool kHintForSeamlessTransition = false; - SurfaceFlinger::RenderAreaFuture renderAreaFuture = ftl::defer([=] { - DisplayRenderAreaBuilder displayRenderArea(sampledBounds, sampledBounds.getSize(), - ui::Dataspace::V0_SRGB, - kHintForSeamlessTransition, - true /* captureSecureLayers */, displayWeak); - return displayRenderArea.build(); - }); - std::unordered_set, SpHash> listeners; auto layerFilterFn = [&](const char* layerName, uint32_t layerId, const Rect& bounds, @@ -381,8 +373,14 @@ void RegionSamplingThread::captureSample() { constexpr bool kIsProtected = false; if (const auto fenceResult = - mFlinger.captureScreenshot(std::move(renderAreaFuture), getLayerSnapshots, buffer, - kRegionSampling, kGrayscale, kIsProtected, nullptr) + mFlinger.captureScreenshot(SurfaceFlinger::RenderAreaBuilderVariant( + std::in_place_type, + sampledBounds, sampledBounds.getSize(), + ui::Dataspace::V0_SRGB, + kHintForSeamlessTransition, + true /* captureSecureLayers */, displayWeak), + getLayerSnapshots, buffer, kRegionSampling, kGrayscale, + kIsProtected, nullptr) .get(); fenceResult.ok()) { fenceResult.value()->waitForever(LOG_TAG); diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 64d80d9bab..0369549d01 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -7892,13 +7892,6 @@ void SurfaceFlinger::captureDisplay(const DisplayCaptureArgs& args, } } - RenderAreaFuture renderAreaFuture = ftl::defer([=] { - DisplayRenderAreaBuilder displayRenderArea(args.sourceCrop, reqSize, args.dataspace, - args.hintForSeamlessTransition, - args.captureSecureLayers, displayWeak); - return displayRenderArea.build(); - }); - GetLayerSnapshotsFunction getLayerSnapshots; if (mLayerLifecycleManagerEnabled) { getLayerSnapshots = @@ -7911,8 +7904,12 @@ void SurfaceFlinger::captureDisplay(const DisplayCaptureArgs& args, getLayerSnapshots = RenderArea::fromTraverseLayersLambda(traverseLayers); } - captureScreenCommon(std::move(renderAreaFuture), getLayerSnapshots, reqSize, args.pixelFormat, - args.allowProtected, args.grayscale, captureListener); + captureScreenCommon(RenderAreaBuilderVariant(std::in_place_type, + args.sourceCrop, reqSize, args.dataspace, + args.hintForSeamlessTransition, + args.captureSecureLayers, displayWeak), + getLayerSnapshots, reqSize, args.pixelFormat, args.allowProtected, + args.grayscale, captureListener); } void SurfaceFlinger::captureDisplay(DisplayId displayId, const CaptureArgs& args, @@ -7948,13 +7945,6 @@ void SurfaceFlinger::captureDisplay(DisplayId displayId, const CaptureArgs& args return; } - RenderAreaFuture renderAreaFuture = ftl::defer([=] { - DisplayRenderAreaBuilder displayRenderArea(Rect(), size, args.dataspace, - args.hintForSeamlessTransition, - false /* captureSecureLayers */, displayWeak); - return displayRenderArea.build(); - }); - GetLayerSnapshotsFunction getLayerSnapshots; if (mLayerLifecycleManagerEnabled) { getLayerSnapshots = getLayerSnapshotsForScreenshots(layerStack, CaptureArgs::UNSET_UID, @@ -7975,8 +7965,12 @@ void SurfaceFlinger::captureDisplay(DisplayId displayId, const CaptureArgs& args constexpr bool kAllowProtected = false; constexpr bool kGrayscale = false; - captureScreenCommon(std::move(renderAreaFuture), getLayerSnapshots, size, args.pixelFormat, - kAllowProtected, kGrayscale, captureListener); + captureScreenCommon(RenderAreaBuilderVariant(std::in_place_type, + Rect(), size, args.dataspace, + args.hintForSeamlessTransition, + false /* captureSecureLayers */, displayWeak), + getLayerSnapshots, size, args.pixelFormat, kAllowProtected, kGrayscale, + captureListener); } ScreenCaptureResults SurfaceFlinger::captureLayersSync(const LayerCaptureArgs& args) { @@ -8057,22 +8051,6 @@ void SurfaceFlinger::captureLayers(const LayerCaptureArgs& args, return; } - RenderAreaFuture renderAreaFuture = ftl::defer( - [=, this]() FTL_FAKE_GUARD(kMainThreadContext) -> std::unique_ptr { - LayerRenderAreaBuilder layerRenderArea(crop, reqSize, dataspace, - args.captureSecureLayers, - args.hintForSeamlessTransition, parent, - args.childrenOnly); - - frontend::LayerSnapshot* snapshot = - mLayerSnapshotBuilder.getSnapshot(parent->getSequence()); - if (!snapshot) { - ALOGW("Couldn't find layer snapshot for %d", parent->getSequence()); - } else { - layerRenderArea.setLayerInfo(snapshot); - } - return layerRenderArea.build(); - }); GetLayerSnapshotsFunction getLayerSnapshots; if (mLayerLifecycleManagerEnabled) { std::optional parentCrop = std::nullopt; @@ -8115,8 +8093,12 @@ void SurfaceFlinger::captureLayers(const LayerCaptureArgs& args, return; } - captureScreenCommon(std::move(renderAreaFuture), getLayerSnapshots, reqSize, args.pixelFormat, - args.allowProtected, args.grayscale, captureListener); + captureScreenCommon(RenderAreaBuilderVariant(std::in_place_type, crop, + reqSize, dataspace, args.captureSecureLayers, + args.hintForSeamlessTransition, parent, + args.childrenOnly), + getLayerSnapshots, reqSize, args.pixelFormat, args.allowProtected, + args.grayscale, captureListener); } // Creates a Future release fence for a layer and keeps track of it in a list to @@ -8143,7 +8125,7 @@ bool SurfaceFlinger::layersHasProtectedLayer( return protectedLayerFound; } -void SurfaceFlinger::captureScreenCommon(RenderAreaFuture renderAreaFuture, +void SurfaceFlinger::captureScreenCommon(RenderAreaBuilderVariant renderAreaBuilder, GetLayerSnapshotsFunction getLayerSnapshots, ui::Size bufferSize, ui::PixelFormat reqPixelFormat, bool allowProtected, bool grayscale, @@ -8192,21 +8174,35 @@ void SurfaceFlinger::captureScreenCommon(RenderAreaFuture renderAreaFuture, renderengine::impl::ExternalTexture::Usage:: WRITEABLE); auto futureFence = - captureScreenshot(std::move(renderAreaFuture), getLayerSnapshots, texture, + captureScreenshot(renderAreaBuilder, getLayerSnapshots, texture, false /* regionSampling */, grayscale, isProtected, captureListener); futureFence.get(); } ftl::SharedFuture SurfaceFlinger::captureScreenshot( - RenderAreaFuture renderAreaFuture, GetLayerSnapshotsFunction getLayerSnapshots, + RenderAreaBuilderVariant renderAreaBuilder, GetLayerSnapshotsFunction getLayerSnapshots, const std::shared_ptr& buffer, bool regionSampling, bool grayscale, bool isProtected, const sp& captureListener) { ATRACE_CALL(); - auto takeScreenshotFn = [=, this, renderAreaFuture = std::move(renderAreaFuture)]() REQUIRES( + auto takeScreenshotFn = [=, this, renderAreaBuilder = std::move(renderAreaBuilder)]() REQUIRES( kMainThreadContext) mutable -> ftl::SharedFuture { // LayerSnapshots must be obtained from the main thread. auto layers = getLayerSnapshots(); + + if (auto* layerRenderAreaBuilder = + std::get_if(&renderAreaBuilder)) { + // LayerSnapshotBuilder should only be accessed from the main thread. + frontend::LayerSnapshot* snapshot = + mLayerSnapshotBuilder.getSnapshot(layerRenderAreaBuilder->layer->getSequence()); + if (!snapshot) { + ALOGW("Couldn't find layer snapshot for %d", + layerRenderAreaBuilder->layer->getSequence()); + } else { + layerRenderAreaBuilder->setLayerInfo(snapshot); + } + } + if (FlagManager::getInstance().ce_fence_promise()) { for (auto& [layer, layerFE] : layers) { attachReleaseFenceFutureToLayer(layer, layerFE.get(), ui::INVALID_LAYER_STACK); @@ -8214,7 +8210,10 @@ ftl::SharedFuture SurfaceFlinger::captureScreenshot( } ScreenCaptureResults captureResults; - std::shared_ptr renderArea = renderAreaFuture.get(); + std::unique_ptr renderArea = + std::visit([](auto&& arg) -> std::unique_ptr { return arg.build(); }, + renderAreaBuilder); + if (!renderArea) { ALOGW("Skipping screen capture because of invalid render area."); if (captureListener) { @@ -8225,8 +8224,8 @@ ftl::SharedFuture SurfaceFlinger::captureScreenshot( } ftl::SharedFuture renderFuture = - renderScreenImpl(renderArea, buffer, regionSampling, grayscale, isProtected, - captureResults, layers); + renderScreenImpl(std::move(renderArea), buffer, regionSampling, grayscale, + isProtected, captureResults, layers); if (captureListener) { // Defer blocking on renderFuture back to the Binder thread. @@ -8243,9 +8242,7 @@ ftl::SharedFuture SurfaceFlinger::captureScreenshot( }; // TODO(b/294936197): Run takeScreenshotsFn() in a binder thread to reduce the number - // of calls on the main thread. renderAreaFuture runs on the main thread and should - // no longer be a future, so that it does not need to make an additional jump on the - // main thread whenever get() is called. + // of calls on the main thread. auto future = mScheduler->schedule(FTL_FAKE_GUARD(kMainThreadContext, std::move(takeScreenshotFn))); @@ -8258,16 +8255,16 @@ ftl::SharedFuture SurfaceFlinger::captureScreenshot( } ftl::SharedFuture SurfaceFlinger::renderScreenImpl( - std::shared_ptr renderArea, GetLayerSnapshotsFunction getLayerSnapshots, + std::unique_ptr renderArea, GetLayerSnapshotsFunction getLayerSnapshots, const std::shared_ptr& buffer, bool regionSampling, bool grayscale, bool isProtected, ScreenCaptureResults& captureResults) { auto layers = getLayerSnapshots(); - return renderScreenImpl(renderArea, buffer, regionSampling, grayscale, isProtected, + return renderScreenImpl(std::move(renderArea), buffer, regionSampling, grayscale, isProtected, captureResults, layers); } ftl::SharedFuture SurfaceFlinger::renderScreenImpl( - std::shared_ptr renderArea, + std::unique_ptr renderArea, const std::shared_ptr& buffer, bool regionSampling, bool grayscale, bool isProtected, ScreenCaptureResults& captureResults, std::vector>>& layers) { diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index fc50fd97f8..af8f3150c8 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -192,6 +192,9 @@ enum class LatchUnsignaledConfig { Always, }; +struct DisplayRenderAreaBuilder; +struct LayerRenderAreaBuilder; + using DisplayColorSetting = compositionengine::OutputColorSetting; class SurfaceFlinger : public BnSurfaceComposer, @@ -383,7 +386,7 @@ private: using TransactionSchedule = scheduler::TransactionSchedule; using GetLayerSnapshotsFunction = std::function>>()>; - using RenderAreaFuture = ftl::Future>; + using RenderAreaBuilderVariant = std::variant; using DumpArgs = Vector; using Dumper = std::function; @@ -891,12 +894,12 @@ private: // Checks if a protected layer exists in a list of layers. bool layersHasProtectedLayer(const std::vector>>& layers) const; - void captureScreenCommon(RenderAreaFuture, GetLayerSnapshotsFunction, ui::Size bufferSize, - ui::PixelFormat, bool allowProtected, bool grayscale, - const sp&); + void captureScreenCommon(RenderAreaBuilderVariant, GetLayerSnapshotsFunction, + ui::Size bufferSize, ui::PixelFormat, bool allowProtected, + bool grayscale, const sp&); ftl::SharedFuture captureScreenshot( - RenderAreaFuture, GetLayerSnapshotsFunction, + RenderAreaBuilderVariant, GetLayerSnapshotsFunction, const std::shared_ptr&, bool regionSampling, bool grayscale, bool isProtected, const sp&); @@ -904,13 +907,13 @@ private: // not yet been captured, and thus cannot yet be passed in as a parameter. // Needed for TestableSurfaceFlinger. ftl::SharedFuture renderScreenImpl( - std::shared_ptr, GetLayerSnapshotsFunction, + std::unique_ptr, GetLayerSnapshotsFunction, const std::shared_ptr&, bool regionSampling, bool grayscale, bool isProtected, ScreenCaptureResults&) EXCLUDES(mStateLock) REQUIRES(kMainThreadContext); ftl::SharedFuture renderScreenImpl( - std::shared_ptr, + std::unique_ptr, const std::shared_ptr&, bool regionSampling, bool grayscale, bool isProtected, ScreenCaptureResults&, std::vector>>& layers) EXCLUDES(mStateLock) diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h index 935346712f..d3b6e174cb 100644 --- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h +++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h @@ -475,7 +475,7 @@ public: return mFlinger->setPowerModeInternal(display, mode); } - auto renderScreenImpl(std::shared_ptr renderArea, + auto renderScreenImpl(std::unique_ptr renderArea, SurfaceFlinger::GetLayerSnapshotsFunction traverseLayers, const std::shared_ptr& buffer, bool regionSampling) { -- cgit v1.2.3-59-g8ed1b From eddfe939edcc9b3f4ce73cb6c2de21f588856df8 Mon Sep 17 00:00:00 2001 From: Melody Hsu Date: Tue, 4 Jun 2024 18:10:41 +0000 Subject: Remove legacy layer path in screenshot pathway Legacy path to get layer snapshots is no longer used and the flag can be removed to clean up the code. Also renames layer snapshot function variable for better clarity that the function is not immediately called wherever declared. Bug: b/330785038 Test: presubmit Test: atest SurfaceFlinger_test Change-Id: I12498f2560de07d69fff93eda901f6b7c072fce6 --- services/surfaceflinger/RegionSamplingThread.cpp | 44 +++--------- services/surfaceflinger/SurfaceFlinger.cpp | 80 +++++----------------- .../tests/unittests/CompositionTest.cpp | 5 +- .../tests/unittests/TestableSurfaceFlinger.h | 7 +- 4 files changed, 35 insertions(+), 101 deletions(-) (limited to 'services/surfaceflinger/RegionSamplingThread.cpp') diff --git a/services/surfaceflinger/RegionSamplingThread.cpp b/services/surfaceflinger/RegionSamplingThread.cpp index 2b4e234604..59eb7f5d4a 100644 --- a/services/surfaceflinger/RegionSamplingThread.cpp +++ b/services/surfaceflinger/RegionSamplingThread.cpp @@ -315,39 +315,15 @@ void RegionSamplingThread::captureSample() { return true; }; - std::function>>()> getLayerSnapshots; - if (mFlinger.mLayerLifecycleManagerEnabled) { - auto filterFn = [&](const frontend::LayerSnapshot& snapshot, - bool& outStopTraversal) -> bool { - const Rect bounds = - frontend::RequestedLayerState::reduce(Rect(snapshot.geomLayerBounds), - snapshot.transparentRegionHint); - const ui::Transform transform = snapshot.geomLayerTransform; - return layerFilterFn(snapshot.name.c_str(), snapshot.path.id, bounds, transform, - outStopTraversal); - }; - getLayerSnapshots = - mFlinger.getLayerSnapshotsForScreenshots(layerStack, CaptureArgs::UNSET_UID, - filterFn); - } else { - auto traverseLayers = [&](const LayerVector::Visitor& visitor) { - bool stopLayerFound = false; - auto filterVisitor = [&](Layer* layer) { - // We don't want to capture any layers beyond the stop layer - if (stopLayerFound) return; - - if (!layerFilterFn(layer->getDebugName(), layer->getSequence(), - Rect(layer->getBounds()), layer->getTransform(), - stopLayerFound)) { - return; - } - visitor(layer); - }; - mFlinger.traverseLayersInLayerStack(layerStack, CaptureArgs::UNSET_UID, {}, - filterVisitor); - }; - getLayerSnapshots = RenderArea::fromTraverseLayersLambda(traverseLayers); - } + auto filterFn = [&](const frontend::LayerSnapshot& snapshot, bool& outStopTraversal) -> bool { + const Rect bounds = frontend::RequestedLayerState::reduce(Rect(snapshot.geomLayerBounds), + snapshot.transparentRegionHint); + const ui::Transform transform = snapshot.geomLayerTransform; + return layerFilterFn(snapshot.name.c_str(), snapshot.path.id, bounds, transform, + outStopTraversal); + }; + auto getLayerSnapshotsFn = + mFlinger.getLayerSnapshotsForScreenshots(layerStack, CaptureArgs::UNSET_UID, filterFn); std::shared_ptr buffer = nullptr; if (mCachedBuffer && mCachedBuffer->getBuffer()->getWidth() == sampledBounds.getWidth() && @@ -379,7 +355,7 @@ void RegionSamplingThread::captureSample() { ui::Dataspace::V0_SRGB, kHintForSeamlessTransition, true /* captureSecureLayers */, displayWeak), - getLayerSnapshots, buffer, kRegionSampling, kGrayscale, + getLayerSnapshotsFn, buffer, kRegionSampling, kGrayscale, kIsProtected, nullptr) .get(); fenceResult.ok()) { diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index d9f7804051..f0b390e1b5 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -7933,23 +7933,14 @@ void SurfaceFlinger::captureDisplay(const DisplayCaptureArgs& args, } } - GetLayerSnapshotsFunction getLayerSnapshots; - if (mLayerLifecycleManagerEnabled) { - getLayerSnapshots = - getLayerSnapshotsForScreenshots(layerStack, args.uid, std::move(excludeLayerIds)); - } else { - auto traverseLayers = [this, args, excludeLayerIds, - layerStack](const LayerVector::Visitor& visitor) { - traverseLayersInLayerStack(layerStack, args.uid, std::move(excludeLayerIds), visitor); - }; - getLayerSnapshots = RenderArea::fromTraverseLayersLambda(traverseLayers); - } + GetLayerSnapshotsFunction getLayerSnapshotsFn = + getLayerSnapshotsForScreenshots(layerStack, args.uid, std::move(excludeLayerIds)); captureScreenCommon(RenderAreaBuilderVariant(std::in_place_type, args.sourceCrop, reqSize, args.dataspace, args.hintForSeamlessTransition, args.captureSecureLayers, displayWeak), - getLayerSnapshots, reqSize, args.pixelFormat, args.allowProtected, + getLayerSnapshotsFn, reqSize, args.pixelFormat, args.allowProtected, args.grayscale, captureListener); } @@ -7986,16 +7977,9 @@ void SurfaceFlinger::captureDisplay(DisplayId displayId, const CaptureArgs& args return; } - GetLayerSnapshotsFunction getLayerSnapshots; - if (mLayerLifecycleManagerEnabled) { - getLayerSnapshots = getLayerSnapshotsForScreenshots(layerStack, CaptureArgs::UNSET_UID, - /*snapshotFilterFn=*/nullptr); - } else { - auto traverseLayers = [this, layerStack](const LayerVector::Visitor& visitor) { - traverseLayersInLayerStack(layerStack, CaptureArgs::UNSET_UID, {}, visitor); - }; - getLayerSnapshots = RenderArea::fromTraverseLayersLambda(traverseLayers); - } + GetLayerSnapshotsFunction getLayerSnapshotsFn = + getLayerSnapshotsForScreenshots(layerStack, CaptureArgs::UNSET_UID, + /*snapshotFilterFn=*/nullptr); if (captureListener == nullptr) { ALOGE("capture screen must provide a capture listener callback"); @@ -8010,7 +7994,7 @@ void SurfaceFlinger::captureDisplay(DisplayId displayId, const CaptureArgs& args Rect(), size, args.dataspace, args.hintForSeamlessTransition, false /* captureSecureLayers */, displayWeak), - getLayerSnapshots, size, args.pixelFormat, kAllowProtected, kGrayscale, + getLayerSnapshotsFn, size, args.pixelFormat, kAllowProtected, kGrayscale, captureListener); } @@ -8092,42 +8076,16 @@ void SurfaceFlinger::captureLayers(const LayerCaptureArgs& args, return; } - GetLayerSnapshotsFunction getLayerSnapshots; - if (mLayerLifecycleManagerEnabled) { - std::optional parentCrop = std::nullopt; - if (args.childrenOnly) { - parentCrop = crop.isEmpty() ? FloatRect(0, 0, reqSize.width, reqSize.height) - : crop.toFloatRect(); - } - - getLayerSnapshots = getLayerSnapshotsForScreenshots(parent->sequence, args.uid, - std::move(excludeLayerIds), - args.childrenOnly, parentCrop); - } else { - auto traverseLayers = [parent, args, excludeLayerIds](const LayerVector::Visitor& visitor) { - parent->traverseChildrenInZOrder(LayerVector::StateSet::Drawing, [&](Layer* layer) { - if (!layer->isVisible()) { - return; - } else if (args.childrenOnly && layer == parent.get()) { - return; - } else if (args.uid != CaptureArgs::UNSET_UID && args.uid != layer->getOwnerUid()) { - return; - } - - auto p = sp::fromExisting(layer); - while (p != nullptr) { - if (excludeLayerIds.count(p->sequence) != 0) { - return; - } - p = p->getParent(); - } - - visitor(layer); - }); - }; - getLayerSnapshots = RenderArea::fromTraverseLayersLambda(traverseLayers); + std::optional parentCrop = std::nullopt; + if (args.childrenOnly) { + parentCrop = crop.isEmpty() ? FloatRect(0, 0, reqSize.width, reqSize.height) + : crop.toFloatRect(); } + GetLayerSnapshotsFunction getLayerSnapshotsFn = + getLayerSnapshotsForScreenshots(parent->sequence, args.uid, std::move(excludeLayerIds), + args.childrenOnly, parentCrop); + if (captureListener == nullptr) { ALOGD("capture screen must provide a capture listener callback"); invokeScreenCaptureError(BAD_VALUE, captureListener); @@ -8138,7 +8096,7 @@ void SurfaceFlinger::captureLayers(const LayerCaptureArgs& args, reqSize, dataspace, args.captureSecureLayers, args.hintForSeamlessTransition, parent, args.childrenOnly), - getLayerSnapshots, reqSize, args.pixelFormat, args.allowProtected, + getLayerSnapshotsFn, reqSize, args.pixelFormat, args.allowProtected, args.grayscale, captureListener); } @@ -8167,7 +8125,7 @@ bool SurfaceFlinger::layersHasProtectedLayer( } void SurfaceFlinger::captureScreenCommon(RenderAreaBuilderVariant renderAreaBuilder, - GetLayerSnapshotsFunction getLayerSnapshots, + GetLayerSnapshotsFunction getLayerSnapshotsFn, ui::Size bufferSize, ui::PixelFormat reqPixelFormat, bool allowProtected, bool grayscale, const sp& captureListener) { @@ -8188,7 +8146,7 @@ void SurfaceFlinger::captureScreenCommon(RenderAreaBuilderVariant renderAreaBuil const bool supportsProtected = getRenderEngine().supportsProtectedContent(); bool hasProtectedLayer = false; if (allowProtected && supportsProtected) { - auto layers = mScheduler->schedule([=]() { return getLayerSnapshots(); }).get(); + auto layers = mScheduler->schedule([=]() { return getLayerSnapshotsFn(); }).get(); hasProtectedLayer = layersHasProtectedLayer(layers); } const bool isProtected = hasProtectedLayer && allowProtected && supportsProtected; @@ -8215,7 +8173,7 @@ void SurfaceFlinger::captureScreenCommon(RenderAreaBuilderVariant renderAreaBuil renderengine::impl::ExternalTexture::Usage:: WRITEABLE); auto futureFence = - captureScreenshot(renderAreaBuilder, getLayerSnapshots, texture, + captureScreenshot(renderAreaBuilder, getLayerSnapshotsFn, texture, false /* regionSampling */, grayscale, isProtected, captureListener); futureFence.get(); } diff --git a/services/surfaceflinger/tests/unittests/CompositionTest.cpp b/services/surfaceflinger/tests/unittests/CompositionTest.cpp index 98f1687ac6..08973de195 100644 --- a/services/surfaceflinger/tests/unittests/CompositionTest.cpp +++ b/services/surfaceflinger/tests/unittests/CompositionTest.cpp @@ -205,7 +205,8 @@ void CompositionTest::captureScreenComposition() { CaptureArgs::UNSET_UID, {}, visitor); }; - auto getLayerSnapshots = RenderArea::fromTraverseLayersLambda(traverseLayers); + // TODO: Use SurfaceFlinger::getLayerSnapshotsForScreenshots instead of this legacy function + auto getLayerSnapshotsFn = RenderArea::fromTraverseLayersLambda(traverseLayers); const uint32_t usage = GRALLOC_USAGE_SW_READ_OFTEN | GRALLOC_USAGE_SW_WRITE_OFTEN | GRALLOC_USAGE_HW_RENDER | GRALLOC_USAGE_HW_TEXTURE; @@ -215,7 +216,7 @@ void CompositionTest::captureScreenComposition() { HAL_PIXEL_FORMAT_RGBA_8888, 1, usage); - auto future = mFlinger.renderScreenImpl(mDisplay, std::move(renderArea), getLayerSnapshots, + auto future = mFlinger.renderScreenImpl(mDisplay, std::move(renderArea), getLayerSnapshotsFn, mCaptureScreenBuffer, regionSampling); ASSERT_TRUE(future.valid()); const auto fenceResult = future.get(); diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h index 1783e179e2..7889096fe2 100644 --- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h +++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h @@ -487,7 +487,7 @@ public: auto renderScreenImpl(const sp display, std::unique_ptr renderArea, - SurfaceFlinger::GetLayerSnapshotsFunction traverseLayers, + SurfaceFlinger::GetLayerSnapshotsFunction getLayerSnapshotsFn, const std::shared_ptr& buffer, bool regionSampling) { Mutex::Autolock lock(mFlinger->mStateLock); @@ -495,7 +495,7 @@ public: ScreenCaptureResults captureResults; SurfaceFlinger::OutputCompositionState state = display->getCompositionDisplay()->getState(); - auto layers = mFlinger->getLayerSnapshotsFromMainThread(traverseLayers); + auto layers = mFlinger->getLayerSnapshotsFromMainThread(getLayerSnapshotsFn); return mFlinger->renderScreenImpl(std::move(renderArea), buffer, regionSampling, false /* grayscale */, false /* isProtected */, @@ -505,8 +505,7 @@ public: auto traverseLayersInLayerStack(ui::LayerStack layerStack, int32_t uid, std::unordered_set excludeLayerIds, const LayerVector::Visitor& visitor) { - return mFlinger->SurfaceFlinger::traverseLayersInLayerStack(layerStack, uid, - excludeLayerIds, visitor); + return mFlinger->traverseLayersInLayerStack(layerStack, uid, excludeLayerIds, visitor); } auto getDisplayNativePrimaries(const sp& displayToken, -- cgit v1.2.3-59-g8ed1b From 8c42cf19d10c9a685e8d6a88c9057435dd738a6b Mon Sep 17 00:00:00 2001 From: Melody Hsu Date: Wed, 5 Jun 2024 00:07:03 +0000 Subject: Remove main thread double hop from screenshots The SF main thread is accessed twice during screenshots, which leads to possible inconsistent states between jumps onto the main thread. Removing the double hop preserves correctness and reduces the amount of computation taking place on the main thread. The only time the main thread should be accessed should be when getting layer snapshots. All other work related to screenshots can take place in a binder thread. Bug: b/294936197, b/285553970 Test: atest SurfaceFlinger_test Flag: com.android.graphics.surfaceflinger.flags.single_hop_screenshot Change-Id: If9fd36f82c2d550bc0821b52fef3ea88b8099116 --- services/surfaceflinger/RegionSamplingThread.cpp | 33 ++- services/surfaceflinger/SurfaceFlinger.cpp | 256 +++++++++++++++------ services/surfaceflinger/SurfaceFlinger.h | 35 ++- services/surfaceflinger/common/FlagManager.cpp | 2 + .../common/include/common/FlagManager.h | 1 + .../surfaceflinger_flags_new.aconfig | 11 + .../tests/unittests/TestableSurfaceFlinger.h | 7 +- 7 files changed, 250 insertions(+), 95 deletions(-) (limited to 'services/surfaceflinger/RegionSamplingThread.cpp') diff --git a/services/surfaceflinger/RegionSamplingThread.cpp b/services/surfaceflinger/RegionSamplingThread.cpp index 59eb7f5d4a..5add290e96 100644 --- a/services/surfaceflinger/RegionSamplingThread.cpp +++ b/services/surfaceflinger/RegionSamplingThread.cpp @@ -348,17 +348,30 @@ void RegionSamplingThread::captureSample() { constexpr bool kGrayscale = false; constexpr bool kIsProtected = false; - if (const auto fenceResult = - mFlinger.captureScreenshot(SurfaceFlinger::RenderAreaBuilderVariant( - std::in_place_type, - sampledBounds, sampledBounds.getSize(), - ui::Dataspace::V0_SRGB, - kHintForSeamlessTransition, - true /* captureSecureLayers */, displayWeak), - getLayerSnapshotsFn, buffer, kRegionSampling, kGrayscale, - kIsProtected, nullptr) + SurfaceFlinger::RenderAreaBuilderVariant + renderAreaBuilder(std::in_place_type, sampledBounds, + sampledBounds.getSize(), ui::Dataspace::V0_SRGB, + kHintForSeamlessTransition, true /* captureSecureLayers */, + displayWeak); + + FenceResult fenceResult; + if (FlagManager::getInstance().single_hop_screenshot() && + FlagManager::getInstance().ce_fence_promise()) { + std::vector> layerFEs; + auto displayState = + mFlinger.getDisplayAndLayerSnapshotsFromMainThread(renderAreaBuilder, + getLayerSnapshotsFn, layerFEs); + fenceResult = + mFlinger.captureScreenshot(renderAreaBuilder, buffer, kRegionSampling, kGrayscale, + kIsProtected, nullptr, displayState, layerFEs) .get(); - fenceResult.ok()) { + } else { + fenceResult = + mFlinger.captureScreenshotLegacy(renderAreaBuilder, getLayerSnapshotsFn, buffer, + kRegionSampling, kGrayscale, kIsProtected, nullptr) + .get(); + } + if (fenceResult.ok()) { fenceResult.value()->waitForever(LOG_TAG); } diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index fb350107d8..a48a1b3a0b 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -8110,12 +8110,15 @@ void SurfaceFlinger::attachReleaseFenceFutureToLayer(Layer* layer, LayerFE* laye owningLayer->prepareReleaseCallbacks(std::move(futureFence), layerStack); } -bool SurfaceFlinger::layersHasProtectedLayer( - const std::vector>>& layers) const { +// Loop over all visible layers to see whether there's any protected layer. A protected layer is +// 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>& layers) const { bool protectedLayerFound = false; - for (auto& [_, layerFe] : layers) { + for (auto& layerFE : layers) { protectedLayerFound |= - (layerFe->mSnapshot->isVisible && layerFe->mSnapshot->hasProtectedContent); + (layerFE->mSnapshot->isVisible && layerFE->mSnapshot->hasProtectedContent); if (protectedLayerFound) { break; } @@ -8123,6 +8126,26 @@ bool SurfaceFlinger::layersHasProtectedLayer( return protectedLayerFound; } +// Getting layer snapshots and display should take place on main thread. +// Accessing display requires mStateLock, and contention for this lock +// is reduced when grabbed from the main thread, thus also reducing +// risk of deadlocks. +std::optional +SurfaceFlinger::getDisplayAndLayerSnapshotsFromMainThread( + RenderAreaBuilderVariant& renderAreaBuilder, GetLayerSnapshotsFunction getLayerSnapshotsFn, + std::vector>& layerFEs) { + return mScheduler + ->schedule([=, this, &renderAreaBuilder, &layerFEs]() REQUIRES(kMainThreadContext) { + auto layers = getLayerSnapshotsFn(); + for (auto& [layer, layerFE] : layers) { + attachReleaseFenceFutureToLayer(layer, layerFE.get(), ui::INVALID_LAYER_STACK); + } + layerFEs = extractLayerFEs(layers); + return getDisplayStateFromRenderAreaBuilder(renderAreaBuilder); + }) + .get(); +} + void SurfaceFlinger::captureScreenCommon(RenderAreaBuilderVariant renderAreaBuilder, GetLayerSnapshotsFunction getLayerSnapshotsFn, ui::Size bufferSize, ui::PixelFormat reqPixelFormat, @@ -8138,47 +8161,85 @@ void SurfaceFlinger::captureScreenCommon(RenderAreaBuilderVariant renderAreaBuil return; } - // Loop over all visible layers to see whether there's any protected layer. A protected layer is - // 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. - const bool supportsProtected = getRenderEngine().supportsProtectedContent(); - bool hasProtectedLayer = false; - if (allowProtected && supportsProtected) { - auto layers = mScheduler->schedule([=]() { return getLayerSnapshotsFn(); }).get(); - hasProtectedLayer = layersHasProtectedLayer(layers); - } - const bool isProtected = hasProtectedLayer && allowProtected && 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 buffer = - getFactory().createGraphicBuffer(bufferSize.getWidth(), bufferSize.getHeight(), - static_cast(reqPixelFormat), - 1 /* layerCount */, usage, "screenshot"); - - const status_t bufferStatus = buffer->initCheck(); - if (bufferStatus != OK) { - // Animations may end up being really janky, but don't crash here. - // Otherwise an irreponsible process may cause an SF crash by allocating - // too much. - ALOGE("%s: Buffer failed to allocate: %d", __func__, bufferStatus); - invokeScreenCaptureError(bufferStatus, captureListener); - return; + if (FlagManager::getInstance().single_hop_screenshot() && + FlagManager::getInstance().ce_fence_promise()) { + std::vector> layerFEs; + auto displayState = + getDisplayAndLayerSnapshotsFromMainThread(renderAreaBuilder, getLayerSnapshotsFn, + layerFEs); + + const bool supportsProtected = getRenderEngine().supportsProtectedContent(); + bool hasProtectedLayer = false; + if (allowProtected && supportsProtected) { + hasProtectedLayer = layersHasProtectedLayer(layerFEs); + } + const bool isProtected = hasProtectedLayer && allowProtected && 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 buffer = + getFactory().createGraphicBuffer(bufferSize.getWidth(), bufferSize.getHeight(), + static_cast(reqPixelFormat), + 1 /* layerCount */, usage, "screenshot"); + + const status_t bufferStatus = buffer->initCheck(); + if (bufferStatus != OK) { + // Animations may end up being really janky, but don't crash here. + // Otherwise an irreponsible process may cause an SF crash by allocating + // too much. + ALOGE("%s: Buffer failed to allocate: %d", __func__, bufferStatus); + invokeScreenCaptureError(bufferStatus, captureListener); + return; + } + const std::shared_ptr texture = std::make_shared< + renderengine::impl::ExternalTexture>(buffer, getRenderEngine(), + renderengine::impl::ExternalTexture::Usage:: + WRITEABLE); + auto futureFence = + captureScreenshot(renderAreaBuilder, texture, false /* regionSampling */, grayscale, + isProtected, captureListener, displayState, layerFEs); + futureFence.get(); + + } else { + const bool supportsProtected = getRenderEngine().supportsProtectedContent(); + bool hasProtectedLayer = false; + if (allowProtected && supportsProtected) { + auto layers = mScheduler->schedule([=]() { return getLayerSnapshotsFn(); }).get(); + hasProtectedLayer = layersHasProtectedLayer(extractLayerFEs(layers)); + } + const bool isProtected = hasProtectedLayer && allowProtected && 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 buffer = + getFactory().createGraphicBuffer(bufferSize.getWidth(), bufferSize.getHeight(), + static_cast(reqPixelFormat), + 1 /* layerCount */, usage, "screenshot"); + + const status_t bufferStatus = buffer->initCheck(); + if (bufferStatus != OK) { + // Animations may end up being really janky, but don't crash here. + // Otherwise an irreponsible process may cause an SF crash by allocating + // too much. + ALOGE("%s: Buffer failed to allocate: %d", __func__, bufferStatus); + invokeScreenCaptureError(bufferStatus, captureListener); + return; + } + const std::shared_ptr texture = std::make_shared< + renderengine::impl::ExternalTexture>(buffer, getRenderEngine(), + renderengine::impl::ExternalTexture::Usage:: + WRITEABLE); + auto futureFence = captureScreenshotLegacy(renderAreaBuilder, getLayerSnapshotsFn, texture, + false /* regionSampling */, grayscale, + isProtected, captureListener); + futureFence.get(); } - const std::shared_ptr texture = std::make_shared< - renderengine::impl::ExternalTexture>(buffer, getRenderEngine(), - renderengine::impl::ExternalTexture::Usage:: - WRITEABLE); - auto futureFence = - captureScreenshot(renderAreaBuilder, getLayerSnapshotsFn, texture, - false /* regionSampling */, grayscale, isProtected, captureListener); - futureFence.get(); } -const sp SurfaceFlinger::getRenderAreaDisplay( - RenderAreaBuilderVariant& renderAreaBuilder, OutputCompositionState& state) { +std::optional +SurfaceFlinger::getDisplayStateFromRenderAreaBuilder(RenderAreaBuilderVariant& renderAreaBuilder) { sp display = nullptr; { Mutex::Autolock lock(mStateLock); @@ -8207,24 +8268,64 @@ const sp SurfaceFlinger::getRenderAreaDisplay( } if (display != nullptr) { - state = display->getCompositionDisplay()->getState(); + return std::optional{display->getCompositionDisplay()->getState()}; } } - return display; + return std::nullopt; } -std::vector>> -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); - } +std::vector> SurfaceFlinger::extractLayerFEs( + const std::vector>>& layers) const { + std::vector> layerFEs; + layerFEs.reserve(layers.size()); + for (const auto& [_, layerFE] : layers) { + layerFEs.push_back(layerFE); } - return layers; + return layerFEs; } ftl::SharedFuture SurfaceFlinger::captureScreenshot( + const RenderAreaBuilderVariant& renderAreaBuilder, + const std::shared_ptr& buffer, bool regionSampling, + bool grayscale, bool isProtected, const sp& captureListener, + std::optional& displayState, std::vector>& layerFEs) { + ATRACE_CALL(); + + ScreenCaptureResults captureResults; + std::unique_ptr renderArea = + std::visit([](auto&& arg) -> std::unique_ptr { return arg.build(); }, + renderAreaBuilder); + + if (!renderArea) { + ALOGW("Skipping screen capture because of invalid render area."); + if (captureListener) { + captureResults.fenceResult = base::unexpected(NO_MEMORY); + captureListener->onScreenCaptureCompleted(captureResults); + } + return ftl::yield(base::unexpected(NO_ERROR)).share(); + } + + // Empty vector needed to pass into renderScreenImpl for legacy path + std::vector>> layers; + ftl::SharedFuture renderFuture = + renderScreenImpl(std::move(renderArea), buffer, regionSampling, grayscale, isProtected, + captureResults, displayState, layers, layerFEs); + + if (captureListener) { + // Defer blocking on renderFuture back to the Binder thread. + return ftl::Future(std::move(renderFuture)) + .then([captureListener, captureResults = std::move(captureResults)]( + FenceResult fenceResult) mutable -> FenceResult { + captureResults.fenceResult = std::move(fenceResult); + captureListener->onScreenCaptureCompleted(captureResults); + return base::unexpected(NO_ERROR); + }) + .share(); + } + return renderFuture; +} + +ftl::SharedFuture SurfaceFlinger::captureScreenshotLegacy( RenderAreaBuilderVariant renderAreaBuilder, GetLayerSnapshotsFunction getLayerSnapshotsFn, const std::shared_ptr& buffer, bool regionSampling, bool grayscale, bool isProtected, const sp& captureListener) { @@ -8232,10 +8333,13 @@ ftl::SharedFuture SurfaceFlinger::captureScreenshot( auto takeScreenshotFn = [=, this, renderAreaBuilder = std::move(renderAreaBuilder)]() REQUIRES( kMainThreadContext) mutable -> ftl::SharedFuture { - auto layers = getLayerSnapshotsFromMainThread(getLayerSnapshotsFn); - - OutputCompositionState state; - const auto display = getRenderAreaDisplay(renderAreaBuilder, state); + auto layers = getLayerSnapshotsFn(); + if (FlagManager::getInstance().ce_fence_promise()) { + for (auto& [layer, layerFE] : layers) { + attachReleaseFenceFutureToLayer(layer, layerFE.get(), ui::INVALID_LAYER_STACK); + } + } + auto displayState = getDisplayStateFromRenderAreaBuilder(renderAreaBuilder); ScreenCaptureResults captureResults; std::unique_ptr renderArea = @@ -8251,9 +8355,10 @@ ftl::SharedFuture SurfaceFlinger::captureScreenshot( return ftl::yield(base::unexpected(NO_ERROR)).share(); } + auto layerFEs = extractLayerFEs(layers); ftl::SharedFuture renderFuture = renderScreenImpl(std::move(renderArea), buffer, regionSampling, grayscale, - isProtected, captureResults, display, state, layers); + isProtected, captureResults, displayState, layers, layerFEs); if (captureListener) { // Defer blocking on renderFuture back to the Binder thread. @@ -8286,11 +8391,11 @@ ftl::SharedFuture SurfaceFlinger::renderScreenImpl( std::unique_ptr renderArea, const std::shared_ptr& buffer, bool regionSampling, bool grayscale, bool isProtected, ScreenCaptureResults& captureResults, - const sp display, const OutputCompositionState& state, - std::vector>>& layers) { + std::optional& displayState, + std::vector>>& layers, std::vector>& layerFEs) { ATRACE_CALL(); - for (auto& [_, layerFE] : layers) { + for (auto& layerFE : layerFEs) { frontend::LayerSnapshot* snapshot = layerFE->mSnapshot.get(); captureResults.capturedSecureLayers |= (snapshot->isVisible && snapshot->isSecure); captureResults.capturedHdrLayers |= isHdrLayer(*snapshot); @@ -8313,7 +8418,8 @@ ftl::SharedFuture SurfaceFlinger::renderScreenImpl( const bool enableLocalTonemapping = FlagManager::getInstance().local_tonemap_screenshots() && !renderArea->getHintForSeamlessTransition(); - if (display != nullptr) { + if (displayState) { + const auto& state = displayState.value(); captureResults.capturedDataspace = pickBestDataspace(requestedDataspace, state, captureResults.capturedHdrLayers, renderArea->getHintForSeamlessTransition()); @@ -8348,18 +8454,18 @@ ftl::SharedFuture SurfaceFlinger::renderScreenImpl( captureResults.buffer = capturedBuffer->getBuffer(); ui::LayerStack layerStack{ui::DEFAULT_LAYER_STACK}; - if (!layers.empty()) { - const sp& layerFE = layers.back().second; + if (!layerFEs.empty()) { + const sp& layerFE = layerFEs.back(); layerStack = layerFE->getCompositionState()->outputFilter.layerStack; } - auto copyLayerFEs = [&layers]() { - std::vector> layerFEs; - layerFEs.reserve(layers.size()); - for (const auto& [_, layerFE] : layers) { - layerFEs.push_back(layerFE); + auto copyLayerFEs = [&layerFEs]() { + std::vector> ceLayerFEs; + ceLayerFEs.reserve(layerFEs.size()); + for (const auto& layerFE : layerFEs) { + ceLayerFEs.push_back(layerFE); } - return layerFEs; + return ceLayerFEs; }; auto present = [this, buffer = capturedBuffer, dataspace = captureResults.capturedDataspace, @@ -8428,8 +8534,16 @@ ftl::SharedFuture SurfaceFlinger::renderScreenImpl( // // TODO(b/196334700) Once we use RenderEngineThreaded everywhere we can always defer the call // to CompositionEngine::present. - auto presentFuture = mRenderEngine->isThreaded() ? ftl::defer(std::move(present)).share() - : ftl::yield(present()).share(); + ftl::SharedFuture presentFuture; + if (FlagManager::getInstance().single_hop_screenshot() && + FlagManager::getInstance().ce_fence_promise()) { + 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(); + } if (!FlagManager::getInstance().ce_fence_promise()) { for (auto& [layer, layerFE] : layers) { diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 209d9bcfe6..12307172f7 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -892,22 +892,35 @@ 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>>& layers) const; + bool layersHasProtectedLayer(const std::vector>& layers) const; + + using OutputCompositionState = compositionengine::impl::OutputCompositionState; + + std::optional getDisplayAndLayerSnapshotsFromMainThread( + RenderAreaBuilderVariant& renderAreaBuilder, + GetLayerSnapshotsFunction getLayerSnapshotsFn, std::vector>& layerFEs); void captureScreenCommon(RenderAreaBuilderVariant, GetLayerSnapshotsFunction, ui::Size bufferSize, ui::PixelFormat, bool allowProtected, bool grayscale, const sp&); - using OutputCompositionState = compositionengine::impl::OutputCompositionState; - - const sp getRenderAreaDisplay(RenderAreaBuilderVariant& renderAreaBuilder, - OutputCompositionState& state) - REQUIRES(kMainThreadContext); + std::optional getDisplayStateFromRenderAreaBuilder( + RenderAreaBuilderVariant& renderAreaBuilder) REQUIRES(kMainThreadContext); - std::vector>> getLayerSnapshotsFromMainThread( - GetLayerSnapshotsFunction getLayerSnapshotsFn) 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> extractLayerFEs( + const std::vector>>& layers) const; ftl::SharedFuture captureScreenshot( + const RenderAreaBuilderVariant& renderAreaBuilder, + const std::shared_ptr& buffer, bool regionSampling, + bool grayscale, bool isProtected, const sp& captureListener, + std::optional& displayState, + std::vector>& layerFEs); + + ftl::SharedFuture captureScreenshotLegacy( RenderAreaBuilderVariant, GetLayerSnapshotsFunction, const std::shared_ptr&, bool regionSampling, bool grayscale, bool isProtected, const sp&); @@ -916,9 +929,9 @@ private: std::unique_ptr, const std::shared_ptr&, bool regionSampling, bool grayscale, bool isProtected, ScreenCaptureResults&, - const sp display, const OutputCompositionState& state, - std::vector>>& layers) - REQUIRES(kMainThreadContext); + std::optional& displayState, + std::vector>>& layers, + std::vector>& layerFEs); // If the uid provided is not UNSET_UID, the traverse will skip any layers that don't have a // matching ownerUid diff --git a/services/surfaceflinger/common/FlagManager.cpp b/services/surfaceflinger/common/FlagManager.cpp index b7ec6e0b26..4216771515 100644 --- a/services/surfaceflinger/common/FlagManager.cpp +++ b/services/surfaceflinger/common/FlagManager.cpp @@ -150,6 +150,7 @@ void FlagManager::dump(std::string& result) const { DUMP_READ_ONLY_FLAG(override_trusted_overlay); DUMP_READ_ONLY_FLAG(flush_buffer_slots_to_uncache); DUMP_READ_ONLY_FLAG(force_compile_graphite_renderengine); + DUMP_READ_ONLY_FLAG(single_hop_screenshot); #undef DUMP_READ_ONLY_FLAG #undef DUMP_SERVER_FLAG @@ -250,6 +251,7 @@ FLAG_MANAGER_READ_ONLY_FLAG(local_tonemap_screenshots, "debug.sf.local_tonemap_s FLAG_MANAGER_READ_ONLY_FLAG(override_trusted_overlay, ""); FLAG_MANAGER_READ_ONLY_FLAG(flush_buffer_slots_to_uncache, ""); FLAG_MANAGER_READ_ONLY_FLAG(force_compile_graphite_renderengine, ""); +FLAG_MANAGER_READ_ONLY_FLAG(single_hop_screenshot, ""); /// Trunk stable server flags /// FLAG_MANAGER_SERVER_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 8f98ed3777..22118ab585 100644 --- a/services/surfaceflinger/common/include/common/FlagManager.h +++ b/services/surfaceflinger/common/include/common/FlagManager.h @@ -89,6 +89,7 @@ public: bool override_trusted_overlay() const; bool flush_buffer_slots_to_uncache() const; bool force_compile_graphite_renderengine() const; + bool single_hop_screenshot() const; protected: // overridden for unit tests diff --git a/services/surfaceflinger/surfaceflinger_flags_new.aconfig b/services/surfaceflinger/surfaceflinger_flags_new.aconfig index ee12325705..f4d4ee978d 100644 --- a/services/surfaceflinger/surfaceflinger_flags_new.aconfig +++ b/services/surfaceflinger/surfaceflinger_flags_new.aconfig @@ -103,6 +103,17 @@ flag { is_fixed_read_only: true } # local_tonemap_screenshots +flag { + name: "single_hop_screenshot" + namespace: "window_surfaces" + description: "Only access SF main thread once during a screenshot" + bug: "285553970" + is_fixed_read_only: true + metadata { + purpose: PURPOSE_BUGFIX + } + } # single_hop_screenshot + flag { name: "override_trusted_overlay" namespace: "window_surfaces" diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h index 7889096fe2..8c72a7dba5 100644 --- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h +++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h @@ -494,12 +494,13 @@ public: ftl::FakeGuard guard(kMainThreadContext); ScreenCaptureResults captureResults; - SurfaceFlinger::OutputCompositionState state = display->getCompositionDisplay()->getState(); - auto layers = mFlinger->getLayerSnapshotsFromMainThread(getLayerSnapshotsFn); + auto displayState = std::optional{display->getCompositionDisplay()->getState()}; + auto layers = getLayerSnapshotsFn(); + auto layerFEs = mFlinger->extractLayerFEs(layers); return mFlinger->renderScreenImpl(std::move(renderArea), buffer, regionSampling, false /* grayscale */, false /* isProtected */, - captureResults, display, state, layers); + captureResults, displayState, layers, layerFEs); } auto traverseLayersInLayerStack(ui::LayerStack layerStack, int32_t uid, -- cgit v1.2.3-59-g8ed1b