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
diff --git a/services/surfaceflinger/CompositionEngine/Android.bp b/services/surfaceflinger/CompositionEngine/Android.bp
index 455e623..e316190 100644
--- a/services/surfaceflinger/CompositionEngine/Android.bp
+++ b/services/surfaceflinger/CompositionEngine/Android.bp
@@ -127,6 +127,10 @@
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 748d87b..7be5fe3 100644
--- a/services/surfaceflinger/CompositionEngine/src/CompositionEngine.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/CompositionEngine.cpp
@@ -90,29 +90,37 @@
}
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());
}
- return numEligibleDisplays;
+
+ 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();
+ }
}
} // namespace
@@ -136,20 +144,7 @@
// 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 a451ab2..602dd23 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 @@
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 @@
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