SF: Move/Refactor doComposition and doDisplayComposition to CompositionEngine
Test: atest libsurfaceflinger_unittest libcompositionengine_test
Test: atest CtsColorModeTestCases
Test: atest CtsDisplayTestCases
Test: atest CtsGraphicsTestCases
Test: atest CtsUiRenderingTestCases
Test: atest CtsViewTestCases
Test: atest android.media.cts.EncodeVirtualDisplayWithCompositionTest
Bug: 121291683
Change-Id: I6c796ec613ce163764a403bcd669dab38300f437
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h
index df7add2..fa2bb46 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h
@@ -162,11 +162,15 @@
// Performs any debug related screen flashing due to the update
virtual void devOptRepaintFlash(const CompositionRefreshArgs&) = 0;
+ // Finishes the current frame on the output, performing client composition
+ // and ensuring the content is displayed.
+ virtual void finishFrame(const CompositionRefreshArgs&) = 0;
+
// Performs client composition as needed for layers on the output. The
// output fence is set to a fence to signal when client composition is
// finished.
- // Returns false if client composition cannot be performed.
- virtual bool composeSurfaces(const Region& debugFence, base::unique_fd* outReadyFence) = 0;
+ // Returns std::nullopt if client composition cannot be performed.
+ virtual std::optional<base::unique_fd> composeSurfaces(const Region&) = 0;
// Posts the new frame, and sets release fences.
virtual void postFramebuffer() = 0;
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Display.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Display.h
index 36e4aac..bdc1291 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Display.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Display.h
@@ -46,6 +46,7 @@
bool getSkipColorTransform() const override;
compositionengine::Output::FrameFences presentAndGetFrameFences() override;
void setExpensiveRenderingExpected(bool) override;
+ void finishFrame(const compositionengine::CompositionRefreshArgs&) override;
// compositionengine::Display overrides
const std::optional<DisplayId>& getId() const override;
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h
index 36d6a69..b854314 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h
@@ -78,7 +78,8 @@
void beginFrame() override;
void prepareFrame() override;
void devOptRepaintFlash(const compositionengine::CompositionRefreshArgs&) override;
- bool composeSurfaces(const Region&, base::unique_fd*) override;
+ void finishFrame(const compositionengine::CompositionRefreshArgs&) override;
+ std::optional<base::unique_fd> composeSurfaces(const Region&) override;
void postFramebuffer() override;
// Testing
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Output.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Output.h
index b9e1c9c..ade745c 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Output.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Output.h
@@ -80,7 +80,9 @@
MOCK_METHOD1(devOptRepaintFlash, void(const compositionengine::CompositionRefreshArgs&));
- MOCK_METHOD2(composeSurfaces, bool(const Region&, base::unique_fd*));
+ MOCK_METHOD1(finishFrame, void(const compositionengine::CompositionRefreshArgs&));
+
+ MOCK_METHOD1(composeSurfaces, std::optional<base::unique_fd>(const Region&));
MOCK_CONST_METHOD0(getSkipColorTransform, bool());
MOCK_METHOD0(postFramebuffer, void());
diff --git a/services/surfaceflinger/CompositionEngine/src/Display.cpp b/services/surfaceflinger/CompositionEngine/src/Display.cpp
index 6cd392e..cabbc08 100644
--- a/services/surfaceflinger/CompositionEngine/src/Display.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/Display.cpp
@@ -16,6 +16,7 @@
#include <android-base/stringprintf.h>
#include <compositionengine/CompositionEngine.h>
+#include <compositionengine/CompositionRefreshArgs.h>
#include <compositionengine/DisplayCreationArgs.h>
#include <compositionengine/DisplaySurface.h>
#include <compositionengine/impl/Display.h>
@@ -259,4 +260,19 @@
}
}
+void Display::finishFrame(const compositionengine::CompositionRefreshArgs& refreshArgs) {
+ // We only need to actually compose the display if:
+ // 1) It is being handled by hardware composer, which may need this to
+ // keep its virtual display state machine in sync, or
+ // 2) There is work to be done (the dirty region isn't empty)
+ if (!mId) {
+ if (getDirtyRegion(refreshArgs.repaintEverything).isEmpty()) {
+ ALOGV("Skipping display composition");
+ return;
+ }
+ }
+
+ impl::Output::finishFrame(refreshArgs);
+}
+
} // namespace android::compositionengine::impl
diff --git a/services/surfaceflinger/CompositionEngine/src/Output.cpp b/services/surfaceflinger/CompositionEngine/src/Output.cpp
index 10534f4..8b156f8 100644
--- a/services/surfaceflinger/CompositionEngine/src/Output.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/Output.cpp
@@ -313,7 +313,7 @@
if (!dirtyRegion.isEmpty()) {
base::unique_fd readyFence;
// redraw the whole screen
- composeSurfaces(dirtyRegion, &readyFence);
+ static_cast<void>(composeSurfaces(dirtyRegion));
mRenderSurface->queueBuffer(std::move(readyFence));
}
@@ -326,14 +326,35 @@
prepareFrame();
}
-bool Output::composeSurfaces(const Region& debugRegion, base::unique_fd* readyFence) {
+void Output::finishFrame(const compositionengine::CompositionRefreshArgs&) {
+ ATRACE_CALL();
+ ALOGV(__FUNCTION__);
+
+ if (!mState.isEnabled) {
+ return;
+ }
+
+ // Repaint the framebuffer (if needed), getting the optional fence for when
+ // the composition completes.
+ auto optReadyFence = composeSurfaces(Region::INVALID_REGION);
+ if (!optReadyFence) {
+ return;
+ }
+
+ // swap buffers (presentation)
+ mRenderSurface->queueBuffer(std::move(*optReadyFence));
+}
+
+std::optional<base::unique_fd> Output::composeSurfaces(const Region& debugRegion) {
ATRACE_CALL();
ALOGV(__FUNCTION__);
const TracedOrdinal<bool> hasClientComposition = {"hasClientComposition",
mState.usesClientComposition};
+ base::unique_fd readyFence;
+
if (!hasClientComposition) {
- return true;
+ return readyFence;
}
ALOGV("hasClientComposition");
@@ -389,7 +410,7 @@
ALOGW("Dequeuing buffer for display [%s] failed, bailing out of "
"client composition for this frame",
mName.c_str());
- return false;
+ return std::nullopt;
}
// We boost GPU frequency here because there will be color spaces conversion
@@ -404,13 +425,13 @@
renderEngine.drawLayers(clientCompositionDisplay, clientCompositionLayers,
buf->getNativeBuffer(), /*useFramebufferCache=*/true, std::move(fd),
- readyFence);
+ &readyFence);
if (expensiveRenderingExpected) {
setExpensiveRenderingExpected(false);
}
- return true;
+ return readyFence;
}
std::vector<renderengine::LayerSettings> Output::generateClientCompositionRequests(
@@ -507,6 +528,9 @@
return;
}
+ mState.dirtyRegion.clear();
+ mRenderSurface->flip();
+
auto frame = presentAndGetFrameFences();
mRenderSurface->onPresentDisplayCompleted();
diff --git a/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp b/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp
index 743da82..e975260 100644
--- a/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp
@@ -537,5 +537,90 @@
mDisplay.setExpensiveRenderingExpected(false);
}
+/*
+ * Display::finishFrame()
+ */
+
+TEST_F(DisplayTest, finishFrameDoesNotSkipCompositionIfNotDirtyOnHwcDisplay) {
+ mock::RenderSurface* renderSurface = new StrictMock<mock::RenderSurface>();
+ mDisplay.setRenderSurfaceForTest(std::unique_ptr<RenderSurface>(renderSurface));
+
+ // We expect no calls to queueBuffer if composition was skipped.
+ EXPECT_CALL(*renderSurface, queueBuffer(_)).Times(1);
+
+ mDisplay.editState().isEnabled = true;
+ mDisplay.editState().usesClientComposition = false;
+ mDisplay.editState().viewport = Rect(0, 0, 1, 1);
+ mDisplay.editState().dirtyRegion = Region::INVALID_REGION;
+
+ CompositionRefreshArgs refreshArgs;
+ refreshArgs.repaintEverything = false;
+
+ mDisplay.finishFrame(refreshArgs);
+}
+
+TEST_F(DisplayTest, finishFrameSkipsCompositionIfNotDirty) {
+ std::shared_ptr<impl::Display> nonHwcDisplay{
+ impl::createDisplay(mCompositionEngine, DisplayCreationArgsBuilder().build())};
+
+ mock::RenderSurface* renderSurface = new StrictMock<mock::RenderSurface>();
+ nonHwcDisplay->setRenderSurfaceForTest(std::unique_ptr<RenderSurface>(renderSurface));
+
+ // We expect no calls to queueBuffer if composition was skipped.
+ EXPECT_CALL(*renderSurface, queueBuffer(_)).Times(0);
+
+ nonHwcDisplay->editState().isEnabled = true;
+ nonHwcDisplay->editState().usesClientComposition = false;
+ nonHwcDisplay->editState().viewport = Rect(0, 0, 1, 1);
+ nonHwcDisplay->editState().dirtyRegion = Region::INVALID_REGION;
+
+ CompositionRefreshArgs refreshArgs;
+ refreshArgs.repaintEverything = false;
+
+ nonHwcDisplay->finishFrame(refreshArgs);
+}
+
+TEST_F(DisplayTest, finishFramePerformsCompositionIfDirty) {
+ std::shared_ptr<impl::Display> nonHwcDisplay{
+ impl::createDisplay(mCompositionEngine, DisplayCreationArgsBuilder().build())};
+
+ mock::RenderSurface* renderSurface = new StrictMock<mock::RenderSurface>();
+ nonHwcDisplay->setRenderSurfaceForTest(std::unique_ptr<RenderSurface>(renderSurface));
+
+ // We expect a single call to queueBuffer when composition is not skipped.
+ EXPECT_CALL(*renderSurface, queueBuffer(_)).Times(1);
+
+ nonHwcDisplay->editState().isEnabled = true;
+ nonHwcDisplay->editState().usesClientComposition = false;
+ nonHwcDisplay->editState().viewport = Rect(0, 0, 1, 1);
+ nonHwcDisplay->editState().dirtyRegion = Region(Rect(0, 0, 1, 1));
+
+ CompositionRefreshArgs refreshArgs;
+ refreshArgs.repaintEverything = false;
+
+ nonHwcDisplay->finishFrame(refreshArgs);
+}
+
+TEST_F(DisplayTest, finishFramePerformsCompositionIfRepaintEverything) {
+ std::shared_ptr<impl::Display> nonHwcDisplay{
+ impl::createDisplay(mCompositionEngine, DisplayCreationArgsBuilder().build())};
+
+ mock::RenderSurface* renderSurface = new StrictMock<mock::RenderSurface>();
+ nonHwcDisplay->setRenderSurfaceForTest(std::unique_ptr<RenderSurface>(renderSurface));
+
+ // We expect a single call to queueBuffer when composition is not skipped.
+ EXPECT_CALL(*renderSurface, queueBuffer(_)).Times(1);
+
+ nonHwcDisplay->editState().isEnabled = true;
+ nonHwcDisplay->editState().usesClientComposition = false;
+ nonHwcDisplay->editState().viewport = Rect(0, 0, 1, 1);
+ nonHwcDisplay->editState().dirtyRegion = Region::INVALID_REGION;
+
+ CompositionRefreshArgs refreshArgs;
+ refreshArgs.repaintEverything = true;
+
+ nonHwcDisplay->finishFrame(refreshArgs);
+}
+
} // namespace
} // namespace android::compositionengine
diff --git a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp
index f4d2cf1..eddb67f 100644
--- a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp
@@ -527,8 +527,8 @@
mOutput.editState().usesClientComposition = false;
Region debugRegion;
- base::unique_fd readyFence;
- EXPECT_EQ(true, mOutput.composeSurfaces(debugRegion, &readyFence));
+ std::optional<base::unique_fd> readyFence = mOutput.composeSurfaces(debugRegion);
+ EXPECT_TRUE(readyFence);
}
TEST_F(OutputComposeSurfacesTest, worksIfNoClientLayersQueued) {
@@ -556,8 +556,8 @@
EXPECT_CALL(mOutput, setExpensiveRenderingExpected(true)).Times(1);
EXPECT_CALL(mOutput, setExpensiveRenderingExpected(false)).Times(1);
- base::unique_fd readyFence;
- EXPECT_EQ(true, mOutput.composeSurfaces(kDebugRegion, &readyFence));
+ std::optional<base::unique_fd> readyFence = mOutput.composeSurfaces(kDebugRegion);
+ EXPECT_TRUE(readyFence);
}
/*
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index dd0890c..1875151 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -1801,7 +1801,8 @@
display->beginFrame();
display->prepareFrame();
display->devOptRepaintFlash(refreshArgs);
- doComposition(displayDevice, refreshArgs.repaintEverything);
+ display->finishFrame(refreshArgs);
+ display->postFramebuffer();
}
postFrame();
@@ -2321,26 +2322,6 @@
profile->getBestColorMode(bestDataSpace, intent, outDataSpace, outMode, outRenderIntent);
}
-void SurfaceFlinger::doComposition(const sp<DisplayDevice>& displayDevice, bool repaintEverything) {
- ATRACE_CALL();
- ALOGV("doComposition");
-
- auto display = displayDevice->getCompositionDisplay();
- const auto& displayState = display->getState();
-
- if (displayState.isEnabled) {
- // transform the dirty region into this screen's coordinate space
- const Region dirtyRegion = display->getDirtyRegion(repaintEverything);
-
- // repaint the framebuffer (if needed)
- doDisplayComposition(displayDevice, dirtyRegion);
-
- display->editState().dirtyRegion.clear();
- display->getRenderSurface()->flip();
- }
- displayDevice->getCompositionDisplay()->postFramebuffer();
-}
-
void SurfaceFlinger::postFrame()
{
// |mStateLock| not needed as we are on the main thread
@@ -3162,26 +3143,6 @@
mGeometryInvalid = true;
}
-void SurfaceFlinger::doDisplayComposition(const sp<DisplayDevice>& displayDevice,
- const Region& inDirtyRegion) {
- auto display = displayDevice->getCompositionDisplay();
- // We only need to actually compose the display if:
- // 1) It is being handled by hardware composer, which may need this to
- // keep its virtual display state machine in sync, or
- // 2) There is work to be done (the dirty region isn't empty)
- if (!displayDevice->getId() && inDirtyRegion.isEmpty()) {
- ALOGV("Skipping display composition");
- return;
- }
-
- ALOGV("doDisplayComposition");
- base::unique_fd readyFence;
- if (!display->composeSurfaces(Region::INVALID_REGION, &readyFence)) return;
-
- // swap buffers (presentation)
- display->getRenderSurface()->queueBuffer(std::move(readyFence));
-}
-
status_t SurfaceFlinger::addClientLayer(const sp<Client>& client, const sp<IBinder>& handle,
const sp<IGraphicBufferProducer>& gbc, const sp<Layer>& lbc,
const sp<IBinder>& parentHandle,
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index bc57284..9acd975 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -762,8 +762,6 @@
ui::Dataspace* outDataSpace, ui::RenderIntent* outRenderIntent) const;
void calculateWorkingSet();
- void doComposition(const sp<DisplayDevice>& display, bool repainEverything);
- void doDisplayComposition(const sp<DisplayDevice>& display, const Region& dirtyRegion);
void postFrame();