CE: Must dequeue a buffer if flipClientTarget requested

To match the logic that always queues a buffer if the HWC has a
flipClientTarget request for the current frame, we must also always
dequeue a buffer, even if no client composition is being performed.

This is an port of an (as yet uncommitted) AOSP change with a ChangeId of
I933cbae2f09f81eef6555b1bb1e5991d2c450930, due to the underlying code
having been refactored.

This version also adds and updates unit tests which were added as part
of the refactoring.

Bug: 151698217
Test: libcompositionengine_test
Test: libsurfaceflinger_test
Change-Id: Ia8a1470affb2596b27986cc4153417f48cf4ed1c

services/surfaceflinger/CompositionEngine/src/Output.cpp

@@ -815,6 +815,43 @@ std::optional<base::unique_fd> Output::composeSurfaces(
     OutputCompositionState& outputCompositionState = editState();
     const TracedOrdinal<bool> hasClientComposition = {"hasClientComposition",
                                                       outputState.usesClientComposition};
+
+    auto& renderEngine = getCompositionEngine().getRenderEngine();
+    const bool supportsProtectedContent = renderEngine.supportsProtectedContent();
+
+    // If we the display is secure, protected content support is enabled, and at
+    // least one layer has protected content, we need to use a secure back
+    // buffer.
+    if (outputState.isSecure && supportsProtectedContent) {
+        auto layers = getOutputLayersOrderedByZ();
+        bool needsProtected = std::any_of(layers.begin(), layers.end(), [](auto* layer) {
+            return layer->getLayerFE().getCompositionState()->hasProtectedContent;
+        });
+        if (needsProtected != renderEngine.isProtected()) {
+            renderEngine.useProtectedContext(needsProtected);
+        }
+        if (needsProtected != mRenderSurface->isProtected() &&
+            needsProtected == renderEngine.isProtected()) {
+            mRenderSurface->setProtected(needsProtected);
+        }
+    }
+
+    base::unique_fd fd;
+    sp<GraphicBuffer> buf;
+
+    // If we aren't doing client composition on this output, but do have a
+    // flipClientTarget request for this frame on this output, we still need to
+    // dequeue a buffer.
+    if (hasClientComposition || outputState.flipClientTarget) {
+        buf = mRenderSurface->dequeueBuffer(&fd);
+        if (buf == nullptr) {
+            ALOGW("Dequeuing buffer for display [%s] failed, bailing out of "
+                  "client composition for this frame",
+                  mName.c_str());
+            return {};
+        }
+    }
+
     base::unique_fd readyFence;
     if (!hasClientComposition) {
         setExpensiveRenderingExpected(false);
@@ -823,9 +860,6 @@ std::optional<base::unique_fd> Output::composeSurfaces(
 
     ALOGV("hasClientComposition");
 
-    auto& renderEngine = getCompositionEngine().getRenderEngine();
-    const bool supportsProtectedContent = renderEngine.supportsProtectedContent();
-
     renderengine::DisplaySettings clientCompositionDisplay;
     clientCompositionDisplay.physicalDisplay = outputState.destinationClip;
     clientCompositionDisplay.clip = outputState.sourceClip;
@@ -851,32 +885,6 @@ std::optional<base::unique_fd> Output::composeSurfaces(
                                               clientCompositionDisplay.outputDataspace);
     appendRegionFlashRequests(debugRegion, clientCompositionLayers);
 
-    // If we the display is secure, protected content support is enabled, and at
-    // least one layer has protected content, we need to use a secure back
-    // buffer.
-    if (outputState.isSecure && supportsProtectedContent) {
-        auto layers = getOutputLayersOrderedByZ();
-        bool needsProtected = std::any_of(layers.begin(), layers.end(), [](auto* layer) {
-            return layer->getLayerFE().getCompositionState()->hasProtectedContent;
-        });
-        if (needsProtected != renderEngine.isProtected()) {
-            renderEngine.useProtectedContext(needsProtected);
-        }
-        if (needsProtected != mRenderSurface->isProtected() &&
-            needsProtected == renderEngine.isProtected()) {
-            mRenderSurface->setProtected(needsProtected);
-        }
-    }
-
-    base::unique_fd fd;
-    sp<GraphicBuffer> buf = mRenderSurface->dequeueBuffer(&fd);
-    if (buf == nullptr) {
-        ALOGW("Dequeuing buffer for display [%s] failed, bailing out of "
-              "client composition for this frame",
-              mName.c_str());
-        return std::nullopt;
-    }
-
     // Check if the client composition requests were rendered into the provided graphic buffer. If
     // so, we can reuse the buffer and avoid client composition.
     if (mClientCompositionRequestCache) {

services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp

@@ -30,6 +30,7 @@
 #include <compositionengine/mock/OutputLayer.h>
 #include <compositionengine/mock/RenderSurface.h>
 #include <gtest/gtest.h>
+#include <renderengine/mock/RenderEngine.h>
 #include <ui/DisplayInfo.h>
 #include <ui/Rect.h>
 
@@ -156,6 +157,8 @@ struct DisplayTestCommon : public testing::Test {
 
     DisplayTestCommon() {
         EXPECT_CALL(mCompositionEngine, getHwComposer()).WillRepeatedly(ReturnRef(mHwComposer));
+        EXPECT_CALL(mCompositionEngine, getRenderEngine()).WillRepeatedly(ReturnRef(mRenderEngine));
+        EXPECT_CALL(mRenderEngine, supportsProtectedContent()).WillRepeatedly(Return(false));
     }
 
     DisplayCreationArgs getDisplayCreationArgsForPhysicalHWCDisplay() {
@@ -182,6 +185,7 @@ struct DisplayTestCommon : public testing::Test {
 
     StrictMock<android::mock::HWComposer> mHwComposer;
     StrictMock<Hwc2::mock::PowerAdvisor> mPowerAdvisor;
+    StrictMock<renderengine::mock::RenderEngine> mRenderEngine;
     StrictMock<mock::CompositionEngine> mCompositionEngine;
     sp<mock::NativeWindow> mNativeWindow = new StrictMock<mock::NativeWindow>();
 };

services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp

@@ -2797,6 +2797,7 @@ struct OutputComposeSurfacesTest : public testing::Test {
         mOutput.mState.usesClientComposition = true;
         mOutput.mState.usesDeviceComposition = false;
         mOutput.mState.reusedClientComposition = false;
+        mOutput.mState.flipClientTarget = false;
 
         EXPECT_CALL(mOutput, getCompositionEngine()).WillRepeatedly(ReturnRef(mCompositionEngine));
         EXPECT_CALL(mCompositionEngine, getRenderEngine()).WillRepeatedly(ReturnRef(mRenderEngine));
@@ -2868,19 +2869,40 @@ const HdrCapabilities OutputComposeSurfacesTest::
 TEST_F(OutputComposeSurfacesTest, doesNothingButSignalNoExpensiveRenderingIfNoClientComposition) {
     mOutput.mState.usesClientComposition = false;
 
+    EXPECT_CALL(mRenderEngine, supportsProtectedContent()).WillRepeatedly(Return(false));
+
     EXPECT_CALL(mOutput, setExpensiveRenderingExpected(false));
 
     verify().execute().expectAFenceWasReturned();
 }
 
-TEST_F(OutputComposeSurfacesTest, doesMinimalWorkIfDequeueBufferFails) {
+TEST_F(OutputComposeSurfacesTest,
-    EXPECT_CALL(mOutput, getSkipColorTransform()).WillRepeatedly(Return(false));
+       dequeuesABufferIfNoClientCompositionButFlipClientTargetRequested) {
-    EXPECT_CALL(*mDisplayColorProfile, hasWideColorGamut()).WillRepeatedly(Return(true));
+    mOutput.mState.usesClientComposition = false;
+    mOutput.mState.flipClientTarget = true;
+
+    EXPECT_CALL(mRenderEngine, supportsProtectedContent()).WillRepeatedly(Return(false));
+
+    EXPECT_CALL(*mRenderSurface, dequeueBuffer(_)).WillOnce(Return(mOutputBuffer));
+    EXPECT_CALL(mOutput, setExpensiveRenderingExpected(false));
+
+    verify().execute().expectAFenceWasReturned();
+}
+
+TEST_F(OutputComposeSurfacesTest, doesMinimalWorkIfDequeueBufferFailsForClientComposition) {
+    EXPECT_CALL(mRenderEngine, supportsProtectedContent()).WillRepeatedly(Return(false));
+
+    EXPECT_CALL(*mRenderSurface, dequeueBuffer(_)).WillOnce(Return(nullptr));
+
+    verify().execute().expectNoFenceWasReturned();
+}
+
+TEST_F(OutputComposeSurfacesTest,
+       doesMinimalWorkIfDequeueBufferFailsForNoClientCompositionButFlipClientTargetRequested) {
+    mOutput.mState.usesClientComposition = false;
+    mOutput.mState.flipClientTarget = true;
+
     EXPECT_CALL(mRenderEngine, supportsProtectedContent()).WillRepeatedly(Return(false));
-    EXPECT_CALL(mOutput, generateClientCompositionRequests(_, _, kDefaultOutputDataspace))
-            .WillRepeatedly(Return(std::vector<LayerFE::LayerSettings>{}));
-    EXPECT_CALL(mOutput, appendRegionFlashRequests(RegionEq(kDebugRegion), _))
-            .WillRepeatedly(Return());
 
     EXPECT_CALL(*mRenderSurface, dequeueBuffer(_)).WillOnce(Return(nullptr));
 

services/surfaceflinger/DisplayHardware/HWC2.cpp

@@ -349,7 +349,7 @@ Error Display::getName(std::string* outName) const
 
 Error Display::getRequests(HWC2::DisplayRequest* outDisplayRequests,
                            std::unordered_map<HWC2::Layer*, LayerRequest>* outLayerRequests) {
-    uint32_t intDisplayRequests;
+    uint32_t intDisplayRequests = 0;
     std::vector<Hwc2::Layer> layerIds;
     std::vector<uint32_t> layerRequests;
     auto intError = mComposer.getDisplayRequests(