SF: Remove multithreaded_present flag
Change (and rename) CompositionEnginePresentTest.worksAsExpected to
avoid an assumption that would now break the test.
Bug: 380251940
Flag: EXEMPT removing multithreaded_present
Test: presubmit
Change-Id: Ifbcbfc7d82e64d909a6e4d310f34bcb6578d1e1f
diff --git a/services/surfaceflinger/CompositionEngine/src/CompositionEngine.cpp b/services/surfaceflinger/CompositionEngine/src/CompositionEngine.cpp
index 989f8e3..34ede33 100644
--- a/services/surfaceflinger/CompositionEngine/src/CompositionEngine.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/CompositionEngine.cpp
@@ -91,7 +91,7 @@
namespace {
void offloadOutputs(Outputs& outputs) {
- if (!FlagManager::getInstance().multithreaded_present() || outputs.size() < 2) {
+ if (outputs.size() < 2) {
return;
}
diff --git a/services/surfaceflinger/CompositionEngine/tests/CompositionEngineTest.cpp b/services/surfaceflinger/CompositionEngine/tests/CompositionEngineTest.cpp
index ad65c44..b278000 100644
--- a/services/surfaceflinger/CompositionEngine/tests/CompositionEngineTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/CompositionEngineTest.cpp
@@ -43,6 +43,10 @@
using ::testing::SaveArg;
using ::testing::StrictMock;
+static constexpr PhysicalDisplayId kDisplayId1 = PhysicalDisplayId::fromPort(123u);
+static constexpr PhysicalDisplayId kDisplayId2 = PhysicalDisplayId::fromPort(234u);
+static constexpr PhysicalDisplayId kDisplayId3 = PhysicalDisplayId::fromPort(567u);
+
struct CompositionEngineTest : public testing::Test {
std::shared_ptr<TimeStats> mTimeStats;
@@ -52,6 +56,26 @@
std::shared_ptr<mock::Output> mOutput1{std::make_shared<StrictMock<mock::Output>>()};
std::shared_ptr<mock::Output> mOutput2{std::make_shared<StrictMock<mock::Output>>()};
std::shared_ptr<mock::Output> mOutput3{std::make_shared<StrictMock<mock::Output>>()};
+
+ std::array<impl::OutputCompositionState, 3> mOutputStates;
+
+ void SetUp() override {
+ EXPECT_CALL(*mOutput1, getDisplayId)
+ .WillRepeatedly(Return(std::make_optional<DisplayId>(kDisplayId1)));
+ EXPECT_CALL(*mOutput2, getDisplayId)
+ .WillRepeatedly(Return(std::make_optional<DisplayId>(kDisplayId2)));
+ EXPECT_CALL(*mOutput3, getDisplayId)
+ .WillRepeatedly(Return(std::make_optional<DisplayId>(kDisplayId3)));
+
+ // Most tests will depend on the outputs being enabled.
+ for (auto& state : mOutputStates) {
+ state.isEnabled = true;
+ }
+
+ EXPECT_CALL(*mOutput1, getState).WillRepeatedly(ReturnRef(mOutputStates[0]));
+ EXPECT_CALL(*mOutput2, getState).WillRepeatedly(ReturnRef(mOutputStates[1]));
+ EXPECT_CALL(*mOutput3, getState).WillRepeatedly(ReturnRef(mOutputStates[2]));
+ }
};
TEST_F(CompositionEngineTest, canInstantiateCompositionEngine) {
@@ -94,7 +118,7 @@
StrictMock<CompositionEnginePartialMock> mEngine;
};
-TEST_F(CompositionEnginePresentTest, worksWithEmptyRequest) {
+TEST_F(CompositionEnginePresentTest, zeroOutputs) {
// present() always calls preComposition() and postComposition()
EXPECT_CALL(mEngine, preComposition(Ref(mRefreshArgs)));
EXPECT_CALL(mEngine, postComposition(Ref(mRefreshArgs)));
@@ -102,7 +126,7 @@
mEngine.present(mRefreshArgs);
}
-TEST_F(CompositionEnginePresentTest, worksAsExpected) {
+TEST_F(CompositionEnginePresentTest, threeOutputs) {
// Expect calls to in a certain sequence
InSequence seq;
@@ -114,9 +138,7 @@
EXPECT_CALL(*mOutput2, prepare(Ref(mRefreshArgs), _));
EXPECT_CALL(*mOutput3, prepare(Ref(mRefreshArgs), _));
- // 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);
+ EXPECT_CALL(*mOutput1, supportsOffloadPresent).WillOnce(Return(false));
// The last step is to actually present each output.
EXPECT_CALL(*mOutput1, present(Ref(mRefreshArgs)))
@@ -284,8 +306,6 @@
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};
@@ -332,7 +352,6 @@
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);
@@ -345,7 +364,6 @@
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);
@@ -358,20 +376,6 @@
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);
@@ -382,7 +386,6 @@
EXPECT_CALL(*mDisplay1, offloadPresentNextFrame).Times(0);
- SET_FLAG_FOR_TEST(flags::multithreaded_present, true);
setOutputs({mDisplay1});
mEngine.present(mRefreshArgs);
@@ -397,7 +400,6 @@
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);
@@ -410,7 +412,6 @@
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);
@@ -423,7 +424,6 @@
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);
@@ -440,7 +440,6 @@
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);
@@ -458,7 +457,6 @@
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);
@@ -478,7 +476,6 @@
EXPECT_CALL(*mDisplay2, offloadPresentNextFrame).Times(0);
EXPECT_CALL(*mHalVirtualDisplay, offloadPresentNextFrame).Times(0);
- SET_FLAG_FOR_TEST(flags::multithreaded_present, true);
setOutputs({mDisplay1, mDisplay2, mHalVirtualDisplay});
mEngine.present(mRefreshArgs);
diff --git a/services/surfaceflinger/DisplayHardware/AidlComposerHal.cpp b/services/surfaceflinger/DisplayHardware/AidlComposerHal.cpp
index bb6bebe..d547af9 100644
--- a/services/surfaceflinger/DisplayHardware/AidlComposerHal.cpp
+++ b/services/surfaceflinger/DisplayHardware/AidlComposerHal.cpp
@@ -1773,7 +1773,6 @@
}
bool AidlComposer::hasMultiThreadedPresentSupport(Display display) {
- if (!FlagManager::getInstance().multithreaded_present()) return false;
const auto displayId = translate<int64_t>(display);
std::vector<AidlDisplayCapability> capabilities;
const auto status = mAidlComposerClient->getDisplayCapabilities(displayId, &capabilities);
diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp
index e587178..911d489 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.cpp
+++ b/services/surfaceflinger/Scheduler/Scheduler.cpp
@@ -554,8 +554,7 @@
ftl::FakeGuard guard(kMainThreadContext);
for (const auto& [id, display] : mDisplays) {
- if (display.powerMode != hal::PowerMode::OFF ||
- !FlagManager::getInstance().multithreaded_present()) {
+ if (display.powerMode != hal::PowerMode::OFF) {
resyncToHardwareVsyncLocked(id, allowToEnable);
}
}
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 9cd2314..3f6b8d0 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -5731,8 +5731,7 @@
}
getHwComposer().setPowerMode(displayId, mode);
- if (mode != hal::PowerMode::DOZE_SUSPEND &&
- (displayId == mActiveDisplayId || FlagManager::getInstance().multithreaded_present())) {
+ if (mode != hal::PowerMode::DOZE_SUSPEND) {
const bool enable =
mScheduler->getVsyncSchedule(displayId)->getPendingHardwareVsyncState();
requestHardwareVsync(displayId, enable);
@@ -5759,14 +5758,11 @@
setSchedAttr(false, kWhence);
if (currentModeNotDozeSuspend) {
- if (!FlagManager::getInstance().multithreaded_present()) {
- mScheduler->disableHardwareVsync(displayId, true);
- }
mScheduler->enableSyntheticVsync();
}
}
}
- if (currentModeNotDozeSuspend && FlagManager::getInstance().multithreaded_present()) {
+ if (currentModeNotDozeSuspend) {
constexpr bool kDisallow = true;
mScheduler->disableHardwareVsync(displayId, kDisallow);
}
@@ -5784,8 +5780,7 @@
} else if (mode == hal::PowerMode::DOZE || mode == hal::PowerMode::ON) {
// Update display while dozing
getHwComposer().setPowerMode(displayId, mode);
- if (currentMode == hal::PowerMode::DOZE_SUSPEND &&
- (displayId == mActiveDisplayId || FlagManager::getInstance().multithreaded_present())) {
+ if (currentMode == hal::PowerMode::DOZE_SUSPEND) {
if (displayId == mActiveDisplayId) {
ALOGI("Force repainting for DOZE_SUSPEND -> DOZE or ON.");
mVisibleRegionsDirty = true;
@@ -5797,10 +5792,9 @@
}
} else if (mode == hal::PowerMode::DOZE_SUSPEND) {
// Leave display going to doze
- if (displayId == mActiveDisplayId || FlagManager::getInstance().multithreaded_present()) {
- constexpr bool kDisallow = true;
- mScheduler->disableHardwareVsync(displayId, kDisallow);
- }
+ constexpr bool kDisallow = true;
+ mScheduler->disableHardwareVsync(displayId, kDisallow);
+
if (displayId == mActiveDisplayId) {
mScheduler->enableSyntheticVsync();
}
diff --git a/services/surfaceflinger/common/FlagManager.cpp b/services/surfaceflinger/common/FlagManager.cpp
index 42fa436..5ff3d82 100644
--- a/services/surfaceflinger/common/FlagManager.cpp
+++ b/services/surfaceflinger/common/FlagManager.cpp
@@ -164,7 +164,6 @@
DUMP_ACONFIG_FLAG(latch_unsignaled_with_auto_refresh_changed);
DUMP_ACONFIG_FLAG(local_tonemap_screenshots);
DUMP_ACONFIG_FLAG(misc1);
- DUMP_ACONFIG_FLAG(multithreaded_present);
DUMP_ACONFIG_FLAG(no_vsyncs_on_screen_off);
DUMP_ACONFIG_FLAG(override_trusted_overlay);
DUMP_ACONFIG_FLAG(protected_if_client);
@@ -259,7 +258,6 @@
FLAG_MANAGER_ACONFIG_FLAG(misc1, "")
FLAG_MANAGER_ACONFIG_FLAG(vrr_config, "debug.sf.enable_vrr_config")
FLAG_MANAGER_ACONFIG_FLAG(hdcp_level_hal, "")
-FLAG_MANAGER_ACONFIG_FLAG(multithreaded_present, "debug.sf.multithreaded_present")
FLAG_MANAGER_ACONFIG_FLAG(add_sf_skipped_frames_to_trace, "")
FLAG_MANAGER_ACONFIG_FLAG(use_known_refresh_rate_for_fps_consistency, "")
FLAG_MANAGER_ACONFIG_FLAG(cache_when_source_crop_layer_only_moved,
diff --git a/services/surfaceflinger/common/include/common/FlagManager.h b/services/surfaceflinger/common/include/common/FlagManager.h
index dd7042d..419e92b 100644
--- a/services/surfaceflinger/common/include/common/FlagManager.h
+++ b/services/surfaceflinger/common/include/common/FlagManager.h
@@ -99,7 +99,6 @@
bool local_tonemap_screenshots() const;
bool luts_api() const;
bool misc1() const;
- bool multithreaded_present() const;
bool no_vsyncs_on_screen_off() const;
bool override_trusted_overlay() const;
bool protected_if_client() const;
diff --git a/services/surfaceflinger/tests/unittests/FlagManagerTest.cpp b/services/surfaceflinger/tests/unittests/FlagManagerTest.cpp
index a5b347a..c6da1a1 100644
--- a/services/surfaceflinger/tests/unittests/FlagManagerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/FlagManagerTest.cpp
@@ -125,13 +125,13 @@
TEST_F(FlagManagerTest, ignoresOverrideInUnitTestMode) {
mFlagManager.setUnitTestMode();
- SET_FLAG_FOR_TEST(flags::multithreaded_present, true);
+ SET_FLAG_FOR_TEST(flags::no_vsyncs_on_screen_off, true);
// If this has not been called in this process, it will be called.
// Regardless, the result is ignored.
EXPECT_CALL(mFlagManager, getBoolProperty).WillRepeatedly(Return(false));
- EXPECT_EQ(true, mFlagManager.multithreaded_present());
+ EXPECT_EQ(true, mFlagManager.no_vsyncs_on_screen_off());
}
TEST_F(FlagManagerTest, returnsValue) {
diff --git a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
index 49c35e2..116fcd9 100644
--- a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
@@ -741,8 +741,6 @@
}
TEST_F(SchedulerTest, resyncAllSkipsOffDisplays) FTL_FAKE_GUARD(kMainThreadContext) {
- SET_FLAG_FOR_TEST(flags::multithreaded_present, true);
-
// resyncAllToHardwareVsync will result in requesting hardware VSYNC on display 1, which is on,
// but not on display 2, which is off.
EXPECT_CALL(mScheduler->mockRequestHardwareVsync, Call(kDisplayId1, true)).Times(1);
@@ -763,28 +761,6 @@
mScheduler->resyncAllToHardwareVsync(kAllowToEnable);
}
-TEST_F(SchedulerTest, resyncAllLegacyAppliesToOffDisplays) FTL_FAKE_GUARD(kMainThreadContext) {
- SET_FLAG_FOR_TEST(flags::multithreaded_present, false);
-
- // In the legacy code, prior to the flag, resync applied to OFF displays.
- EXPECT_CALL(mScheduler->mockRequestHardwareVsync, Call(kDisplayId1, true)).Times(1);
- EXPECT_CALL(mScheduler->mockRequestHardwareVsync, Call(kDisplayId2, true)).Times(1);
-
- mScheduler->setDisplayPowerMode(kDisplayId1, hal::PowerMode::ON);
-
- mScheduler->registerDisplay(kDisplayId2,
- std::make_shared<RefreshRateSelector>(kDisplay2Modes,
- kDisplay2Mode60->getId()));
- ASSERT_EQ(hal::PowerMode::OFF, mScheduler->getDisplayPowerMode(kDisplayId2));
-
- static constexpr bool kDisallow = true;
- mScheduler->disableHardwareVsync(kDisplayId1, kDisallow);
- mScheduler->disableHardwareVsync(kDisplayId2, kDisallow);
-
- static constexpr bool kAllowToEnable = true;
- mScheduler->resyncAllToHardwareVsync(kAllowToEnable);
-}
-
class AttachedChoreographerTest : public SchedulerTest {
protected:
void frameRateTestScenario(Fps layerFps, int8_t frameRateCompatibility, Fps displayFps,
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_FoldableTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_FoldableTest.cpp
index fde2749..8972840 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_FoldableTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_FoldableTest.cpp
@@ -167,7 +167,6 @@
}
TEST_F(FoldableTest, requestVsyncOnPowerOn) {
- SET_FLAG_FOR_TEST(flags::multithreaded_present, true);
EXPECT_CALL(mFlinger.scheduler()->mockRequestHardwareVsync, Call(kInnerDisplayId, true))
.Times(1);
EXPECT_CALL(mFlinger.scheduler()->mockRequestHardwareVsync, Call(kOuterDisplayId, true))
@@ -178,7 +177,6 @@
}
TEST_F(FoldableTest, disableVsyncOnPowerOffPacesetter) {
- SET_FLAG_FOR_TEST(flags::multithreaded_present, true);
// When the device boots, the inner display should be the pacesetter.
ASSERT_EQ(mFlinger.scheduler()->pacesetterDisplayId(), kInnerDisplayId);
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPhysicalDisplayPowerModeTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPhysicalDisplayPowerModeTest.cpp
index f2fbbdd..a1e37ff 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPhysicalDisplayPowerModeTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPhysicalDisplayPowerModeTest.cpp
@@ -457,7 +457,6 @@
}
TEST_F(SetPhysicalDisplayPowerModeTest, transitionsDisplayFromOffToOnExternalDisplay) {
- SET_FLAG_FOR_TEST(flags::multithreaded_present, true);
transitionDisplayCommon<ExternalDisplayPowerCase<TransitionOffToOnVariant>>();
}
@@ -466,7 +465,6 @@
}
TEST_F(SetPhysicalDisplayPowerModeTest, transitionsDisplayFromOnToOffExternalDisplay) {
- SET_FLAG_FOR_TEST(flags::multithreaded_present, true);
transitionDisplayCommon<ExternalDisplayPowerCase<TransitionOnToOffVariant>>();
}
@@ -479,7 +477,6 @@
}
TEST_F(SetPhysicalDisplayPowerModeTest, transitionsDisplayFromDozeSuspendToDozeExternalDisplay) {
- SET_FLAG_FOR_TEST(flags::multithreaded_present, true);
transitionDisplayCommon<ExternalDisplayPowerCase<TransitionDozeSuspendToDozeVariant>>();
}
@@ -488,12 +485,10 @@
}
TEST_F(SetPhysicalDisplayPowerModeTest, transitionsDisplayFromDozeSuspendToOnExternalDisplay) {
- SET_FLAG_FOR_TEST(flags::multithreaded_present, true);
transitionDisplayCommon<ExternalDisplayPowerCase<TransitionDozeSuspendToOnVariant>>();
}
TEST_F(SetPhysicalDisplayPowerModeTest, transitionsDisplayFromOnToDozeSuspendExternalDisplay) {
- SET_FLAG_FOR_TEST(flags::multithreaded_present, true);
transitionDisplayCommon<ExternalDisplayPowerCase<TransitionOnToDozeSuspendVariant>>();
}