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();