334 lines
19 KiB
Diff
334 lines
19 KiB
Diff
From aa5e6f794b3e2c9f5bdbc40da702b14dd9a1764e Mon Sep 17 00:00:00 2001
|
|
From: Peter Cai <peter@typeblog.net>
|
|
Date: Sun, 16 Mar 2025 13:19:17 -0400
|
|
Subject: [PATCH 5/7] Revert "Reorder release fence attachment for non-threaded
|
|
RE"
|
|
|
|
This reverts commit 0b80c74300b73e937ee9a9cb58487bd126daa4d8.
|
|
---
|
|
.../surfaceflinger/RegionSamplingThread.cpp | 11 ++-
|
|
services/surfaceflinger/SurfaceFlinger.cpp | 97 ++++++++++---------
|
|
services/surfaceflinger/SurfaceFlinger.h | 16 ++-
|
|
.../tests/unittests/TestableSurfaceFlinger.h | 3 +-
|
|
4 files changed, 69 insertions(+), 58 deletions(-)
|
|
|
|
diff --git a/services/surfaceflinger/RegionSamplingThread.cpp b/services/surfaceflinger/RegionSamplingThread.cpp
|
|
index 08f831c8e5..011fd9e20a 100644
|
|
--- a/services/surfaceflinger/RegionSamplingThread.cpp
|
|
+++ b/services/surfaceflinger/RegionSamplingThread.cpp
|
|
@@ -354,13 +354,14 @@ void RegionSamplingThread::captureSample() {
|
|
RenderArea::Options::CAPTURE_SECURE_LAYERS);
|
|
|
|
FenceResult fenceResult;
|
|
- if (FlagManager::getInstance().single_hop_screenshot()) {
|
|
- std::vector<std::pair<Layer*, sp<LayerFE>>> layers;
|
|
- auto displayState =
|
|
- mFlinger.getSnapshotsFromMainThread(renderAreaBuilder, getLayerSnapshotsFn, layers);
|
|
+ if (FlagManager::getInstance().single_hop_screenshot() &&
|
|
+ mFlinger.mRenderEngine->isThreaded()) {
|
|
+ std::vector<sp<LayerFE>> layerFEs;
|
|
+ auto displayState = mFlinger.getSnapshotsFromMainThread(renderAreaBuilder,
|
|
+ getLayerSnapshotsFn, layerFEs);
|
|
fenceResult = mFlinger.captureScreenshot(renderAreaBuilder, buffer, kRegionSampling,
|
|
kGrayscale, kIsProtected, kAttachGainmap, nullptr,
|
|
- displayState, layers)
|
|
+ displayState, layerFEs)
|
|
.get();
|
|
} else {
|
|
fenceResult = mFlinger.captureScreenshotLegacy(renderAreaBuilder, getLayerSnapshotsFn,
|
|
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
|
|
index 852a8b81e1..067a8bafe0 100644
|
|
--- a/services/surfaceflinger/SurfaceFlinger.cpp
|
|
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
|
|
@@ -7335,10 +7335,9 @@ 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<std::pair<Layer*, sp<LayerFE>>>& layers) const {
|
|
+bool SurfaceFlinger::layersHasProtectedLayer(const std::vector<sp<LayerFE>>& layers) const {
|
|
bool protectedLayerFound = false;
|
|
- for (auto& [_, layerFE] : layers) {
|
|
+ for (auto& layerFE : layers) {
|
|
protectedLayerFound |=
|
|
(layerFE->mSnapshot->isVisible && layerFE->mSnapshot->hasProtectedContent);
|
|
if (protectedLayerFound) {
|
|
@@ -7354,21 +7353,15 @@ bool SurfaceFlinger::layersHasProtectedLayer(
|
|
// risk of deadlocks.
|
|
std::optional<SurfaceFlinger::OutputCompositionState> SurfaceFlinger::getSnapshotsFromMainThread(
|
|
RenderAreaBuilderVariant& renderAreaBuilder, GetLayerSnapshotsFunction getLayerSnapshotsFn,
|
|
- std::vector<std::pair<Layer*, sp<LayerFE>>>& layers) {
|
|
+ std::vector<sp<LayerFE>>& layerFEs) {
|
|
return mScheduler
|
|
- ->schedule([=, this, &renderAreaBuilder, &layers]() REQUIRES(kMainThreadContext) {
|
|
+ ->schedule([=, this, &renderAreaBuilder, &layerFEs]() REQUIRES(kMainThreadContext) {
|
|
SFTRACE_NAME("getSnapshotsFromMainThread");
|
|
- 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);
|
|
- }
|
|
+ auto layers = getLayerSnapshotsFn();
|
|
+ for (auto& [layer, layerFE] : layers) {
|
|
+ attachReleaseFenceFutureToLayer(layer, layerFE.get(), ui::INVALID_LAYER_STACK);
|
|
}
|
|
+ layerFEs = extractLayerFEs(layers);
|
|
return getDisplayStateFromRenderAreaBuilder(renderAreaBuilder);
|
|
})
|
|
.get();
|
|
@@ -7389,15 +7382,15 @@ void SurfaceFlinger::captureScreenCommon(RenderAreaBuilderVariant renderAreaBuil
|
|
return;
|
|
}
|
|
|
|
- if (FlagManager::getInstance().single_hop_screenshot()) {
|
|
- std::vector<std::pair<Layer*, sp<LayerFE>>> layers;
|
|
+ if (FlagManager::getInstance().single_hop_screenshot() && mRenderEngine->isThreaded()) {
|
|
+ std::vector<sp<LayerFE>> layerFEs;
|
|
auto displayState =
|
|
- getSnapshotsFromMainThread(renderAreaBuilder, getLayerSnapshotsFn, layers);
|
|
+ getSnapshotsFromMainThread(renderAreaBuilder, getLayerSnapshotsFn, layerFEs);
|
|
|
|
const bool supportsProtected = getRenderEngine().supportsProtectedContent();
|
|
bool hasProtectedLayer = false;
|
|
if (allowProtected && supportsProtected) {
|
|
- hasProtectedLayer = layersHasProtectedLayer(layers);
|
|
+ hasProtectedLayer = layersHasProtectedLayer(layerFEs);
|
|
}
|
|
const bool isProtected = hasProtectedLayer && allowProtected && supportsProtected;
|
|
const uint32_t usage = GRALLOC_USAGE_HW_COMPOSER | GRALLOC_USAGE_HW_RENDER |
|
|
@@ -7424,7 +7417,7 @@ void SurfaceFlinger::captureScreenCommon(RenderAreaBuilderVariant renderAreaBuil
|
|
WRITEABLE);
|
|
auto futureFence = captureScreenshot(renderAreaBuilder, texture, false /* regionSampling */,
|
|
grayscale, isProtected, attachGainmap, captureListener,
|
|
- displayState, layers);
|
|
+ displayState, layerFEs);
|
|
futureFence.get();
|
|
|
|
} else {
|
|
@@ -7432,7 +7425,7 @@ void SurfaceFlinger::captureScreenCommon(RenderAreaBuilderVariant renderAreaBuil
|
|
bool hasProtectedLayer = false;
|
|
if (allowProtected && supportsProtected) {
|
|
auto layers = mScheduler->schedule([=]() { return getLayerSnapshotsFn(); }).get();
|
|
- hasProtectedLayer = layersHasProtectedLayer(layers);
|
|
+ hasProtectedLayer = layersHasProtectedLayer(extractLayerFEs(layers));
|
|
}
|
|
const bool isProtected = hasProtectedLayer && allowProtected && supportsProtected;
|
|
const uint32_t usage = GRALLOC_USAGE_HW_COMPOSER | GRALLOC_USAGE_HW_RENDER |
|
|
@@ -7500,13 +7493,22 @@ SurfaceFlinger::getDisplayStateFromRenderAreaBuilder(RenderAreaBuilderVariant& r
|
|
return std::nullopt;
|
|
}
|
|
|
|
+std::vector<sp<LayerFE>> SurfaceFlinger::extractLayerFEs(
|
|
+ const std::vector<std::pair<Layer*, sp<LayerFE>>>& layers) const {
|
|
+ std::vector<sp<LayerFE>> layerFEs;
|
|
+ layerFEs.reserve(layers.size());
|
|
+ for (const auto& [_, layerFE] : layers) {
|
|
+ layerFEs.push_back(layerFE);
|
|
+ }
|
|
+ return layerFEs;
|
|
+}
|
|
+
|
|
ftl::SharedFuture<FenceResult> SurfaceFlinger::captureScreenshot(
|
|
const RenderAreaBuilderVariant& renderAreaBuilder,
|
|
const std::shared_ptr<renderengine::ExternalTexture>& buffer, bool regionSampling,
|
|
bool grayscale, bool isProtected, bool attachGainmap,
|
|
const sp<IScreenCaptureListener>& captureListener,
|
|
- std::optional<OutputCompositionState>& displayState,
|
|
- std::vector<std::pair<Layer*, sp<LayerFE>>>& layers) {
|
|
+ std::optional<OutputCompositionState>& displayState, std::vector<sp<LayerFE>>& layerFEs) {
|
|
SFTRACE_CALL();
|
|
|
|
ScreenCaptureResults captureResults;
|
|
@@ -7525,9 +7527,11 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::captureScreenshot(
|
|
float displayBrightnessNits = displayState.value().displayBrightnessNits;
|
|
float sdrWhitePointNits = displayState.value().sdrWhitePointNits;
|
|
|
|
+ // Empty vector needed to pass into renderScreenImpl for legacy path
|
|
+ std::vector<std::pair<Layer*, sp<android::LayerFE>>> layers;
|
|
ftl::SharedFuture<FenceResult> renderFuture =
|
|
renderScreenImpl(renderArea.get(), buffer, regionSampling, grayscale, isProtected,
|
|
- attachGainmap, captureResults, displayState, layers);
|
|
+ attachGainmap, captureResults, displayState, layers, layerFEs);
|
|
|
|
if (captureResults.capturedHdrLayers && attachGainmap &&
|
|
FlagManager::getInstance().true_hdr_screenshots()) {
|
|
@@ -7563,7 +7567,7 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::captureScreenshot(
|
|
ftl::SharedFuture<FenceResult> hdrRenderFuture =
|
|
renderScreenImpl(renderArea.get(), hdrTexture, regionSampling, grayscale,
|
|
isProtected, attachGainmap, unusedResults, displayState,
|
|
- layers);
|
|
+ layers, layerFEs);
|
|
|
|
renderFuture =
|
|
ftl::Future(std::move(renderFuture))
|
|
@@ -7619,6 +7623,9 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::captureScreenshotLegacy(
|
|
auto takeScreenshotFn = [=, this, renderAreaBuilder = std::move(renderAreaBuilder)]() REQUIRES(
|
|
kMainThreadContext) mutable -> ftl::SharedFuture<FenceResult> {
|
|
auto layers = getLayerSnapshotsFn();
|
|
+ for (auto& [layer, layerFE] : layers) {
|
|
+ attachReleaseFenceFutureToLayer(layer, layerFE.get(), ui::INVALID_LAYER_STACK);
|
|
+ }
|
|
auto displayState = getDisplayStateFromRenderAreaBuilder(renderAreaBuilder);
|
|
|
|
ScreenCaptureResults captureResults;
|
|
@@ -7635,9 +7642,10 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::captureScreenshotLegacy(
|
|
return ftl::yield<FenceResult>(base::unexpected(NO_ERROR)).share();
|
|
}
|
|
|
|
+ auto layerFEs = extractLayerFEs(layers);
|
|
ftl::SharedFuture<FenceResult> renderFuture =
|
|
renderScreenImpl(renderArea.get(), buffer, regionSampling, grayscale, isProtected,
|
|
- attachGainmap, captureResults, displayState, layers);
|
|
+ attachGainmap, captureResults, displayState, layers, layerFEs);
|
|
|
|
if (captureListener) {
|
|
// Defer blocking on renderFuture back to the Binder thread.
|
|
@@ -7670,10 +7678,10 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::renderScreenImpl(
|
|
const RenderArea* renderArea, const std::shared_ptr<renderengine::ExternalTexture>& buffer,
|
|
bool regionSampling, bool grayscale, bool isProtected, bool attachGainmap,
|
|
ScreenCaptureResults& captureResults, std::optional<OutputCompositionState>& displayState,
|
|
- std::vector<std::pair<Layer*, sp<LayerFE>>>& layers) {
|
|
+ std::vector<std::pair<Layer*, sp<LayerFE>>>& layers, std::vector<sp<LayerFE>>& layerFEs) {
|
|
SFTRACE_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);
|
|
@@ -7732,32 +7740,29 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::renderScreenImpl(
|
|
captureResults.buffer = capturedBuffer->getBuffer();
|
|
|
|
ui::LayerStack layerStack{ui::DEFAULT_LAYER_STACK};
|
|
- if (!layers.empty()) {
|
|
- const sp<LayerFE>& layerFE = layers.back().second;
|
|
+ if (!layerFEs.empty()) {
|
|
+ const sp<LayerFE>& layerFE = layerFEs.back();
|
|
layerStack = layerFE->getCompositionState()->outputFilter.layerStack;
|
|
}
|
|
|
|
+ auto copyLayerFEs = [&layerFEs]() {
|
|
+ std::vector<sp<compositionengine::LayerFE>> ceLayerFEs;
|
|
+ ceLayerFEs.reserve(layerFEs.size());
|
|
+ for (const auto& layerFE : layerFEs) {
|
|
+ ceLayerFEs.push_back(layerFE);
|
|
+ }
|
|
+ return ceLayerFEs;
|
|
+ };
|
|
+
|
|
auto present = [this, buffer = capturedBuffer, dataspace = captureResults.capturedDataspace,
|
|
sdrWhitePointNits, displayBrightnessNits, grayscale, isProtected,
|
|
- layers = std::move(layers), layerStack, regionSampling,
|
|
+ layerFEs = copyLayerFEs(), layerStack, regionSampling,
|
|
renderArea = std::move(renderArea), renderIntent,
|
|
enableLocalTonemapping]() -> FenceResult {
|
|
std::unique_ptr<compositionengine::CompositionEngine> compositionEngine =
|
|
mFactory.createCompositionEngine();
|
|
compositionEngine->setRenderEngine(mRenderEngine.get());
|
|
|
|
- std::vector<sp<compositionengine::LayerFE>> layerFEs;
|
|
- layerFEs.reserve(layers.size());
|
|
- for (auto& [layer, layerFE] : layers) {
|
|
- // Release fences were not yet added for non-threaded render engine. To avoid
|
|
- // deadlocks between main thread and binder threads waiting for the future fence
|
|
- // result, fences should be added to layers in the same hop onto the main thread.
|
|
- if (!mRenderEngine->isThreaded()) {
|
|
- attachReleaseFenceFutureToLayer(layer, layerFE.get(), ui::INVALID_LAYER_STACK);
|
|
- }
|
|
- layerFEs.push_back(layerFE);
|
|
- }
|
|
-
|
|
compositionengine::Output::ColorProfile colorProfile{.dataspace = dataspace,
|
|
.renderIntent = renderIntent};
|
|
|
|
@@ -7816,10 +7821,8 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::renderScreenImpl(
|
|
// TODO(b/196334700) Once we use RenderEngineThreaded everywhere we can always defer the call
|
|
// to CompositionEngine::present.
|
|
ftl::SharedFuture<FenceResult> presentFuture;
|
|
- if (FlagManager::getInstance().single_hop_screenshot()) {
|
|
- presentFuture = mRenderEngine->isThreaded()
|
|
- ? ftl::yield(present()).share()
|
|
- : mScheduler->schedule(std::move(present)).share();
|
|
+ if (FlagManager::getInstance().single_hop_screenshot() && mRenderEngine->isThreaded()) {
|
|
+ presentFuture = ftl::yield(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 835cb572b4..1df78acbf9 100644
|
|
--- a/services/surfaceflinger/SurfaceFlinger.h
|
|
+++ b/services/surfaceflinger/SurfaceFlinger.h
|
|
@@ -861,14 +861,13 @@ 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<std::pair<Layer*, sp<LayerFE>>>& layers) const;
|
|
+ bool layersHasProtectedLayer(const std::vector<sp<LayerFE>>& layers) const;
|
|
|
|
using OutputCompositionState = compositionengine::impl::OutputCompositionState;
|
|
|
|
std::optional<OutputCompositionState> getSnapshotsFromMainThread(
|
|
RenderAreaBuilderVariant& renderAreaBuilder,
|
|
- GetLayerSnapshotsFunction getLayerSnapshotsFn,
|
|
- std::vector<std::pair<Layer*, sp<LayerFE>>>& layers);
|
|
+ GetLayerSnapshotsFunction getLayerSnapshotsFn, std::vector<sp<LayerFE>>& layerFEs);
|
|
|
|
void captureScreenCommon(RenderAreaBuilderVariant, GetLayerSnapshotsFunction,
|
|
ui::Size bufferSize, ui::PixelFormat, bool allowProtected,
|
|
@@ -877,13 +876,19 @@ private:
|
|
std::optional<OutputCompositionState> getDisplayStateFromRenderAreaBuilder(
|
|
RenderAreaBuilderVariant& renderAreaBuilder) REQUIRES(kMainThreadContext);
|
|
|
|
+ // Legacy layer raw pointer is not safe to access outside the main thread.
|
|
+ // Creates a new vector consisting only of LayerFEs, which can be safely
|
|
+ // accessed outside the main thread.
|
|
+ std::vector<sp<LayerFE>> extractLayerFEs(
|
|
+ const std::vector<std::pair<Layer*, sp<LayerFE>>>& layers) const;
|
|
+
|
|
ftl::SharedFuture<FenceResult> captureScreenshot(
|
|
const RenderAreaBuilderVariant& renderAreaBuilder,
|
|
const std::shared_ptr<renderengine::ExternalTexture>& buffer, bool regionSampling,
|
|
bool grayscale, bool isProtected, bool attachGainmap,
|
|
const sp<IScreenCaptureListener>& captureListener,
|
|
std::optional<OutputCompositionState>& displayState,
|
|
- std::vector<std::pair<Layer*, sp<LayerFE>>>& layers);
|
|
+ std::vector<sp<LayerFE>>& layerFEs);
|
|
|
|
ftl::SharedFuture<FenceResult> captureScreenshotLegacy(
|
|
RenderAreaBuilderVariant, GetLayerSnapshotsFunction,
|
|
@@ -895,7 +900,8 @@ private:
|
|
const RenderArea*, const std::shared_ptr<renderengine::ExternalTexture>&,
|
|
bool regionSampling, bool grayscale, bool isProtected, bool attachGainmap,
|
|
ScreenCaptureResults&, std::optional<OutputCompositionState>& displayState,
|
|
- std::vector<std::pair<Layer*, sp<LayerFE>>>& layers);
|
|
+ std::vector<std::pair<Layer*, sp<LayerFE>>>& layers,
|
|
+ std::vector<sp<LayerFE>>& layerFEs);
|
|
|
|
void readPersistentProperties();
|
|
|
|
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
|
|
index fd8bc0d22d..2ba739b462 100644
|
|
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
|
|
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
|
|
@@ -472,11 +472,12 @@ 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);
|
|
+ layers, layerFEs);
|
|
}
|
|
|
|
auto getLayerSnapshotsForScreenshotsFn(ui::LayerStack layerStack, uint32_t uid) {
|
|
--
|
|
2.48.1
|
|
|