SF: Do not duplicate fences per layer per frame
Convert the unique_fd of RenderEngineResult (and futures thereof) into
sp<Fence> such that postFramebuffer does not duplicate release/present
fences.
Remove a few copies of shared futures/pointers with std::move.
Bug: 232436803
Test: simpleperf (-33% cycles in sys_dup)
Change-Id: Ia7c6c8333a712441f3612fb5c720ea2932799636
diff --git a/services/surfaceflinger/BufferQueueLayer.cpp b/services/surfaceflinger/BufferQueueLayer.cpp
index 926aa1d..6a1a38b 100644
--- a/services/surfaceflinger/BufferQueueLayer.cpp
+++ b/services/surfaceflinger/BufferQueueLayer.cpp
@@ -48,9 +48,8 @@
// Interface implementation for Layer
// -----------------------------------------------------------------------
-void BufferQueueLayer::onLayerDisplayed(
- std::shared_future<renderengine::RenderEngineResult> futureRenderEngineResult) {
- sp<Fence> releaseFence = new Fence(dup(futureRenderEngineResult.get().drawFence));
+void BufferQueueLayer::onLayerDisplayed(std::shared_future<FenceResult> futureFenceResult) {
+ const sp<Fence> releaseFence = futureFenceResult.get().value_or(Fence::NO_FENCE);
mConsumer->setReleaseFence(releaseFence);
// Prevent tracing the same release multiple times.
diff --git a/services/surfaceflinger/BufferQueueLayer.h b/services/surfaceflinger/BufferQueueLayer.h
index c6e0727..4587b5e 100644
--- a/services/surfaceflinger/BufferQueueLayer.h
+++ b/services/surfaceflinger/BufferQueueLayer.h
@@ -42,8 +42,7 @@
// Implements Layer.
const char* getType() const override { return "BufferQueueLayer"; }
- void onLayerDisplayed(
- std::shared_future<renderengine::RenderEngineResult> futureRenderEngineResult) override;
+ void onLayerDisplayed(std::shared_future<FenceResult>) override;
// If a buffer was replaced this frame, release the former buffer
void releasePendingBuffer(nsecs_t dequeueReadyTime) override;
diff --git a/services/surfaceflinger/BufferStateLayer.cpp b/services/surfaceflinger/BufferStateLayer.cpp
index c5d7a60..c885110 100644
--- a/services/surfaceflinger/BufferStateLayer.cpp
+++ b/services/surfaceflinger/BufferStateLayer.cpp
@@ -73,8 +73,7 @@
// -----------------------------------------------------------------------
// Interface implementation for Layer
// -----------------------------------------------------------------------
-void BufferStateLayer::onLayerDisplayed(
- std::shared_future<renderengine::RenderEngineResult> futureRenderEngineResult) {
+void BufferStateLayer::onLayerDisplayed(std::shared_future<FenceResult> futureFenceResult) {
// If we are displayed on multiple displays in a single composition cycle then we would
// need to do careful tracking to enable the use of the mLastClientCompositionFence.
// For example we can only use it if all the displays are client comp, and we need
@@ -118,7 +117,7 @@
if (ch != nullptr) {
ch->previousReleaseCallbackId = mPreviousReleaseCallbackId;
- ch->previousReleaseFences.emplace_back(futureRenderEngineResult);
+ ch->previousReleaseFences.emplace_back(std::move(futureFenceResult));
ch->name = mName;
}
}
diff --git a/services/surfaceflinger/BufferStateLayer.h b/services/surfaceflinger/BufferStateLayer.h
index 8a696f1..cc510d8 100644
--- a/services/surfaceflinger/BufferStateLayer.h
+++ b/services/surfaceflinger/BufferStateLayer.h
@@ -39,8 +39,7 @@
// Implements Layer.
const char* getType() const override { return "BufferStateLayer"; }
- void onLayerDisplayed(
- std::shared_future<renderengine::RenderEngineResult> futureRenderEngineResult) override;
+ void onLayerDisplayed(std::shared_future<FenceResult>) override;
void releasePendingBuffer(nsecs_t dequeueReadyTime) override;
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/FenceResult.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/FenceResult.h
new file mode 100644
index 0000000..0ce263b
--- /dev/null
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/FenceResult.h
@@ -0,0 +1,49 @@
+/*
+ * Copyright 2022 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 <android-base/expected.h>
+#include <utils/Errors.h>
+#include <utils/StrongPointer.h>
+
+// TODO(b/232535621): Pull this file to <ui/FenceResult.h> so that RenderEngine::drawLayers returns
+// FenceResult rather than RenderEngineResult.
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wconversion"
+#include <renderengine/RenderEngine.h>
+#pragma clang diagnostic pop
+
+namespace android {
+
+class Fence;
+
+using FenceResult = base::expected<sp<Fence>, status_t>;
+
+// TODO(b/232535621): Prevent base::unexpected(NO_ERROR) from being a valid FenceResult.
+inline status_t fenceStatus(const FenceResult& fenceResult) {
+ return fenceResult.ok() ? NO_ERROR : fenceResult.error();
+}
+
+inline FenceResult toFenceResult(renderengine::RenderEngineResult&& result) {
+ if (auto [status, fence] = std::move(result); fence.ok()) {
+ return sp<Fence>::make(std::move(fence));
+ } else {
+ return base::unexpected(status);
+ }
+}
+
+} // namespace android
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFE.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFE.h
index e77e155..b7fc62f 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFE.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFE.h
@@ -21,13 +21,14 @@
#include <ostream>
#include <unordered_set>
+#include <compositionengine/FenceResult.h>
+
// TODO(b/129481165): remove the #pragma below and fix conversion issues
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wconversion"
#pragma clang diagnostic ignored "-Wextra"
#include <renderengine/LayerSettings.h>
-#include <renderengine/RenderEngine.h>
// TODO(b/129481165): remove the #pragma below and fix conversion issues
#pragma clang diagnostic pop // ignored "-Wconversion -Wextra"
@@ -156,7 +157,7 @@
ClientCompositionTargetSettings&) = 0;
// Called after the layer is displayed to update the presentation fence
- virtual void onLayerDisplayed(std::shared_future<renderengine::RenderEngineResult>) = 0;
+ virtual void onLayerDisplayed(std::shared_future<FenceResult>) = 0;
// Gets some kind of identifier for the layer for debug purposes.
virtual const char* getDebugName() const = 0;
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/LayerFE.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/LayerFE.h
index ee0c53d..871599d 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/LayerFE.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/LayerFE.h
@@ -49,7 +49,7 @@
std::vector<compositionengine::LayerFE::LayerSettings>(
compositionengine::LayerFE::ClientCompositionTargetSettings&));
- MOCK_METHOD1(onLayerDisplayed, void(std::shared_future<renderengine::RenderEngineResult>));
+ MOCK_METHOD1(onLayerDisplayed, void(std::shared_future<FenceResult>));
MOCK_CONST_METHOD0(getDebugName, const char*());
MOCK_CONST_METHOD0(getSequence, int32_t());
diff --git a/services/surfaceflinger/CompositionEngine/src/Output.cpp b/services/surfaceflinger/CompositionEngine/src/Output.cpp
index 4c30f99..742332c 100644
--- a/services/surfaceflinger/CompositionEngine/src/Output.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/Output.cpp
@@ -1245,34 +1245,31 @@
// probably to encapsulate the output buffer into a structure that dispatches resource cleanup
// over to RenderEngine, in which case this flag can be removed from the drawLayers interface.
const bool useFramebufferCache = outputState.layerFilter.toInternalDisplay;
- auto [status, drawFence] =
- renderEngine
- .drawLayers(clientCompositionDisplay, clientRenderEngineLayers, tex,
- useFramebufferCache, std::move(fd))
- .get();
- if (status != NO_ERROR && mClientCompositionRequestCache) {
+ auto fenceResult =
+ toFenceResult(renderEngine
+ .drawLayers(clientCompositionDisplay, clientRenderEngineLayers,
+ tex, useFramebufferCache, std::move(fd))
+ .get());
+
+ if (mClientCompositionRequestCache && fenceStatus(fenceResult) != NO_ERROR) {
// If rendering was not successful, remove the request from the cache.
mClientCompositionRequestCache->remove(tex->getBuffer()->getId());
}
- auto& timeStats = getCompositionEngine().getTimeStats();
- if (drawFence.get() < 0) {
- timeStats.recordRenderEngineDuration(renderEngineStart, systemTime());
+ const auto fence = std::move(fenceResult).value_or(Fence::NO_FENCE);
+
+ if (auto& timeStats = getCompositionEngine().getTimeStats(); fence->isValid()) {
+ timeStats.recordRenderEngineDuration(renderEngineStart, std::make_shared<FenceTime>(fence));
} else {
- timeStats.recordRenderEngineDuration(renderEngineStart,
- std::make_shared<FenceTime>(
- new Fence(dup(drawFence.get()))));
+ timeStats.recordRenderEngineDuration(renderEngineStart, systemTime());
}
- if (clientCompositionLayersFE.size() > 0) {
- sp<Fence> clientCompFence = new Fence(dup(drawFence.get()));
- for (auto clientComposedLayer : clientCompositionLayersFE) {
- clientComposedLayer->setWasClientComposed(clientCompFence);
- }
+ for (auto* clientComposedLayer : clientCompositionLayersFE) {
+ clientComposedLayer->setWasClientComposed(fence);
}
- return std::move(drawFence);
+ return base::unique_fd(fence->dup());
}
std::vector<LayerFE::LayerSettings> Output::generateClientCompositionRequests(
@@ -1429,19 +1426,15 @@
Fence::merge("LayerRelease", releaseFence, frame.clientTargetAcquireFence);
}
layer->getLayerFE().onLayerDisplayed(
- ftl::yield<renderengine::RenderEngineResult>(
- {NO_ERROR, base::unique_fd(releaseFence->dup())})
- .share());
+ ftl::yield<FenceResult>(std::move(releaseFence)).share());
}
// We've got a list of layers needing fences, that are disjoint with
// OutputLayersOrderedByZ. The best we can do is to
// supply them with the present fence.
for (auto& weakLayer : mReleasedLayers) {
- if (auto layer = weakLayer.promote(); layer != nullptr) {
- layer->onLayerDisplayed(ftl::yield<renderengine::RenderEngineResult>(
- {NO_ERROR, base::unique_fd(frame.presentFence->dup())})
- .share());
+ if (const auto layer = weakLayer.promote()) {
+ layer->onLayerDisplayed(ftl::yield<FenceResult>(frame.presentFence).share());
}
}
diff --git a/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp b/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp
index aaca4fe..d15b0a7 100644
--- a/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp
@@ -271,13 +271,16 @@
bufferFence.reset(texture->getReadyFence()->dup());
}
- auto [status, drawFence] = renderEngine
- .drawLayers(displaySettings, layerSettings, texture->get(),
- false, std::move(bufferFence))
- .get();
+ constexpr bool kUseFramebufferCache = false;
- if (status == NO_ERROR) {
- mDrawFence = new Fence(drawFence.release());
+ auto fenceResult =
+ toFenceResult(renderEngine
+ .drawLayers(displaySettings, layerSettings, texture->get(),
+ kUseFramebufferCache, std::move(bufferFence))
+ .get());
+
+ if (fenceStatus(fenceResult) == NO_ERROR) {
+ mDrawFence = std::move(fenceResult).value_or(Fence::NO_FENCE);
mOutputSpace = outputState.framebufferSpace;
mTexture = texture;
mTexture->setReadyFence(mDrawFence);
diff --git a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp
index 3a3c91e..784abea 100644
--- a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp
@@ -3171,9 +3171,9 @@
mOutput.mState.isEnabled = true;
// Create three unique fence instances
- sp<Fence> layer1Fence = new Fence();
- sp<Fence> layer2Fence = new Fence();
- sp<Fence> layer3Fence = new Fence();
+ sp<Fence> layer1Fence = sp<Fence>::make();
+ sp<Fence> layer2Fence = sp<Fence>::make();
+ sp<Fence> layer3Fence = sp<Fence>::make();
Output::FrameFences frameFences;
frameFences.layerFences.emplace(&mLayer1.hwc2Layer, layer1Fence);
@@ -3188,23 +3188,17 @@
// are passed. This happens to work with the current implementation, but
// would not survive certain calls like Fence::merge() which would return a
// new instance.
- base::unique_fd layer1FD(layer1Fence->dup());
- base::unique_fd layer2FD(layer2Fence->dup());
- base::unique_fd layer3FD(layer3Fence->dup());
EXPECT_CALL(*mLayer1.layerFE, onLayerDisplayed(_))
- .WillOnce([&layer1FD](std::shared_future<renderengine::RenderEngineResult>
- futureRenderEngineResult) {
- EXPECT_EQ(layer1FD, futureRenderEngineResult.get().drawFence);
+ .WillOnce([&layer1Fence](std::shared_future<FenceResult> futureFenceResult) {
+ EXPECT_EQ(FenceResult(layer1Fence), futureFenceResult.get());
});
EXPECT_CALL(*mLayer2.layerFE, onLayerDisplayed(_))
- .WillOnce([&layer2FD](std::shared_future<renderengine::RenderEngineResult>
- futureRenderEngineResult) {
- EXPECT_EQ(layer2FD, futureRenderEngineResult.get().drawFence);
+ .WillOnce([&layer2Fence](std::shared_future<FenceResult> futureFenceResult) {
+ EXPECT_EQ(FenceResult(layer2Fence), futureFenceResult.get());
});
EXPECT_CALL(*mLayer3.layerFE, onLayerDisplayed(_))
- .WillOnce([&layer3FD](std::shared_future<renderengine::RenderEngineResult>
- futureRenderEngineResult) {
- EXPECT_EQ(layer3FD, futureRenderEngineResult.get().drawFence);
+ .WillOnce([&layer3Fence](std::shared_future<FenceResult> futureFenceResult) {
+ EXPECT_EQ(FenceResult(layer3Fence), futureFenceResult.get());
});
mOutput.postFramebuffer();
@@ -3214,15 +3208,11 @@
mOutput.mState.isEnabled = true;
mOutput.mState.usesClientComposition = true;
- sp<Fence> clientTargetAcquireFence = new Fence();
- sp<Fence> layer1Fence = new Fence();
- sp<Fence> layer2Fence = new Fence();
- sp<Fence> layer3Fence = new Fence();
Output::FrameFences frameFences;
- frameFences.clientTargetAcquireFence = clientTargetAcquireFence;
- frameFences.layerFences.emplace(&mLayer1.hwc2Layer, layer1Fence);
- frameFences.layerFences.emplace(&mLayer2.hwc2Layer, layer2Fence);
- frameFences.layerFences.emplace(&mLayer3.hwc2Layer, layer3Fence);
+ frameFences.clientTargetAcquireFence = sp<Fence>::make();
+ frameFences.layerFences.emplace(&mLayer1.hwc2Layer, sp<Fence>::make());
+ frameFences.layerFences.emplace(&mLayer2.hwc2Layer, sp<Fence>::make());
+ frameFences.layerFences.emplace(&mLayer3.hwc2Layer, sp<Fence>::make());
EXPECT_CALL(*mRenderSurface, flip());
EXPECT_CALL(mOutput, presentAndGetFrameFences()).WillOnce(Return(frameFences));
@@ -3256,7 +3246,7 @@
mOutput.setReleasedLayers(std::move(layers));
// Set up a fake present fence
- sp<Fence> presentFence = new Fence();
+ sp<Fence> presentFence = sp<Fence>::make();
Output::FrameFences frameFences;
frameFences.presentFence = presentFence;
@@ -3265,21 +3255,17 @@
EXPECT_CALL(*mRenderSurface, onPresentDisplayCompleted());
// Each released layer should be given the presentFence.
- base::unique_fd layerFD(presentFence.get()->dup());
EXPECT_CALL(*releasedLayer1, onLayerDisplayed(_))
- .WillOnce([&layerFD](std::shared_future<renderengine::RenderEngineResult>
- futureRenderEngineResult) {
- EXPECT_EQ(layerFD, futureRenderEngineResult.get().drawFence);
+ .WillOnce([&presentFence](std::shared_future<FenceResult> futureFenceResult) {
+ EXPECT_EQ(FenceResult(presentFence), futureFenceResult.get());
});
EXPECT_CALL(*releasedLayer2, onLayerDisplayed(_))
- .WillOnce([&layerFD](std::shared_future<renderengine::RenderEngineResult>
- futureRenderEngineResult) {
- EXPECT_EQ(layerFD, futureRenderEngineResult.get().drawFence);
+ .WillOnce([&presentFence](std::shared_future<FenceResult> futureFenceResult) {
+ EXPECT_EQ(FenceResult(presentFence), futureFenceResult.get());
});
EXPECT_CALL(*releasedLayer3, onLayerDisplayed(_))
- .WillOnce([&layerFD](std::shared_future<renderengine::RenderEngineResult>
- futureRenderEngineResult) {
- EXPECT_EQ(layerFD, futureRenderEngineResult.get().drawFence);
+ .WillOnce([&presentFence](std::shared_future<FenceResult> futureFenceResult) {
+ EXPECT_EQ(FenceResult(presentFence), futureFenceResult.get());
});
mOutput.postFramebuffer();
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index e1eec8b..2298f03 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -213,8 +213,7 @@
* Layer. So, the implementation is done in BufferLayer. When called on a
* EffectLayer object, it's essentially a NOP.
*/
-void Layer::onLayerDisplayed(
- std::shared_future<renderengine::RenderEngineResult> /*futureRenderEngineResult*/) {}
+void Layer::onLayerDisplayed(std::shared_future<FenceResult>) {}
void Layer::removeRelativeZ(const std::vector<Layer*>& layersInTree) {
if (mDrawingState.zOrderRelativeOf == nullptr) {
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index ecea744..1007043 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -620,8 +620,7 @@
void prepareCompositionState(compositionengine::LayerFE::StateSubset subset) override;
std::vector<compositionengine::LayerFE::LayerSettings> prepareClientCompositionList(
compositionengine::LayerFE::ClientCompositionTargetSettings&) override;
- void onLayerDisplayed(
- std::shared_future<renderengine::RenderEngineResult> futureRenderEngineResult) override;
+ void onLayerDisplayed(std::shared_future<FenceResult>) override;
void setWasClientComposed(const sp<Fence>& fence) override {
mLastClientCompositionFence = fence;
diff --git a/services/surfaceflinger/RegionSamplingThread.cpp b/services/surfaceflinger/RegionSamplingThread.cpp
index e29e6ab..2487dbd 100644
--- a/services/surfaceflinger/RegionSamplingThread.cpp
+++ b/services/surfaceflinger/RegionSamplingThread.cpp
@@ -355,12 +355,15 @@
WRITEABLE);
}
- auto captureScreenResultFuture =
- mFlinger.captureScreenCommon(std::move(renderAreaFuture), traverseLayers, buffer,
- true /* regionSampling */, false /* grayscale */, nullptr);
- auto& captureScreenResult = captureScreenResultFuture.get();
- if (captureScreenResult.drawFence.ok()) {
- sync_wait(captureScreenResult.drawFence.get(), -1);
+ constexpr bool kRegionSampling = true;
+ constexpr bool kGrayscale = false;
+
+ if (const auto fenceResult =
+ mFlinger.captureScreenCommon(std::move(renderAreaFuture), traverseLayers, buffer,
+ kRegionSampling, kGrayscale, nullptr)
+ .get();
+ fenceResult.ok()) {
+ fenceResult.value()->waitForever(LOG_TAG);
}
std::vector<Descriptor> activeDescriptors;
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index e72e21c..d8a2696 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -6410,10 +6410,10 @@
traverseLayersInLayerStack(layerStack, args.uid, visitor);
};
- auto captureResultFuture = captureScreenCommon(std::move(renderAreaFuture), traverseLayers,
- reqSize, args.pixelFormat, args.allowProtected,
- args.grayscale, captureListener);
- return captureResultFuture.get().status;
+ auto future = captureScreenCommon(std::move(renderAreaFuture), traverseLayers, reqSize,
+ args.pixelFormat, args.allowProtected, args.grayscale,
+ captureListener);
+ return fenceStatus(future.get());
}
status_t SurfaceFlinger::captureDisplay(DisplayId displayId,
@@ -6452,11 +6452,14 @@
ALOGE("capture screen must provide a capture listener callback");
return BAD_VALUE;
}
- auto captureResultFuture =
- captureScreenCommon(std::move(renderAreaFuture), traverseLayers, size,
- ui::PixelFormat::RGBA_8888, false /* allowProtected */,
- false /* grayscale */, captureListener);
- return captureResultFuture.get().status;
+
+ constexpr bool kAllowProtected = false;
+ constexpr bool kGrayscale = false;
+
+ auto future = captureScreenCommon(std::move(renderAreaFuture), traverseLayers, size,
+ ui::PixelFormat::RGBA_8888, kAllowProtected, kGrayscale,
+ captureListener);
+ return fenceStatus(future.get());
}
status_t SurfaceFlinger::captureLayers(const LayerCaptureArgs& args,
@@ -6568,13 +6571,13 @@
return BAD_VALUE;
}
- auto captureResultFuture = captureScreenCommon(std::move(renderAreaFuture), traverseLayers,
- reqSize, args.pixelFormat, args.allowProtected,
- args.grayscale, captureListener);
- return captureResultFuture.get().status;
+ auto future = captureScreenCommon(std::move(renderAreaFuture), traverseLayers, reqSize,
+ args.pixelFormat, args.allowProtected, args.grayscale,
+ captureListener);
+ return fenceStatus(future.get());
}
-std::shared_future<renderengine::RenderEngineResult> SurfaceFlinger::captureScreenCommon(
+std::shared_future<FenceResult> SurfaceFlinger::captureScreenCommon(
RenderAreaFuture renderAreaFuture, TraverseLayersFunction traverseLayers,
ui::Size bufferSize, ui::PixelFormat reqPixelFormat, bool allowProtected, bool grayscale,
const sp<IScreenCaptureListener>& captureListener) {
@@ -6584,7 +6587,7 @@
ALOGE("Attempted to capture screen with size (%" PRId32 ", %" PRId32
") that exceeds render target size limit.",
bufferSize.getWidth(), bufferSize.getHeight());
- return ftl::yield<renderengine::RenderEngineResult>({BAD_VALUE, base::unique_fd()}).share();
+ return ftl::yield<FenceResult>(base::unexpected(BAD_VALUE)).share();
}
// Loop over all visible layers to see whether there's any protected layer. A protected layer is
@@ -6626,7 +6629,7 @@
false /* regionSampling */, grayscale, captureListener);
}
-std::shared_future<renderengine::RenderEngineResult> SurfaceFlinger::captureScreenCommon(
+std::shared_future<FenceResult> SurfaceFlinger::captureScreenCommon(
RenderAreaFuture renderAreaFuture, TraverseLayersFunction traverseLayers,
const std::shared_ptr<renderengine::ExternalTexture>& buffer, bool regionSampling,
bool grayscale, const sp<IScreenCaptureListener>& captureListener) {
@@ -6634,59 +6637,53 @@
bool canCaptureBlackoutContent = hasCaptureBlackoutContentPermission();
- auto scheduleResultFuture = mScheduler->schedule([=,
- renderAreaFuture =
- std::move(renderAreaFuture)]() mutable
- -> std::shared_future<
- renderengine::RenderEngineResult> {
+ auto future = mScheduler->schedule([=, renderAreaFuture = std::move(renderAreaFuture)]() mutable
+ -> std::shared_future<FenceResult> {
ScreenCaptureResults captureResults;
std::unique_ptr<RenderArea> renderArea = renderAreaFuture.get();
if (!renderArea) {
ALOGW("Skipping screen capture because of invalid render area.");
captureResults.result = NO_MEMORY;
captureListener->onScreenCaptureCompleted(captureResults);
- return ftl::yield<renderengine::RenderEngineResult>({NO_ERROR, base::unique_fd()})
- .share();
+ return ftl::yield<FenceResult>(base::unexpected(NO_ERROR)).share();
}
- std::shared_future<renderengine::RenderEngineResult> renderEngineResultFuture;
-
+ std::shared_future<FenceResult> renderFuture;
renderArea->render([&] {
- renderEngineResultFuture =
- renderScreenImpl(*renderArea, traverseLayers, buffer,
- canCaptureBlackoutContent, regionSampling, grayscale,
- captureResults);
+ renderFuture =
+ renderScreenImpl(*renderArea, traverseLayers, buffer, canCaptureBlackoutContent,
+ regionSampling, grayscale, captureResults);
});
- // spring up a thread to unblock SF main thread and wait for
- // RenderEngineResult to be available
- if (captureListener != nullptr) {
+
+ if (captureListener) {
+ // TODO: The future returned by std::async blocks the main thread. Return a chain of
+ // futures to the Binder thread instead.
std::async([=]() mutable {
ATRACE_NAME("captureListener is nonnull!");
- auto& [status, drawFence] = renderEngineResultFuture.get();
- captureResults.result = status;
- captureResults.fence = new Fence(dup(drawFence));
+ auto fenceResult = renderFuture.get();
+ // TODO(b/232535621): Change ScreenCaptureResults to store a FenceResult.
+ captureResults.result = fenceStatus(fenceResult);
+ captureResults.fence = std::move(fenceResult).value_or(Fence::NO_FENCE);
captureListener->onScreenCaptureCompleted(captureResults);
});
}
- return renderEngineResultFuture;
+ return renderFuture;
});
- // flatten scheduleResultFuture object to single shared_future object
- if (captureListener == nullptr) {
- std::future<renderengine::RenderEngineResult> captureScreenResultFuture =
- ftl::chain(std::move(scheduleResultFuture))
- .then([=](std::shared_future<renderengine::RenderEngineResult> futureObject)
- -> renderengine::RenderEngineResult {
- auto& [status, drawFence] = futureObject.get();
- return {status, base::unique_fd(dup(drawFence))};
- });
- return captureScreenResultFuture.share();
- } else {
- return ftl::yield<renderengine::RenderEngineResult>({NO_ERROR, base::unique_fd()}).share();
+ if (captureListener) {
+ return ftl::yield<FenceResult>(base::unexpected(NO_ERROR)).share();
}
+
+ // Flatten nested futures.
+ std::future<FenceResult> chain =
+ ftl::chain(std::move(future)).then([](std::shared_future<FenceResult> future) {
+ return future.get();
+ });
+
+ return chain.share();
}
-std::shared_future<renderengine::RenderEngineResult> SurfaceFlinger::renderScreenImpl(
+std::shared_future<FenceResult> SurfaceFlinger::renderScreenImpl(
const RenderArea& renderArea, TraverseLayersFunction traverseLayers,
const std::shared_ptr<renderengine::ExternalTexture>& buffer,
bool canCaptureBlackoutContent, bool regionSampling, bool grayscale,
@@ -6705,8 +6702,7 @@
// the impetus on WindowManager to not persist them.
if (captureResults.capturedSecureLayers && !canCaptureBlackoutContent) {
ALOGW("FB is protected: PERMISSION_DENIED");
- return ftl::yield<renderengine::RenderEngineResult>({PERMISSION_DENIED, base::unique_fd()})
- .share();
+ return ftl::yield<FenceResult>(base::unexpected(PERMISSION_DENIED)).share();
}
captureResults.buffer = buffer->getBuffer();
@@ -6827,23 +6823,22 @@
base::unique_fd bufferFence;
getRenderEngine().useProtectedContext(useProtected);
- const constexpr bool kUseFramebufferCache = false;
- std::future<renderengine::RenderEngineResult> drawLayersResult =
- getRenderEngine().drawLayers(clientCompositionDisplay, clientRenderEngineLayers, buffer,
- kUseFramebufferCache, std::move(bufferFence));
+ constexpr bool kUseFramebufferCache = false;
+ std::future<FenceResult> chain =
+ ftl::chain(getRenderEngine().drawLayers(clientCompositionDisplay,
+ clientRenderEngineLayers, buffer,
+ kUseFramebufferCache, std::move(bufferFence)))
+ .then(&toFenceResult);
- std::shared_future<renderengine::RenderEngineResult> drawLayersResultFuture =
- drawLayersResult.share(); // drawLayersResult will be moved to shared one
-
+ const auto future = chain.share();
for (auto* layer : renderedLayers) {
- // make a copy of shared_future object for each layer
- layer->onLayerDisplayed(drawLayersResultFuture);
+ layer->onLayerDisplayed(future);
}
// Always switch back to unprotected context.
getRenderEngine().useProtectedContext(false);
- return drawLayersResultFuture;
+ return future;
}
void SurfaceFlinger::windowInfosReported() {
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index c70e174..dc54ac2 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -49,6 +49,7 @@
#include <utils/Trace.h>
#include <utils/threads.h>
+#include <compositionengine/FenceResult.h>
#include <compositionengine/OutputColorSetting.h>
#include <scheduler/Fps.h>
@@ -867,14 +868,15 @@
// Boot animation, on/off animations and screen capture
void startBootAnim();
- std::shared_future<renderengine::RenderEngineResult> captureScreenCommon(
- RenderAreaFuture, TraverseLayersFunction, ui::Size bufferSize, ui::PixelFormat,
- bool allowProtected, bool grayscale, const sp<IScreenCaptureListener>&);
- std::shared_future<renderengine::RenderEngineResult> captureScreenCommon(
+ std::shared_future<FenceResult> captureScreenCommon(RenderAreaFuture, TraverseLayersFunction,
+ ui::Size bufferSize, ui::PixelFormat,
+ bool allowProtected, bool grayscale,
+ const sp<IScreenCaptureListener>&);
+ std::shared_future<FenceResult> captureScreenCommon(
RenderAreaFuture, TraverseLayersFunction,
const std::shared_ptr<renderengine::ExternalTexture>&, bool regionSampling,
bool grayscale, const sp<IScreenCaptureListener>&);
- std::shared_future<renderengine::RenderEngineResult> renderScreenImpl(
+ std::shared_future<FenceResult> renderScreenImpl(
const RenderArea&, TraverseLayersFunction,
const std::shared_ptr<renderengine::ExternalTexture>&, bool canCaptureBlackoutContent,
bool regionSampling, bool grayscale, ScreenCaptureResults&) EXCLUDES(mStateLock);
diff --git a/services/surfaceflinger/TransactionCallbackInvoker.cpp b/services/surfaceflinger/TransactionCallbackInvoker.cpp
index e1f348f..fa8cffc 100644
--- a/services/surfaceflinger/TransactionCallbackInvoker.cpp
+++ b/services/surfaceflinger/TransactionCallbackInvoker.cpp
@@ -133,10 +133,10 @@
if (surfaceControl) {
sp<Fence> prevFence = nullptr;
- for (const auto& futureStruct : handle->previousReleaseFences) {
- sp<Fence> currentFence = sp<Fence>::make(dup(futureStruct.get().drawFence));
+ for (const auto& future : handle->previousReleaseFences) {
+ sp<Fence> currentFence = future.get().value_or(Fence::NO_FENCE);
if (prevFence == nullptr && currentFence->getStatus() != Fence::Status::Invalid) {
- prevFence = currentFence;
+ prevFence = std::move(currentFence);
handle->previousReleaseFence = prevFence;
} else if (prevFence != nullptr) {
// If both fences are signaled or both are unsignaled, we need to merge
@@ -147,7 +147,7 @@
snprintf(fenceName, 32, "%.28s", handle->name.c_str());
sp<Fence> mergedFence = Fence::merge(fenceName, prevFence, currentFence);
if (mergedFence->isValid()) {
- handle->previousReleaseFence = mergedFence;
+ handle->previousReleaseFence = std::move(mergedFence);
prevFence = handle->previousReleaseFence;
}
} else if (currentFence->getStatus() == Fence::Status::Unsignaled) {
@@ -158,11 +158,12 @@
// by this point, they will have both signaled and only the timestamp
// will be slightly off; any dependencies after this point will
// already have been met.
- handle->previousReleaseFence = currentFence;
+ handle->previousReleaseFence = std::move(currentFence);
}
}
}
- handle->previousReleaseFences = {};
+ handle->previousReleaseFences.clear();
+
FrameEventHistoryStats eventStats(handle->frameNumber,
handle->gpuCompositionDoneFence->getSnapshot().fence,
handle->compositorTiming, handle->refreshStartTime,
diff --git a/services/surfaceflinger/TransactionCallbackInvoker.h b/services/surfaceflinger/TransactionCallbackInvoker.h
index a68cd87..b96444d 100644
--- a/services/surfaceflinger/TransactionCallbackInvoker.h
+++ b/services/surfaceflinger/TransactionCallbackInvoker.h
@@ -28,8 +28,8 @@
#include <android-base/thread_annotations.h>
#include <binder/IBinder.h>
+#include <compositionengine/FenceResult.h>
#include <gui/ITransactionCompletedListener.h>
-#include <renderengine/RenderEngine.h>
#include <ui/Fence.h>
namespace android {
@@ -46,7 +46,7 @@
bool releasePreviousBuffer = false;
std::string name;
sp<Fence> previousReleaseFence;
- std::vector<std::shared_future<renderengine::RenderEngineResult>> previousReleaseFences;
+ std::vector<std::shared_future<FenceResult>> previousReleaseFences;
std::variant<nsecs_t, sp<Fence>> acquireTimeOrFence = -1;
nsecs_t latchTime = -1;
uint32_t transformHint = 0;
diff --git a/services/surfaceflinger/fuzzer/surfaceflinger_layer_fuzzer.cpp b/services/surfaceflinger/fuzzer/surfaceflinger_layer_fuzzer.cpp
index 46d52dd..bd8081f 100644
--- a/services/surfaceflinger/fuzzer/surfaceflinger_layer_fuzzer.cpp
+++ b/services/surfaceflinger/fuzzer/surfaceflinger_layer_fuzzer.cpp
@@ -21,6 +21,7 @@
#include <LayerRejecter.h>
#include <LayerRenderArea.h>
#include <MonitoredProducer.h>
+#include <ftl/future.h>
#include <fuzzer/FuzzedDataProvider.h>
#include <gui/IProducerListener.h>
#include <gui/LayerDebugInfo.h>
@@ -117,11 +118,11 @@
const CompositorTiming compositor = {mFdp.ConsumeIntegral<int64_t>(),
mFdp.ConsumeIntegral<int64_t>(),
mFdp.ConsumeIntegral<int64_t>()};
- std::packaged_task<renderengine::RenderEngineResult()> renderResult([&] {
- return renderengine::RenderEngineResult{mFdp.ConsumeIntegral<int32_t>(),
- base::unique_fd(fence->get())};
- });
- layer->onLayerDisplayed(renderResult.get_future());
+
+ layer->onLayerDisplayed(ftl::yield<FenceResult>(fence).share());
+ layer->onLayerDisplayed(
+ ftl::yield<FenceResult>(base::unexpected(mFdp.ConsumeIntegral<status_t>())).share());
+
layer->releasePendingBuffer(mFdp.ConsumeIntegral<int64_t>());
layer->finalizeFrameEventHistory(fenceTime, compositor);
layer->onPostComposition(nullptr, fenceTime, fenceTime, compositor);
diff --git a/services/surfaceflinger/tests/unittests/CompositionTest.cpp b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
index c541b92..bbfedc7 100644
--- a/services/surfaceflinger/tests/unittests/CompositionTest.cpp
+++ b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
@@ -245,15 +245,14 @@
HAL_PIXEL_FORMAT_RGBA_8888, 1,
usage);
- auto result = mFlinger.renderScreenImpl(*renderArea, traverseLayers, mCaptureScreenBuffer,
+ auto future = mFlinger.renderScreenImpl(*renderArea, traverseLayers, mCaptureScreenBuffer,
forSystem, regionSampling);
- EXPECT_TRUE(result.valid());
+ ASSERT_TRUE(future.valid());
+ const auto fenceResult = future.get();
- auto& [status, drawFence] = result.get();
-
- EXPECT_EQ(NO_ERROR, status);
- if (drawFence.ok()) {
- sync_wait(drawFence.get(), -1);
+ EXPECT_EQ(NO_ERROR, fenceStatus(fenceResult));
+ if (fenceResult.ok()) {
+ fenceResult.value()->waitForever(LOG_TAG);
}
LayerCase::cleanup(this);