diff options
19 files changed, 201 insertions, 174 deletions
diff --git a/services/surfaceflinger/BufferQueueLayer.cpp b/services/surfaceflinger/BufferQueueLayer.cpp index 926aa1dfb2..6a1a38b39c 100644 --- a/services/surfaceflinger/BufferQueueLayer.cpp +++ b/services/surfaceflinger/BufferQueueLayer.cpp @@ -48,9 +48,8 @@ BufferQueueLayer::~BufferQueueLayer() { // 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 c6e0727806..4587b5e27c 100644 --- a/services/surfaceflinger/BufferQueueLayer.h +++ b/services/surfaceflinger/BufferQueueLayer.h @@ -42,8 +42,7 @@ public: // 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 c5d7a601c5..c88511049b 100644 --- a/services/surfaceflinger/BufferStateLayer.cpp +++ b/services/surfaceflinger/BufferStateLayer.cpp @@ -73,8 +73,7 @@ BufferStateLayer::~BufferStateLayer() { // ----------------------------------------------------------------------- // 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 @@ void BufferStateLayer::onLayerDisplayed( 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 8a696f11b4..cc510d8c74 100644 --- a/services/surfaceflinger/BufferStateLayer.h +++ b/services/surfaceflinger/BufferStateLayer.h @@ -39,8 +39,7 @@ public: // 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 0000000000..0ce263b930 --- /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 e77e155cb8..b7fc62fd21 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 @@ public: 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 ee0c53ded7..871599d2cd 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/LayerFE.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/LayerFE.h @@ -49,7 +49,7 @@ public: 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 4c30f99610..742332ca24 100644 --- a/services/surfaceflinger/CompositionEngine/src/Output.cpp +++ b/services/surfaceflinger/CompositionEngine/src/Output.cpp @@ -1245,34 +1245,31 @@ std::optional<base::unique_fd> Output::composeSurfaces( // 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 @@ void Output::postFramebuffer() { 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 aaca4fe424..d15b0a7646 100644 --- a/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp +++ b/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp @@ -271,13 +271,16 @@ void CachedSet::render(renderengine::RenderEngine& renderEngine, TexturePool& te 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 3a3c91e518..784abeac29 100644 --- a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp +++ b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp @@ -3171,9 +3171,9 @@ TEST_F(OutputPostFramebufferTest, releaseFencesAreSentToLayerFE) { 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 @@ TEST_F(OutputPostFramebufferTest, releaseFencesAreSentToLayerFE) { // 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 @@ TEST_F(OutputPostFramebufferTest, releaseFencesIncludeClientTargetAcquireFence) 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 @@ TEST_F(OutputPostFramebufferTest, releasedLayersSentPresentFence) { 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 @@ TEST_F(OutputPostFramebufferTest, releasedLayersSentPresentFence) { 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 e1eec8b97e..2298f038e9 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -213,8 +213,7 @@ LayerCreationArgs::LayerCreationArgs(SurfaceFlinger* flinger, sp<Client> client, * 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 ecea74413c..100704369e 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -620,8 +620,7 @@ public: 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 e29e6ab05f..2487dbd793 100644 --- a/services/surfaceflinger/RegionSamplingThread.cpp +++ b/services/surfaceflinger/RegionSamplingThread.cpp @@ -355,12 +355,15 @@ void RegionSamplingThread::captureSample() { 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 e72e21ceb3..d8a2696ef0 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -6410,10 +6410,10 @@ status_t SurfaceFlinger::captureDisplay(const DisplayCaptureArgs& args, 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 @@ status_t SurfaceFlinger::captureDisplay(DisplayId displayId, 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 @@ status_t SurfaceFlinger::captureLayers(const LayerCaptureArgs& args, 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 @@ std::shared_future<renderengine::RenderEngineResult> SurfaceFlinger::captureScre 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 @@ std::shared_future<renderengine::RenderEngineResult> SurfaceFlinger::captureScre 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 @@ std::shared_future<renderengine::RenderEngineResult> SurfaceFlinger::captureScre 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 @@ std::shared_future<renderengine::RenderEngineResult> SurfaceFlinger::renderScree // 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 @@ std::shared_future<renderengine::RenderEngineResult> SurfaceFlinger::renderScree 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)); - - std::shared_future<renderengine::RenderEngineResult> drawLayersResultFuture = - drawLayersResult.share(); // drawLayersResult will be moved to shared one + constexpr bool kUseFramebufferCache = false; + std::future<FenceResult> chain = + ftl::chain(getRenderEngine().drawLayers(clientCompositionDisplay, + clientRenderEngineLayers, buffer, + kUseFramebufferCache, std::move(bufferFence))) + .then(&toFenceResult); + 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 c70e1749ff..dc54ac245b 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 @@ private: // 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 e1f348fdba..fa8cffc32a 100644 --- a/services/surfaceflinger/TransactionCallbackInvoker.cpp +++ b/services/surfaceflinger/TransactionCallbackInvoker.cpp @@ -133,10 +133,10 @@ status_t TransactionCallbackInvoker::addCallbackHandle(const sp<CallbackHandle>& 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 @@ status_t TransactionCallbackInvoker::addCallbackHandle(const sp<CallbackHandle>& 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 @@ status_t TransactionCallbackInvoker::addCallbackHandle(const sp<CallbackHandle>& // 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 a68cd87313..b96444dcfb 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 @@ public: 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 46d52dd221..bd8081f863 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 @@ void LayerFuzzer::invokeBufferStateLayer() { 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 c541b9291f..bbfedc7685 100644 --- a/services/surfaceflinger/tests/unittests/CompositionTest.cpp +++ b/services/surfaceflinger/tests/unittests/CompositionTest.cpp @@ -245,15 +245,14 @@ void CompositionTest::captureScreenComposition() { 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); |