From ab551a3e3a41216f0eeb17de5c9461971a8bbbfa Mon Sep 17 00:00:00 2001 From: Leon Scroggins III Date: Wed, 22 Nov 2023 10:08:25 -0500 Subject: 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 --- .../surfaceflinger/CompositionEngine/Android.bp | 4 + .../CompositionEngine/src/CompositionEngine.cpp | 63 ++++---- .../tests/CompositionEngineTest.cpp | 171 ++++++++++++++++++++- 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(support); - return support; - })) { - return 0; + ui::PhysicalDisplayVector 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> 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 +#include #include #include #include @@ -29,6 +31,8 @@ #include +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 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 mDisplay1{std::make_shared>()}; + std::shared_ptr mDisplay2{std::make_shared>()}; + std::shared_ptr mVirtualDisplay{std::make_shared>()}; + std::shared_ptr mHalVirtualDisplay{std::make_shared>()}; + + 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(kDisplayId1))); + EXPECT_CALL(*mDisplay2, getDisplayId) + .WillRepeatedly(Return(std::make_optional(kDisplayId2))); + EXPECT_CALL(*mVirtualDisplay, getDisplayId) + .WillRepeatedly(Return(std::make_optional(kGpuVirtualDisplayId))); + EXPECT_CALL(*mHalVirtualDisplay, getDisplayId) + .WillRepeatedly(Return(std::make_optional(kHalVirtualDisplayId))); + } + + void setOutputs(std::initializer_list> 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({}))); + + 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 -- cgit v1.2.3-59-g8ed1b