SF multithreaded_present: clean up and add more tests

Add tests for CompositionEngine to verify that the displays which are
expected to offload HWC calls do so.

In CompositionEngine::present, improve the process for selecting which
displays to offload. In the old process, a virtual display without HAL
support could in theory return true for supportsOffloadPresent. Even
though it wasn't counted in the original number, it could then be told
to offload present, resulting in one of the intended displays not
offloading.

In the new process, store a list of eligible displays, so that only
eligible displays are told to offload. This also means that
supportsOffloadPresent is called at most once per frame.

Bug: 241285491
Bug: 259132483
Test: libcompositionengine_test
Change-Id: Ie2836e07e900c0269d60112ec0ebfa82ee4b82d7

3 files changed, 199 insertions, 39 deletions

diff --git a/services/surfaceflinger/CompositionEngine/Android.bp b/services/surfaceflinger/CompositionEngine/Android.bp
index 455e623c37..e316190499 100644
--- a/services/surfaceflinger/CompositionEngine/Android.bp
+++ b/services/surfaceflinger/CompositionEngine/Android.bp
@@ -127,6 +127,10 @@ cc_library {
 cc_test {
     name: "libcompositionengine_test",
     test_suites: ["device-tests"],
+    include_dirs: [
+        "frameworks/native/services/surfaceflinger/common/include",
+        "frameworks/native/services/surfaceflinger/tests/unittests",
+    ],
     defaults: ["libcompositionengine_defaults"],
     srcs: [
         "tests/planner/CachedSetTest.cpp",
diff --git a/services/surfaceflinger/CompositionEngine/src/CompositionEngine.cpp b/services/surfaceflinger/CompositionEngine/src/CompositionEngine.cpp
index 748d87bdc3..7be5fe323a 100644
--- a/services/surfaceflinger/CompositionEngine/src/CompositionEngine.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/CompositionEngine.cpp
@@ -90,29 +90,37 @@ nsecs_t CompositionEngine::getLastFrameRefreshTimestamp() const {
 }
 
 namespace {
-int numDisplaysWithOffloadPresentSupport(const CompositionRefreshArgs& args) {
-    if (!FlagManager::getInstance().multithreaded_present() || args.outputs.size() < 2) {
-        return 0;
+void offloadOutputs(Outputs& outputs) {
+    if (!FlagManager::getInstance().multithreaded_present() || outputs.size() < 2) {
+        return;
     }
 
-    int numEligibleDisplays = 0;
-    // Only run present in multiple threads if all HWC-enabled displays
-    // being refreshed support it.
-    if (!std::all_of(args.outputs.begin(), args.outputs.end(),
-                     [&numEligibleDisplays](const auto& output) {
-                         if (!ftl::Optional(output->getDisplayId())
-                                      .and_then(HalDisplayId::tryCast)) {
-                             // Not HWC-enabled, so it is always
-                             // client-composited.
-                             return true;
-                         }
-                         const bool support = output->supportsOffloadPresent();
-                         numEligibleDisplays += static_cast<int>(support);
-                         return support;
-                     })) {
-        return 0;
+    ui::PhysicalDisplayVector<compositionengine::Output*> outputsToOffload;
+    for (const auto& output : outputs) {
+        if (!ftl::Optional(output->getDisplayId()).and_then(HalDisplayId::tryCast)) {
+            // Not HWC-enabled, so it is always client-composited. No need to offload.
+            continue;
+        }
+
+        // Only run present in multiple threads if all HWC-enabled displays
+        // being refreshed support it.
+        if (!output->supportsOffloadPresent()) {
+            return;
+        }
+        outputsToOffload.push_back(output.get());
+    }
+
+    if (outputsToOffload.size() < 2) {
+        return;
+    }
+
+    // Leave the last eligible display on the main thread, which will
+    // allow it to run concurrently without an extra thread hop.
+    outputsToOffload.pop_back();
+
+    for (compositionengine::Output* output : outputsToOffload) {
+        output->offloadPresentNextFrame();
     }
-    return numEligibleDisplays;
 }
 } // namespace
 
@@ -136,20 +144,7 @@ void CompositionEngine::present(CompositionRefreshArgs& args) {
     // Offloading the HWC call for `present` allows us to simultaneously call it
     // on multiple displays. This is desirable because these calls block and can
     // be slow.
-    if (const int numEligibleDisplays = numDisplaysWithOffloadPresentSupport(args);
-        numEligibleDisplays > 1) {
-        // Leave the last eligible display on the main thread, which will
-        // allow it to run concurrently without an extra thread hop.
-        int numToOffload = numEligibleDisplays - 1;
-        for (auto& output : args.outputs) {
-            if (output->supportsOffloadPresent()) {
-                output->offloadPresentNextFrame();
-                if (--numToOffload == 0) {
-                    break;
-                }
-            }
-        }
-    }
+    offloadOutputs(args.outputs);
 
     ui::DisplayVector<ftl::Future<std::monostate>> presentFutures;
     for (const auto& output : args.outputs) {
diff --git a/services/surfaceflinger/CompositionEngine/tests/CompositionEngineTest.cpp b/services/surfaceflinger/CompositionEngine/tests/CompositionEngineTest.cpp
index a451ab2b77..602dd236f7 100644
--- a/services/surfaceflinger/CompositionEngine/tests/CompositionEngineTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/CompositionEngineTest.cpp
@@ -14,6 +14,8 @@
  * limitations under the License.
  */
 
+#include <com_android_graphics_surfaceflinger_flags.h>
+#include <common/test/FlagUtils.h>
 #include <compositionengine/CompositionRefreshArgs.h>
 #include <compositionengine/LayerFECompositionState.h>
 #include <compositionengine/impl/CompositionEngine.h>
@@ -29,6 +31,8 @@
 
 #include <variant>
 
+using namespace com::android::graphics::surfaceflinger;
+
 namespace android::compositionengine {
 namespace {
 
@@ -110,11 +114,9 @@ TEST_F(CompositionEnginePresentTest, worksAsExpected) {
     EXPECT_CALL(*mOutput2, prepare(Ref(mRefreshArgs), _));
     EXPECT_CALL(*mOutput3, prepare(Ref(mRefreshArgs), _));
 
-    if (FlagManager::getInstance().multithreaded_present()) {
-        EXPECT_CALL(*mOutput1, getDisplayId()).WillOnce(Return(std::nullopt));
-        EXPECT_CALL(*mOutput2, getDisplayId()).WillOnce(Return(std::nullopt));
-        EXPECT_CALL(*mOutput3, getDisplayId()).WillOnce(Return(std::nullopt));
-    }
+    // All of mOutput<i> are StrictMocks. If the flag is true, it will introduce
+    // calls to getDisplayId, which are not relevant to this test.
+    SET_FLAG_FOR_TEST(flags::multithreaded_present, false);
 
     // The last step is to actually present each output.
     EXPECT_CALL(*mOutput1, present(Ref(mRefreshArgs)))
@@ -272,5 +274,164 @@ TEST_F(CompositionTestPreComposition,
     EXPECT_TRUE(mEngine.needsAnotherUpdate());
 }
 
+struct CompositionEngineOffloadTest : public testing::Test {
+    impl::CompositionEngine mEngine;
+    CompositionRefreshArgs mRefreshArgs;
+
+    std::shared_ptr<mock::Output> mDisplay1{std::make_shared<StrictMock<mock::Output>>()};
+    std::shared_ptr<mock::Output> mDisplay2{std::make_shared<StrictMock<mock::Output>>()};
+    std::shared_ptr<mock::Output> mVirtualDisplay{std::make_shared<StrictMock<mock::Output>>()};
+    std::shared_ptr<mock::Output> mHalVirtualDisplay{std::make_shared<StrictMock<mock::Output>>()};
+
+    static constexpr PhysicalDisplayId kDisplayId1 = PhysicalDisplayId::fromPort(123u);
+    static constexpr PhysicalDisplayId kDisplayId2 = PhysicalDisplayId::fromPort(234u);
+    static constexpr GpuVirtualDisplayId kGpuVirtualDisplayId{789u};
+    static constexpr HalVirtualDisplayId kHalVirtualDisplayId{456u};
+
+    void SetUp() override {
+        EXPECT_CALL(*mDisplay1, getDisplayId)
+                .WillRepeatedly(Return(std::make_optional<DisplayId>(kDisplayId1)));
+        EXPECT_CALL(*mDisplay2, getDisplayId)
+                .WillRepeatedly(Return(std::make_optional<DisplayId>(kDisplayId2)));
+        EXPECT_CALL(*mVirtualDisplay, getDisplayId)
+                .WillRepeatedly(Return(std::make_optional<DisplayId>(kGpuVirtualDisplayId)));
+        EXPECT_CALL(*mHalVirtualDisplay, getDisplayId)
+                .WillRepeatedly(Return(std::make_optional<DisplayId>(kHalVirtualDisplayId)));
+    }
+
+    void setOutputs(std::initializer_list<std::shared_ptr<mock::Output>> outputs) {
+        for (auto& output : outputs) {
+            // If we call mEngine.present, prepare and present will be called on all the
+            // outputs in mRefreshArgs, but that's not the interesting part of the test.
+            EXPECT_CALL(*output, prepare(Ref(mRefreshArgs), _)).Times(1);
+            EXPECT_CALL(*output, present(Ref(mRefreshArgs)))
+                    .WillOnce(Return(ftl::yield<std::monostate>({})));
+
+            mRefreshArgs.outputs.push_back(std::move(output));
+        }
+    }
+};
+
+TEST_F(CompositionEngineOffloadTest, basic) {
+    EXPECT_CALL(*mDisplay1, supportsOffloadPresent).WillOnce(Return(true));
+    EXPECT_CALL(*mDisplay2, supportsOffloadPresent).WillOnce(Return(true));
+
+    EXPECT_CALL(*mDisplay1, offloadPresentNextFrame).Times(1);
+    EXPECT_CALL(*mDisplay2, offloadPresentNextFrame).Times(0);
+
+    SET_FLAG_FOR_TEST(flags::multithreaded_present, true);
+    setOutputs({mDisplay1, mDisplay2});
+
+    mEngine.present(mRefreshArgs);
+}
+
+TEST_F(CompositionEngineOffloadTest, dependsOnSupport) {
+    EXPECT_CALL(*mDisplay1, supportsOffloadPresent).WillOnce(Return(false));
+    EXPECT_CALL(*mDisplay2, supportsOffloadPresent).Times(0);
+
+    EXPECT_CALL(*mDisplay1, offloadPresentNextFrame).Times(0);
+    EXPECT_CALL(*mDisplay2, offloadPresentNextFrame).Times(0);
+
+    SET_FLAG_FOR_TEST(flags::multithreaded_present, true);
+    setOutputs({mDisplay1, mDisplay2});
+
+    mEngine.present(mRefreshArgs);
+}
+
+TEST_F(CompositionEngineOffloadTest, dependsOnSupport2) {
+    EXPECT_CALL(*mDisplay1, supportsOffloadPresent).WillOnce(Return(true));
+    EXPECT_CALL(*mDisplay2, supportsOffloadPresent).WillOnce(Return(false));
+
+    EXPECT_CALL(*mDisplay1, offloadPresentNextFrame).Times(0);
+    EXPECT_CALL(*mDisplay2, offloadPresentNextFrame).Times(0);
+
+    SET_FLAG_FOR_TEST(flags::multithreaded_present, true);
+    setOutputs({mDisplay1, mDisplay2});
+
+    mEngine.present(mRefreshArgs);
+}
+
+TEST_F(CompositionEngineOffloadTest, dependsOnFlag) {
+    EXPECT_CALL(*mDisplay1, supportsOffloadPresent).Times(0);
+    EXPECT_CALL(*mDisplay2, supportsOffloadPresent).Times(0);
+
+    EXPECT_CALL(*mDisplay1, offloadPresentNextFrame).Times(0);
+    EXPECT_CALL(*mDisplay2, offloadPresentNextFrame).Times(0);
+
+    SET_FLAG_FOR_TEST(flags::multithreaded_present, false);
+    setOutputs({mDisplay1, mDisplay2});
+
+    mEngine.present(mRefreshArgs);
+}
+
+TEST_F(CompositionEngineOffloadTest, oneDisplay) {
+    EXPECT_CALL(*mDisplay1, supportsOffloadPresent).Times(0);
+
+    EXPECT_CALL(*mDisplay1, offloadPresentNextFrame).Times(0);
+
+    SET_FLAG_FOR_TEST(flags::multithreaded_present, true);
+    setOutputs({mDisplay1});
+
+    mEngine.present(mRefreshArgs);
+}
+
+TEST_F(CompositionEngineOffloadTest, virtualDisplay) {
+    EXPECT_CALL(*mDisplay1, supportsOffloadPresent).WillOnce(Return(true));
+    EXPECT_CALL(*mDisplay2, supportsOffloadPresent).WillOnce(Return(true));
+    EXPECT_CALL(*mVirtualDisplay, supportsOffloadPresent).Times(0);
+
+    EXPECT_CALL(*mDisplay1, offloadPresentNextFrame).Times(1);
+    EXPECT_CALL(*mDisplay2, offloadPresentNextFrame).Times(0);
+    EXPECT_CALL(*mVirtualDisplay, offloadPresentNextFrame).Times(0);
+
+    SET_FLAG_FOR_TEST(flags::multithreaded_present, true);
+    setOutputs({mDisplay1, mDisplay2, mVirtualDisplay});
+
+    mEngine.present(mRefreshArgs);
+}
+
+TEST_F(CompositionEngineOffloadTest, virtualDisplay2) {
+    EXPECT_CALL(*mDisplay1, supportsOffloadPresent).WillOnce(Return(true));
+    EXPECT_CALL(*mVirtualDisplay, supportsOffloadPresent).Times(0);
+
+    EXPECT_CALL(*mDisplay1, offloadPresentNextFrame).Times(0);
+    EXPECT_CALL(*mVirtualDisplay, offloadPresentNextFrame).Times(0);
+
+    SET_FLAG_FOR_TEST(flags::multithreaded_present, true);
+    setOutputs({mDisplay1, mVirtualDisplay});
+
+    mEngine.present(mRefreshArgs);
+}
+
+TEST_F(CompositionEngineOffloadTest, halVirtual) {
+    EXPECT_CALL(*mDisplay1, supportsOffloadPresent).WillOnce(Return(true));
+    EXPECT_CALL(*mHalVirtualDisplay, supportsOffloadPresent).WillOnce(Return(true));
+
+    EXPECT_CALL(*mDisplay1, offloadPresentNextFrame).Times(1);
+    EXPECT_CALL(*mHalVirtualDisplay, offloadPresentNextFrame).Times(0);
+
+    SET_FLAG_FOR_TEST(flags::multithreaded_present, true);
+    setOutputs({mDisplay1, mHalVirtualDisplay});
+
+    mEngine.present(mRefreshArgs);
+}
+
+TEST_F(CompositionEngineOffloadTest, ordering) {
+    EXPECT_CALL(*mVirtualDisplay, supportsOffloadPresent).Times(0);
+    EXPECT_CALL(*mHalVirtualDisplay, supportsOffloadPresent).WillOnce(Return(true));
+    EXPECT_CALL(*mDisplay1, supportsOffloadPresent).WillOnce(Return(true));
+    EXPECT_CALL(*mDisplay2, supportsOffloadPresent).WillOnce(Return(true));
+
+    EXPECT_CALL(*mVirtualDisplay, offloadPresentNextFrame).Times(0);
+    EXPECT_CALL(*mHalVirtualDisplay, offloadPresentNextFrame).Times(1);
+    EXPECT_CALL(*mDisplay1, offloadPresentNextFrame).Times(1);
+    EXPECT_CALL(*mDisplay2, offloadPresentNextFrame).Times(0);
+
+    SET_FLAG_FOR_TEST(flags::multithreaded_present, true);
+    setOutputs({mVirtualDisplay, mHalVirtualDisplay, mDisplay1, mDisplay2});
+
+    mEngine.present(mRefreshArgs);
+}
+
 } // namespace
 } // namespace android::compositionengine