PeterGSI/patches
1
0
Fork 2
Code Issues Pull requests Projects Releases Wiki Activity
patches/frameworks/native/0004-Revert-Reorder-release-fence-attachment-for-non-thre.patch

349 lines
18 KiB
Diff
Raw Permalink Blame History

From 0aeb91404524322d4b1ccd0eeb76e5b2f735691f Mon Sep 17 00:00:00 2001
From: Peter Cai <peter@typeblog.net>
Date: Sun, 15 Jun 2025 18:31:42 -0400
Subject: [PATCH 4/6] Revert "Reorder release fence attachment for non-threaded
RE"
This reverts commit 0b80c74300b73e937ee9a9cb58487bd126daa4d8.
Change-Id: I7974588c1ca5c129b81846fbfb5a3422f4f98cf5
Conflicts:
services/surfaceflinger/RegionSamplingThread.cpp
services/surfaceflinger/SurfaceFlinger.cpp
services/surfaceflinger/SurfaceFlinger.h
services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
---
.../surfaceflinger/RegionSamplingThread.cpp | 9 +-
services/surfaceflinger/SurfaceFlinger.cpp | 104 ++++++++++--------
services/surfaceflinger/SurfaceFlinger.h | 15 ++-
.../tests/unittests/TestableSurfaceFlinger.h | 3 +-
4 files changed, 74 insertions(+), 57 deletions(-)
diff --git a/services/surfaceflinger/RegionSamplingThread.cpp b/services/surfaceflinger/RegionSamplingThread.cpp
index e266656b5d..7e2144d6b1 100644
--- a/services/surfaceflinger/RegionSamplingThread.cpp
+++ b/services/surfaceflinger/RegionSamplingThread.cpp
@@ -356,12 +356,13 @@ void RegionSamplingThread::captureSample() {
screenshotArgs.seamlessTransition = false;
FenceResult fenceResult;
- if (FlagManager::getInstance().single_hop_screenshot()) {
- std::vector<std::pair<Layer*, sp<LayerFE>>> layers;
+ if (FlagManager::getInstance().single_hop_screenshot() &&
+ mFlinger.mRenderEngine->isThreaded()) {
+ std::vector<sp<LayerFE>> layerFEs;
auto displayState =
- mFlinger.getSnapshotsFromMainThread(screenshotArgs, getLayerSnapshotsFn, layers);
+ mFlinger.getSnapshotsFromMainThread(screenshotArgs, getLayerSnapshotsFn, layerFEs);
fenceResult = mFlinger.captureScreenshot(screenshotArgs, buffer, kRegionSampling,
- kGrayscale, kIsProtected, nullptr, layers)
+ kGrayscale, kIsProtected, nullptr, layerFEs)
.get();
} else {
fenceResult = mFlinger.captureScreenshotLegacy(screenshotArgs, getLayerSnapshotsFn, buffer,
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index d601b24cbd..235ddfb762 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -7650,10 +7650,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) {
@@ -7669,23 +7668,17 @@ bool SurfaceFlinger::layersHasProtectedLayer(
// risk of deadlocks. Returns false if no display is found.
bool SurfaceFlinger::getSnapshotsFromMainThread(
ScreenshotArgs& args, GetLayerSnapshotsFunction getLayerSnapshotsFn,
- std::vector<std::pair<Layer*, sp<LayerFE>>>& layers) {
+ std::vector<sp<LayerFE>>& layerFEs) {
return mScheduler
- ->schedule([=, this, &args, &layers]() REQUIRES(kMainThreadContext) {
+ ->schedule([=, this, &args, &layerFEs]() REQUIRES(kMainThreadContext) {
SFTRACE_NAME_FOR_TRACK(WorkloadTracer::TRACK_NAME, "Screenshot");
mPowerAdvisor->setScreenshotWorkload();
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 getDisplayStateOnMainThread(args);
})
.get();
@@ -7706,22 +7699,22 @@ void SurfaceFlinger::captureScreenCommon(ScreenshotArgs& args,
return;
}
- if (FlagManager::getInstance().single_hop_screenshot()) {
- std::vector<std::pair<Layer*, sp<LayerFE>>> layers;
- bool hasDisplayState = getSnapshotsFromMainThread(args, getLayerSnapshotsFn, layers);
+ if (FlagManager::getInstance().single_hop_screenshot() && mRenderEngine->isThreaded()) {
+ std::vector<sp<LayerFE>> layerFEs;
+ bool hasDisplayState = getSnapshotsFromMainThread(args, getLayerSnapshotsFn, layerFEs);
if (!hasDisplayState) {
ALOGD("Display state not found");
invokeScreenCaptureError(NO_MEMORY, captureListener);
}
- const bool hasHdrLayer = std::any_of(layers.cbegin(), layers.cend(), [this](const auto& layer) {
- return isHdrLayer(*(layer.second->mSnapshot.get()));
+ const bool hasHdrLayer = std::any_of(layerFEs.cbegin(), layerFEs.cend(), [this](const auto& layer) {
+ return isHdrLayer(*(layer->mSnapshot.get()));
});
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 |
@@ -7787,14 +7780,14 @@ void SurfaceFlinger::captureScreenCommon(ScreenshotArgs& args,
auto futureFence =
captureScreenshot(args, texture, false /* regionSampling */, grayscale, isProtected,
- captureListener, layers, hdrTexture, gainmapTexture);
+ captureListener, layerFEs, hdrTexture, gainmapTexture);
futureFence.get();
} else {
const bool supportsProtected = getRenderEngine().supportsProtectedContent();
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 |
@@ -7874,11 +7867,21 @@ bool SurfaceFlinger::getDisplayStateOnMainThread(ScreenshotArgs& args) {
return false;
}
+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(
ScreenshotArgs& args, const std::shared_ptr<renderengine::ExternalTexture>& buffer,
bool regionSampling, bool grayscale, bool isProtected,
const sp<IScreenCaptureListener>& captureListener,
- const std::vector<std::pair<Layer*, sp<LayerFE>>>& layers,
+ std::vector<sp<LayerFE>>& layerFEs,
const std::shared_ptr<renderengine::ExternalTexture>& hdrBuffer,
const std::shared_ptr<renderengine::ExternalTexture>& gainmapBuffer) {
SFTRACE_CALL();
@@ -7888,10 +7891,13 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::captureScreenshot(
float hdrSdrRatio = args.displayBrightnessNits / args.sdrWhitePointNits;
+ // Empty vector needed to pass into renderScreenImpl for legacy path
+ std::vector<std::pair<Layer*, sp<android::LayerFE>>> layers;
+
if (hdrBuffer && gainmapBuffer) {
ftl::SharedFuture<FenceResult> hdrRenderFuture =
renderScreenImpl(args, hdrBuffer, regionSampling, grayscale, isProtected,
- true, captureResults, layers);
+ true, captureResults, layers, layerFEs);
captureResults.buffer = buffer->getBuffer();
captureResults.optionalGainMap = gainmapBuffer->getBuffer();
@@ -7913,7 +7919,7 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::captureScreenshot(
.share();
} else {
renderFuture = renderScreenImpl(args, buffer, regionSampling, grayscale, isProtected,
- false, captureResults, layers);
+ false, captureResults, layers, layerFEs);
}
if (captureListener) {
@@ -7942,11 +7948,16 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::captureScreenshotLegacy(
kMainThreadContext) mutable -> ftl::SharedFuture<FenceResult> {
auto layers = getLayerSnapshotsFn();
+ for (auto& [layer, layerFE] : layers) {
+ attachReleaseFenceFutureToLayer(layer, layerFE.get(), ui::INVALID_LAYER_STACK);
+ }
+
ScreenCaptureResults captureResults;
+ auto layerFEs = extractLayerFEs(layers);
ftl::SharedFuture<FenceResult> renderFuture =
renderScreenImpl(args, buffer, regionSampling, grayscale, isProtected,
- attachGainmap, captureResults, layers);
+ attachGainmap, captureResults, layers, layerFEs);
if (captureListener) {
// Defer blocking on renderFuture back to the Binder thread.
@@ -7978,10 +7989,11 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::captureScreenshotLegacy(
ftl::SharedFuture<FenceResult> SurfaceFlinger::renderScreenImpl(
ScreenshotArgs& args, const std::shared_ptr<renderengine::ExternalTexture>& buffer,
bool regionSampling, bool grayscale, bool isProtected, bool attachGainmap,
- ScreenCaptureResults& captureResults, const std::vector<std::pair<Layer*, sp<LayerFE>>>& layers) {
+ ScreenCaptureResults& captureResults, 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);
@@ -8026,31 +8038,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,
- grayscale, isProtected, layers, layerStack, regionSampling, args, renderIntent,
+ grayscale, isProtected, layers, layerFEs = copyLayerFEs(),
+ layerStack, regionSampling, args, renderIntent,
enableLocalTonemapping]() -> FenceResult {
std::unique_ptr<compositionengine::CompositionEngine> compositionEngine =
mFactory.createCompositionEngine();
compositionEngine->setRenderEngine(mRenderEngine.get());
compositionEngine->setHwComposer(mHWComposer.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};
@@ -8120,10 +8130,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 88c1701998..260fe20620 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -872,7 +872,7 @@ 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;
@@ -927,7 +927,7 @@ private:
bool getSnapshotsFromMainThread(ScreenshotArgs& args,
GetLayerSnapshotsFunction getLayerSnapshotsFn,
- std::vector<std::pair<Layer*, sp<LayerFE>>>& layers);
+ std::vector<sp<LayerFE>>& layerFEs);
void captureScreenCommon(ScreenshotArgs& args, GetLayerSnapshotsFunction, ui::Size bufferSize,
ui::PixelFormat, bool allowProtected, bool grayscale,
@@ -935,11 +935,17 @@ private:
bool getDisplayStateOnMainThread(ScreenshotArgs& args) 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(
ScreenshotArgs& args, const std::shared_ptr<renderengine::ExternalTexture>& buffer,
bool regionSampling, bool grayscale, bool isProtected,
const sp<IScreenCaptureListener>& captureListener,
- const std::vector<std::pair<Layer*, sp<LayerFE>>>& layers,
+ std::vector<sp<LayerFE>>& layerFEs,
const std::shared_ptr<renderengine::ExternalTexture>& hdrBuffer = nullptr,
const std::shared_ptr<renderengine::ExternalTexture>& gainmapBuffer = nullptr);
@@ -952,7 +958,8 @@ private:
ftl::SharedFuture<FenceResult> renderScreenImpl(
ScreenshotArgs& args, const std::shared_ptr<renderengine::ExternalTexture>&,
bool regionSampling, bool grayscale, bool isProtected, bool attachGainmap,
- ScreenCaptureResults&, const std::vector<std::pair<Layer*, sp<LayerFE>>>& layers);
+ ScreenCaptureResults&, 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 7e519d3842..062df4d5e2 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -478,6 +478,7 @@ public:
ScreenCaptureResults captureResults;
const auto& state = display->getCompositionDisplay()->getState();
auto layers = getLayerSnapshotsFn();
+ auto layerFEs = mFlinger->extractLayerFEs(layers);
SurfaceFlinger::ScreenshotArgs screenshotArgs;
screenshotArgs.captureTypeVariant = display;
@@ -494,7 +495,7 @@ public:
return mFlinger->renderScreenImpl(screenshotArgs, buffer, regionSampling,
false /* grayscale */, false /* isProtected */,
- false /*attachGainmap */, captureResults, layers);
+ false /*attachGainmap */, captureResults, layers, layerFEs);
}
auto getLayerSnapshotsForScreenshotsFn(ui::LayerStack layerStack, uint32_t uid) {
--
2.48.1
Reference in a new issue View git blame Copy permalink