From 0077fde3aba6f2bde4e878f88c0dd466350fc1b1 Mon Sep 17 00:00:00 2001 From: Melody Hsu Date: Thu, 17 Oct 2024 21:42:50 +0000 Subject: Remove release fence flags ce_fence_promise and screenshot_fence_preservation flags have been out for several releases. They can be deleted, along with any obsolete code related to the flag removal. Bug: 351894825 Test: atest SurfaceFlinger_test Flag: EXEMPT flag removal Change-Id: Iba6166358c96ff6cfe5c64b68640fcd992c4089f --- services/surfaceflinger/RegionSamplingThread.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'services/surfaceflinger/RegionSamplingThread.cpp') diff --git a/services/surfaceflinger/RegionSamplingThread.cpp b/services/surfaceflinger/RegionSamplingThread.cpp index 06c2f26a6d..011fd9e20a 100644 --- a/services/surfaceflinger/RegionSamplingThread.cpp +++ b/services/surfaceflinger/RegionSamplingThread.cpp @@ -355,7 +355,7 @@ void RegionSamplingThread::captureSample() { FenceResult fenceResult; if (FlagManager::getInstance().single_hop_screenshot() && - FlagManager::getInstance().ce_fence_promise() && mFlinger.mRenderEngine->isThreaded()) { + mFlinger.mRenderEngine->isThreaded()) { std::vector> layerFEs; auto displayState = mFlinger.getSnapshotsFromMainThread(renderAreaBuilder, getLayerSnapshotsFn, layerFEs); -- cgit v1.2.3-59-g8ed1b From 0b80c74300b73e937ee9a9cb58487bd126daa4d8 Mon Sep 17 00:00:00 2001 From: Melody Hsu Date: Tue, 29 Oct 2024 07:24:29 +0000 Subject: Reorder release fence attachment for non-threaded RE Changes when release fences are attached to layers for non-threaded RenderEngine to ensure that fences are added and fired in the same hop to the main thread. This removes the dependency on legacy screenshot code and prevents a deadlock where screenshot requests are blocked on the main thread while the main thread is blocked on the previous screenshot request finishing. Bug: b/351894825 Test: atest SurfaceFlinger_test Flag: EXEMPT refactor and for flag removal Change-Id: If9d4134aba1106484f94231c5104a57a605147b8 --- services/surfaceflinger/RegionSamplingThread.cpp | 11 ++- services/surfaceflinger/SurfaceFlinger.cpp | 97 +++++++++++----------- services/surfaceflinger/SurfaceFlinger.h | 16 ++-- .../tests/unittests/TestableSurfaceFlinger.h | 3 +- 4 files changed, 58 insertions(+), 69 deletions(-) (limited to 'services/surfaceflinger/RegionSamplingThread.cpp') diff --git a/services/surfaceflinger/RegionSamplingThread.cpp b/services/surfaceflinger/RegionSamplingThread.cpp index 011fd9e20a..08f831c8e5 100644 --- a/services/surfaceflinger/RegionSamplingThread.cpp +++ b/services/surfaceflinger/RegionSamplingThread.cpp @@ -354,14 +354,13 @@ void RegionSamplingThread::captureSample() { RenderArea::Options::CAPTURE_SECURE_LAYERS); FenceResult fenceResult; - if (FlagManager::getInstance().single_hop_screenshot() && - mFlinger.mRenderEngine->isThreaded()) { - std::vector> layerFEs; - auto displayState = mFlinger.getSnapshotsFromMainThread(renderAreaBuilder, - getLayerSnapshotsFn, layerFEs); + if (FlagManager::getInstance().single_hop_screenshot()) { + std::vector>> layers; + auto displayState = + mFlinger.getSnapshotsFromMainThread(renderAreaBuilder, getLayerSnapshotsFn, layers); fenceResult = mFlinger.captureScreenshot(renderAreaBuilder, buffer, kRegionSampling, kGrayscale, kIsProtected, kAttachGainmap, nullptr, - displayState, layerFEs) + displayState, layers) .get(); } else { fenceResult = mFlinger.captureScreenshotLegacy(renderAreaBuilder, getLayerSnapshotsFn, diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index d35a76ad4b..34798e1216 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -7158,9 +7158,10 @@ void SurfaceFlinger::attachReleaseFenceFutureToLayer(Layer* layer, LayerFE* laye // typically a layer with DRM contents, or have the GRALLOC_USAGE_PROTECTED set on the buffer. // A protected layer has no implication on whether it's secure, which is explicitly set by // application to avoid being screenshot or drawn via unsecure display. -bool SurfaceFlinger::layersHasProtectedLayer(const std::vector>& layers) const { +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); if (protectedLayerFound) { @@ -7176,15 +7177,21 @@ bool SurfaceFlinger::layersHasProtectedLayer(const std::vector>& lay // risk of deadlocks. std::optional SurfaceFlinger::getSnapshotsFromMainThread( RenderAreaBuilderVariant& renderAreaBuilder, GetLayerSnapshotsFunction getLayerSnapshotsFn, - std::vector>& layerFEs) { + std::vector>>& layers) { return mScheduler - ->schedule([=, this, &renderAreaBuilder, &layerFEs]() REQUIRES(kMainThreadContext) { + ->schedule([=, this, &renderAreaBuilder, &layers]() REQUIRES(kMainThreadContext) { SFTRACE_NAME("getSnapshotsFromMainThread"); - auto layers = getLayerSnapshotsFn(); - for (auto& [layer, layerFE] : layers) { - attachReleaseFenceFutureToLayer(layer, layerFE.get(), ui::INVALID_LAYER_STACK); + layers = getLayerSnapshotsFn(); + // Non-threaded RenderEngine eventually returns to the main thread a 2nd time + // to complete the screenshot. Release fences should only be added during the 2nd + // hop to main thread in order to avoid potential deadlocks from waiting for the + // the future fence to fire. + if (mRenderEngine->isThreaded()) { + for (auto& [layer, layerFE] : layers) { + attachReleaseFenceFutureToLayer(layer, layerFE.get(), + ui::INVALID_LAYER_STACK); + } } - layerFEs = extractLayerFEs(layers); return getDisplayStateFromRenderAreaBuilder(renderAreaBuilder); }) .get(); @@ -7205,15 +7212,15 @@ void SurfaceFlinger::captureScreenCommon(RenderAreaBuilderVariant renderAreaBuil return; } - if (FlagManager::getInstance().single_hop_screenshot() && mRenderEngine->isThreaded()) { - std::vector> layerFEs; + if (FlagManager::getInstance().single_hop_screenshot()) { + std::vector>> layers; auto displayState = - getSnapshotsFromMainThread(renderAreaBuilder, getLayerSnapshotsFn, layerFEs); + getSnapshotsFromMainThread(renderAreaBuilder, getLayerSnapshotsFn, layers); const bool supportsProtected = getRenderEngine().supportsProtectedContent(); bool hasProtectedLayer = false; if (allowProtected && supportsProtected) { - hasProtectedLayer = layersHasProtectedLayer(layerFEs); + hasProtectedLayer = layersHasProtectedLayer(layers); } const bool isProtected = hasProtectedLayer && allowProtected && supportsProtected; const uint32_t usage = GRALLOC_USAGE_HW_COMPOSER | GRALLOC_USAGE_HW_RENDER | @@ -7240,7 +7247,7 @@ void SurfaceFlinger::captureScreenCommon(RenderAreaBuilderVariant renderAreaBuil WRITEABLE); auto futureFence = captureScreenshot(renderAreaBuilder, texture, false /* regionSampling */, grayscale, isProtected, attachGainmap, captureListener, - displayState, layerFEs); + displayState, layers); futureFence.get(); } else { @@ -7248,7 +7255,7 @@ void SurfaceFlinger::captureScreenCommon(RenderAreaBuilderVariant renderAreaBuil bool hasProtectedLayer = false; if (allowProtected && supportsProtected) { auto layers = mScheduler->schedule([=]() { return getLayerSnapshotsFn(); }).get(); - hasProtectedLayer = layersHasProtectedLayer(extractLayerFEs(layers)); + hasProtectedLayer = layersHasProtectedLayer(layers); } const bool isProtected = hasProtectedLayer && allowProtected && supportsProtected; const uint32_t usage = GRALLOC_USAGE_HW_COMPOSER | GRALLOC_USAGE_HW_RENDER | @@ -7316,22 +7323,13 @@ SurfaceFlinger::getDisplayStateFromRenderAreaBuilder(RenderAreaBuilderVariant& r return std::nullopt; } -std::vector> SurfaceFlinger::extractLayerFEs( - const std::vector>>& layers) const { - std::vector> layerFEs; - layerFEs.reserve(layers.size()); - for (const auto& [_, layerFE] : layers) { - layerFEs.push_back(layerFE); - } - return layerFEs; -} - ftl::SharedFuture SurfaceFlinger::captureScreenshot( const RenderAreaBuilderVariant& renderAreaBuilder, const std::shared_ptr& buffer, bool regionSampling, bool grayscale, bool isProtected, bool attachGainmap, const sp& captureListener, - std::optional& displayState, std::vector>& layerFEs) { + std::optional& displayState, + std::vector>>& layers) { SFTRACE_CALL(); ScreenCaptureResults captureResults; @@ -7350,11 +7348,9 @@ ftl::SharedFuture SurfaceFlinger::captureScreenshot( float displayBrightnessNits = displayState.value().displayBrightnessNits; float sdrWhitePointNits = displayState.value().sdrWhitePointNits; - // Empty vector needed to pass into renderScreenImpl for legacy path - std::vector>> layers; ftl::SharedFuture renderFuture = renderScreenImpl(renderArea.get(), buffer, regionSampling, grayscale, isProtected, - attachGainmap, captureResults, displayState, layers, layerFEs); + attachGainmap, captureResults, displayState, layers); if (captureResults.capturedHdrLayers && attachGainmap && FlagManager::getInstance().true_hdr_screenshots()) { @@ -7390,7 +7386,7 @@ ftl::SharedFuture SurfaceFlinger::captureScreenshot( ftl::SharedFuture hdrRenderFuture = renderScreenImpl(renderArea.get(), hdrTexture, regionSampling, grayscale, isProtected, attachGainmap, unusedResults, displayState, - layers, layerFEs); + layers); renderFuture = ftl::Future(std::move(renderFuture)) @@ -7446,9 +7442,6 @@ ftl::SharedFuture SurfaceFlinger::captureScreenshotLegacy( auto takeScreenshotFn = [=, this, renderAreaBuilder = std::move(renderAreaBuilder)]() REQUIRES( kMainThreadContext) mutable -> ftl::SharedFuture { auto layers = getLayerSnapshotsFn(); - for (auto& [layer, layerFE] : layers) { - attachReleaseFenceFutureToLayer(layer, layerFE.get(), ui::INVALID_LAYER_STACK); - } auto displayState = getDisplayStateFromRenderAreaBuilder(renderAreaBuilder); ScreenCaptureResults captureResults; @@ -7465,10 +7458,9 @@ ftl::SharedFuture SurfaceFlinger::captureScreenshotLegacy( return ftl::yield(base::unexpected(NO_ERROR)).share(); } - auto layerFEs = extractLayerFEs(layers); ftl::SharedFuture renderFuture = renderScreenImpl(renderArea.get(), buffer, regionSampling, grayscale, isProtected, - attachGainmap, captureResults, displayState, layers, layerFEs); + attachGainmap, captureResults, displayState, layers); if (captureListener) { // Defer blocking on renderFuture back to the Binder thread. @@ -7501,10 +7493,10 @@ ftl::SharedFuture SurfaceFlinger::renderScreenImpl( const RenderArea* renderArea, const std::shared_ptr& buffer, bool regionSampling, bool grayscale, bool isProtected, bool attachGainmap, ScreenCaptureResults& captureResults, std::optional& displayState, - std::vector>>& layers, std::vector>& layerFEs) { + std::vector>>& layers) { SFTRACE_CALL(); - for (auto& layerFE : layerFEs) { + for (auto& [_, layerFE] : layers) { frontend::LayerSnapshot* snapshot = layerFE->mSnapshot.get(); captureResults.capturedSecureLayers |= (snapshot->isVisible && snapshot->isSecure); captureResults.capturedHdrLayers |= isHdrLayer(*snapshot); @@ -7563,29 +7555,32 @@ ftl::SharedFuture SurfaceFlinger::renderScreenImpl( captureResults.buffer = capturedBuffer->getBuffer(); ui::LayerStack layerStack{ui::DEFAULT_LAYER_STACK}; - if (!layerFEs.empty()) { - const sp& layerFE = layerFEs.back(); + if (!layers.empty()) { + const sp& layerFE = layers.back().second; layerStack = layerFE->getCompositionState()->outputFilter.layerStack; } - auto copyLayerFEs = [&layerFEs]() { - std::vector> ceLayerFEs; - ceLayerFEs.reserve(layerFEs.size()); - for (const auto& layerFE : layerFEs) { - ceLayerFEs.push_back(layerFE); - } - return ceLayerFEs; - }; - auto present = [this, buffer = capturedBuffer, dataspace = captureResults.capturedDataspace, sdrWhitePointNits, displayBrightnessNits, grayscale, isProtected, - layerFEs = copyLayerFEs(), layerStack, regionSampling, + layers = std::move(layers), layerStack, regionSampling, renderArea = std::move(renderArea), renderIntent, enableLocalTonemapping]() -> FenceResult { std::unique_ptr compositionEngine = mFactory.createCompositionEngine(); compositionEngine->setRenderEngine(mRenderEngine.get()); + std::vector> layerFEs; + layerFEs.reserve(layers.size()); + for (auto& [layer, layerFE] : layers) { + // Release fences were not yet added for non-threaded render engine. To avoid + // deadlocks between main thread and binder threads waiting for the future fence + // result, fences should be added to layers in the same hop onto the main thread. + if (!mRenderEngine->isThreaded()) { + attachReleaseFenceFutureToLayer(layer, layerFE.get(), ui::INVALID_LAYER_STACK); + } + layerFEs.push_back(layerFE); + } + compositionengine::Output::ColorProfile colorProfile{.dataspace = dataspace, .renderIntent = renderIntent}; @@ -7644,8 +7639,10 @@ ftl::SharedFuture SurfaceFlinger::renderScreenImpl( // TODO(b/196334700) Once we use RenderEngineThreaded everywhere we can always defer the call // to CompositionEngine::present. ftl::SharedFuture presentFuture; - if (FlagManager::getInstance().single_hop_screenshot() && mRenderEngine->isThreaded()) { - presentFuture = ftl::yield(present()).share(); + if (FlagManager::getInstance().single_hop_screenshot()) { + presentFuture = mRenderEngine->isThreaded() + ? ftl::yield(present()).share() + : mScheduler->schedule(std::move(present)).share(); } else { presentFuture = mRenderEngine->isThreaded() ? ftl::defer(std::move(present)).share() : ftl::yield(present()).share(); diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 31218edbe7..114a09b521 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -851,13 +851,14 @@ private: void attachReleaseFenceFutureToLayer(Layer* layer, LayerFE* layerFE, ui::LayerStack layerStack); // Checks if a protected layer exists in a list of layers. - bool layersHasProtectedLayer(const std::vector>& layers) const; + bool layersHasProtectedLayer(const std::vector>>& layers) const; using OutputCompositionState = compositionengine::impl::OutputCompositionState; std::optional getSnapshotsFromMainThread( RenderAreaBuilderVariant& renderAreaBuilder, - GetLayerSnapshotsFunction getLayerSnapshotsFn, std::vector>& layerFEs); + GetLayerSnapshotsFunction getLayerSnapshotsFn, + std::vector>>& layers); void captureScreenCommon(RenderAreaBuilderVariant, GetLayerSnapshotsFunction, ui::Size bufferSize, ui::PixelFormat, bool allowProtected, @@ -866,19 +867,13 @@ private: std::optional getDisplayStateFromRenderAreaBuilder( RenderAreaBuilderVariant& renderAreaBuilder) REQUIRES(kMainThreadContext); - // Legacy layer raw pointer is not safe to access outside the main thread. - // Creates a new vector consisting only of LayerFEs, which can be safely - // accessed outside the main thread. - std::vector> extractLayerFEs( - const std::vector>>& layers) const; - ftl::SharedFuture captureScreenshot( const RenderAreaBuilderVariant& renderAreaBuilder, const std::shared_ptr& buffer, bool regionSampling, bool grayscale, bool isProtected, bool attachGainmap, const sp& captureListener, std::optional& displayState, - std::vector>& layerFEs); + std::vector>>& layers); ftl::SharedFuture captureScreenshotLegacy( RenderAreaBuilderVariant, GetLayerSnapshotsFunction, @@ -890,8 +885,7 @@ private: const RenderArea*, const std::shared_ptr&, bool regionSampling, bool grayscale, bool isProtected, bool attachGainmap, ScreenCaptureResults&, std::optional& displayState, - std::vector>>& layers, - std::vector>& layerFEs); + std::vector>>& layers); void readPersistentProperties(); diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h index 4dec5f6b6a..81b929beec 100644 --- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h +++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h @@ -472,12 +472,11 @@ public: ScreenCaptureResults captureResults; auto displayState = std::optional{display->getCompositionDisplay()->getState()}; auto layers = getLayerSnapshotsFn(); - auto layerFEs = mFlinger->extractLayerFEs(layers); return mFlinger->renderScreenImpl(renderArea.get(), buffer, regionSampling, false /* grayscale */, false /* isProtected */, false /* attachGainmap */, captureResults, displayState, - layers, layerFEs); + layers); } auto getLayerSnapshotsForScreenshotsFn(ui::LayerStack layerStack, uint32_t uid) { -- cgit v1.2.3-59-g8ed1b From 16c4c32c55231dc241c386a0423710baa452b7de Mon Sep 17 00:00:00 2001 From: Melody Hsu Date: Tue, 29 Oct 2024 07:50:14 +0000 Subject: Remove flag single_hop_screenshots Flag has been rolled out for several months and is safe to remove along with any legacy methods and dead code. Bug: b/351894825 Test: atest SurfaceFlinger_test Flag: EXEMPT flag removal Change-Id: Id6fdfc534a0bff6ff9dfb8727c08ff821bc4965a --- services/surfaceflinger/RegionSamplingThread.cpp | 22 +-- services/surfaceflinger/SurfaceFlinger.cpp | 184 +++++---------------- services/surfaceflinger/SurfaceFlinger.h | 10 +- services/surfaceflinger/common/FlagManager.cpp | 2 - .../common/include/common/FlagManager.h | 1 - .../tests/unittests/TestableSurfaceFlinger.h | 3 +- 6 files changed, 51 insertions(+), 171 deletions(-) (limited to 'services/surfaceflinger/RegionSamplingThread.cpp') diff --git a/services/surfaceflinger/RegionSamplingThread.cpp b/services/surfaceflinger/RegionSamplingThread.cpp index 08f831c8e5..21d3396ebe 100644 --- a/services/surfaceflinger/RegionSamplingThread.cpp +++ b/services/surfaceflinger/RegionSamplingThread.cpp @@ -353,21 +353,13 @@ void RegionSamplingThread::captureSample() { sampledBounds.getSize(), ui::Dataspace::V0_SRGB, displayWeak, RenderArea::Options::CAPTURE_SECURE_LAYERS); - FenceResult fenceResult; - if (FlagManager::getInstance().single_hop_screenshot()) { - std::vector>> layers; - auto displayState = - mFlinger.getSnapshotsFromMainThread(renderAreaBuilder, getLayerSnapshotsFn, layers); - fenceResult = mFlinger.captureScreenshot(renderAreaBuilder, buffer, kRegionSampling, - kGrayscale, kIsProtected, kAttachGainmap, nullptr, - displayState, layers) - .get(); - } else { - fenceResult = mFlinger.captureScreenshotLegacy(renderAreaBuilder, getLayerSnapshotsFn, - buffer, kRegionSampling, kGrayscale, - kIsProtected, kAttachGainmap, nullptr) - .get(); - } + std::vector>> layers; + auto displayState = + mFlinger.getSnapshotsFromMainThread(renderAreaBuilder, getLayerSnapshotsFn, layers); + FenceResult fenceResult = + mFlinger.captureScreenshot(renderAreaBuilder, buffer, kRegionSampling, kGrayscale, + kIsProtected, kAttachGainmap, nullptr, displayState, layers) + .get(); if (fenceResult.ok()) { fenceResult.value()->waitForever(LOG_TAG); } diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 34798e1216..d1d5eb5e51 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -7212,79 +7212,41 @@ void SurfaceFlinger::captureScreenCommon(RenderAreaBuilderVariant renderAreaBuil return; } - if (FlagManager::getInstance().single_hop_screenshot()) { - std::vector>> layers; - auto displayState = - getSnapshotsFromMainThread(renderAreaBuilder, getLayerSnapshotsFn, layers); - - const bool supportsProtected = getRenderEngine().supportsProtectedContent(); - bool hasProtectedLayer = false; - if (allowProtected && supportsProtected) { - 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; - } - 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, attachGainmap, captureListener, - displayState, layers); - futureFence.get(); - - } else { - 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; - } - 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, attachGainmap, captureListener); - futureFence.get(); + std::vector>> layers; + auto displayState = getSnapshotsFromMainThread(renderAreaBuilder, getLayerSnapshotsFn, layers); + + const bool supportsProtected = getRenderEngine().supportsProtectedContent(); + bool hasProtectedLayer = false; + if (allowProtected && supportsProtected) { + 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; } + 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, attachGainmap, captureListener, displayState, layers); + futureFence.get(); } std::optional @@ -7350,7 +7312,7 @@ ftl::SharedFuture SurfaceFlinger::captureScreenshot( ftl::SharedFuture renderFuture = renderScreenImpl(renderArea.get(), buffer, regionSampling, grayscale, isProtected, - attachGainmap, captureResults, displayState, layers); + captureResults, displayState, layers); if (captureResults.capturedHdrLayers && attachGainmap && FlagManager::getInstance().true_hdr_screenshots()) { @@ -7385,8 +7347,7 @@ ftl::SharedFuture SurfaceFlinger::captureScreenshot( ScreenCaptureResults unusedResults; ftl::SharedFuture hdrRenderFuture = renderScreenImpl(renderArea.get(), hdrTexture, regionSampling, grayscale, - isProtected, attachGainmap, unusedResults, displayState, - layers); + isProtected, unusedResults, displayState, layers); renderFuture = ftl::Future(std::move(renderFuture)) @@ -7432,67 +7393,10 @@ ftl::SharedFuture SurfaceFlinger::captureScreenshot( return renderFuture; } -ftl::SharedFuture SurfaceFlinger::captureScreenshotLegacy( - RenderAreaBuilderVariant renderAreaBuilder, GetLayerSnapshotsFunction getLayerSnapshotsFn, - const std::shared_ptr& buffer, bool regionSampling, - bool grayscale, bool isProtected, bool attachGainmap, - const sp& captureListener) { - SFTRACE_CALL(); - - auto takeScreenshotFn = [=, this, renderAreaBuilder = std::move(renderAreaBuilder)]() REQUIRES( - kMainThreadContext) mutable -> ftl::SharedFuture { - auto layers = getLayerSnapshotsFn(); - auto displayState = getDisplayStateFromRenderAreaBuilder(renderAreaBuilder); - - 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(); - } - - ftl::SharedFuture renderFuture = - renderScreenImpl(renderArea.get(), buffer, regionSampling, grayscale, isProtected, - attachGainmap, captureResults, displayState, layers); - - 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; - }; - - // TODO(b/294936197): Run takeScreenshotsFn() in a binder thread to reduce the number - // of calls on the main thread. - auto future = - mScheduler->schedule(FTL_FAKE_GUARD(kMainThreadContext, std::move(takeScreenshotFn))); - - // Flatten nested futures. - auto chain = ftl::Future(std::move(future)).then([](ftl::SharedFuture future) { - return future; - }); - - return chain.share(); -} - ftl::SharedFuture SurfaceFlinger::renderScreenImpl( const RenderArea* renderArea, const std::shared_ptr& buffer, - bool regionSampling, bool grayscale, bool isProtected, bool attachGainmap, - ScreenCaptureResults& captureResults, std::optional& displayState, + bool regionSampling, bool grayscale, bool isProtected, ScreenCaptureResults& captureResults, + std::optional& displayState, std::vector>>& layers) { SFTRACE_CALL(); @@ -7638,15 +7542,9 @@ ftl::SharedFuture SurfaceFlinger::renderScreenImpl( // // TODO(b/196334700) Once we use RenderEngineThreaded everywhere we can always defer the call // to CompositionEngine::present. - ftl::SharedFuture presentFuture; - if (FlagManager::getInstance().single_hop_screenshot()) { - 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(); - } + ftl::SharedFuture presentFuture = mRenderEngine->isThreaded() + ? ftl::yield(present()).share() + : mScheduler->schedule(std::move(present)).share(); return presentFuture; } diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 114a09b521..ad3106c05e 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -875,16 +875,10 @@ private: std::optional& displayState, std::vector>>& layers); - ftl::SharedFuture captureScreenshotLegacy( - RenderAreaBuilderVariant, GetLayerSnapshotsFunction, - const std::shared_ptr&, bool regionSampling, - bool grayscale, bool isProtected, bool attachGainmap, - const sp&); - ftl::SharedFuture renderScreenImpl( const RenderArea*, const std::shared_ptr&, - bool regionSampling, bool grayscale, bool isProtected, bool attachGainmap, - ScreenCaptureResults&, std::optional& displayState, + bool regionSampling, bool grayscale, bool isProtected, ScreenCaptureResults&, + std::optional& displayState, std::vector>>& layers); void readPersistentProperties(); diff --git a/services/surfaceflinger/common/FlagManager.cpp b/services/surfaceflinger/common/FlagManager.cpp index 658bca6f31..feeafccf36 100644 --- a/services/surfaceflinger/common/FlagManager.cpp +++ b/services/surfaceflinger/common/FlagManager.cpp @@ -153,7 +153,6 @@ 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); DUMP_READ_ONLY_FLAG(trace_frame_rate_override); DUMP_READ_ONLY_FLAG(true_hdr_screenshots); DUMP_READ_ONLY_FLAG(display_config_error_hal); @@ -259,7 +258,6 @@ 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, ""); FLAG_MANAGER_READ_ONLY_FLAG(true_hdr_screenshots, "debug.sf.true_hdr_screenshots"); FLAG_MANAGER_READ_ONLY_FLAG(display_config_error_hal, ""); diff --git a/services/surfaceflinger/common/include/common/FlagManager.h b/services/surfaceflinger/common/include/common/FlagManager.h index 4f34718f4e..e4305d3bd8 100644 --- a/services/surfaceflinger/common/include/common/FlagManager.h +++ b/services/surfaceflinger/common/include/common/FlagManager.h @@ -91,7 +91,6 @@ public: bool override_trusted_overlay() const; bool flush_buffer_slots_to_uncache() const; bool force_compile_graphite_renderengine() const; - bool single_hop_screenshot() const; bool trace_frame_rate_override() const; bool true_hdr_screenshots() const; bool display_config_error_hal() const; diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h index 81b929beec..6778af3547 100644 --- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h +++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h @@ -475,8 +475,7 @@ public: return mFlinger->renderScreenImpl(renderArea.get(), buffer, regionSampling, false /* grayscale */, false /* isProtected */, - false /* attachGainmap */, captureResults, displayState, - layers); + captureResults, displayState, layers); } auto getLayerSnapshotsForScreenshotsFn(ui::LayerStack layerStack, uint32_t uid) { -- cgit v1.2.3-59-g8ed1b