SF: Avoid registering DisplayDevice with Scheduler
The Scheduler should only care about the RefreshRateSelector part, and
should not needlessly extend the compositionengine::Display's lifetime
until the DisplayDevice is unregistered.
Make Scheduler::registerDisplay infallible, such that SurfaceFlinger::
processDisplayChanged does not need to unregister before registering.
Bug: 241285191
Test: libsurfaceflinger_unittest
Change-Id: I12b3855167e98f48ae368d39264edcb456efb293
diff --git a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
index 066083f..ea4666e 100644
--- a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
@@ -20,7 +20,6 @@
#include <mutex>
-#include "FakeDisplayInjector.h"
#include "Scheduler/EventThread.h"
#include "Scheduler/RefreshRateSelector.h"
#include "TestableScheduler.h"
@@ -41,7 +40,6 @@
using MockEventThread = android::mock::EventThread;
using MockLayer = android::mock::MockLayer;
-using FakeDisplayDeviceInjector = TestableSurfaceFlinger::FakeDisplayDeviceInjector;
class SchedulerTest : public testing::Test {
protected:
@@ -90,10 +88,6 @@
sp<MockEventThreadConnection> mEventThreadConnection;
TestableSurfaceFlinger mFlinger;
- Hwc2::mock::PowerAdvisor mPowerAdvisor;
- sp<android::mock::NativeWindow> mNativeWindow = sp<android::mock::NativeWindow>::make();
-
- FakeDisplayInjector mFakeDisplayInjector{mFlinger, mPowerAdvisor, mNativeWindow};
};
SchedulerTest::SchedulerTest() {
@@ -240,14 +234,11 @@
}
TEST_F(SchedulerTest, chooseRefreshRateForContentSelectsMaxRefreshRate) {
- const auto display = mFakeDisplayInjector.injectInternalDisplay(
- [&](FakeDisplayDeviceInjector& injector) {
- injector.setDisplayModes(kDisplay1Modes, kDisplay1Mode60->getId());
- },
- {.displayId = kDisplayId1});
+ const auto selectorPtr =
+ std::make_shared<RefreshRateSelector>(kDisplay1Modes, kDisplay1Mode60->getId());
- mScheduler->registerDisplay(display);
- mScheduler->setRefreshRateSelector(display->holdRefreshRateSelector());
+ mScheduler->registerDisplay(kDisplayId1, selectorPtr);
+ mScheduler->setRefreshRateSelector(selectorPtr);
const sp<MockLayer> layer = sp<MockLayer>::make(mFlinger.flinger());
EXPECT_CALL(*layer, isVisible()).WillOnce(Return(true));
@@ -269,13 +260,9 @@
}
TEST_F(SchedulerTest, chooseDisplayModesSingleDisplay) {
- const auto display = mFakeDisplayInjector.injectInternalDisplay(
- [&](FakeDisplayDeviceInjector& injector) {
- injector.setDisplayModes(kDisplay1Modes, kDisplay1Mode60->getId());
- },
- {.displayId = kDisplayId1});
-
- mScheduler->registerDisplay(display);
+ mScheduler->registerDisplay(kDisplayId1,
+ std::make_shared<RefreshRateSelector>(kDisplay1Modes,
+ kDisplay1Mode60->getId()));
std::vector<RefreshRateSelector::LayerRequirement> layers =
std::vector<RefreshRateSelector::LayerRequirement>({{.weight = 1.f}, {.weight = 1.f}});
@@ -314,23 +301,16 @@
EXPECT_EQ(choice->get(), DisplayModeChoice(kDisplay1Mode120, globalSignals));
mScheduler->unregisterDisplay(kDisplayId1);
- EXPECT_TRUE(mScheduler->mutableDisplays().empty());
+ EXPECT_FALSE(mScheduler->hasRefreshRateSelectors());
}
TEST_F(SchedulerTest, chooseDisplayModesMultipleDisplays) {
- const auto display1 = mFakeDisplayInjector.injectInternalDisplay(
- [&](FakeDisplayDeviceInjector& injector) {
- injector.setDisplayModes(kDisplay1Modes, kDisplay1Mode60->getId());
- },
- {.displayId = kDisplayId1, .hwcDisplayId = 42, .isPrimary = true});
- const auto display2 = mFakeDisplayInjector.injectInternalDisplay(
- [&](FakeDisplayDeviceInjector& injector) {
- injector.setDisplayModes(kDisplay2Modes, kDisplay2Mode60->getId());
- },
- {.displayId = kDisplayId2, .hwcDisplayId = 41, .isPrimary = false});
-
- mScheduler->registerDisplay(display1);
- mScheduler->registerDisplay(display2);
+ mScheduler->registerDisplay(kDisplayId1,
+ std::make_shared<RefreshRateSelector>(kDisplay1Modes,
+ kDisplay1Mode60->getId()));
+ mScheduler->registerDisplay(kDisplayId2,
+ std::make_shared<RefreshRateSelector>(kDisplay2Modes,
+ kDisplay2Mode60->getId()));
using DisplayModeChoice = TestableScheduler::DisplayModeChoice;
TestableScheduler::DisplayModeChoiceMap expectedChoices;
@@ -380,13 +360,10 @@
}
{
// This display does not support 120 Hz, so we should choose 60 Hz despite the touch signal.
- const auto display3 = mFakeDisplayInjector.injectInternalDisplay(
- [&](FakeDisplayDeviceInjector& injector) {
- injector.setDisplayModes(kDisplay3Modes, kDisplay3Mode60->getId());
- },
- {.displayId = kDisplayId3, .hwcDisplayId = 40, .isPrimary = false});
-
- mScheduler->registerDisplay(display3);
+ mScheduler
+ ->registerDisplay(kDisplayId3,
+ std::make_shared<RefreshRateSelector>(kDisplay3Modes,
+ kDisplay3Mode60->getId()));
const GlobalSignals globalSignals = {.touch = true};
mScheduler->replaceTouchTimer(10);
diff --git a/services/surfaceflinger/tests/unittests/TestableScheduler.h b/services/surfaceflinger/tests/unittests/TestableScheduler.h
index 2814d38..95c9915 100644
--- a/services/surfaceflinger/tests/unittests/TestableScheduler.h
+++ b/services/surfaceflinger/tests/unittests/TestableScheduler.h
@@ -32,15 +32,13 @@
class TestableScheduler : public Scheduler, private ICompositor {
public:
- TestableScheduler(std::shared_ptr<RefreshRateSelector> selectorPtr,
- ISchedulerCallback& callback)
+ TestableScheduler(RefreshRateSelectorPtr selectorPtr, ISchedulerCallback& callback)
: TestableScheduler(std::make_unique<mock::VsyncController>(),
std::make_unique<mock::VSyncTracker>(), std::move(selectorPtr),
callback) {}
TestableScheduler(std::unique_ptr<VsyncController> controller,
- std::unique_ptr<VSyncTracker> tracker,
- std::shared_ptr<RefreshRateSelector> selectorPtr,
+ std::unique_ptr<VSyncTracker> tracker, RefreshRateSelectorPtr selectorPtr,
ISchedulerCallback& callback)
: Scheduler(*this, callback, Feature::kContentDetection) {
mVsyncSchedule.emplace(VsyncSchedule(std::move(tracker), nullptr, std::move(controller)));
@@ -68,16 +66,15 @@
auto& mutablePrimaryHWVsyncEnabled() { return mPrimaryHWVsyncEnabled; }
auto& mutableHWVsyncAvailable() { return mHWVsyncAvailable; }
- auto& mutableLayerHistory() { return mLayerHistory; }
+ auto refreshRateSelector() { return holdRefreshRateSelector(); }
+ bool hasRefreshRateSelectors() const { return !mRefreshRateSelectors.empty(); }
- auto& mutableDisplays() { return mDisplays; }
+ auto& mutableLayerHistory() { return mLayerHistory; }
size_t layerHistorySize() NO_THREAD_SAFETY_ANALYSIS {
return mLayerHistory.mActiveLayerInfos.size() + mLayerHistory.mInactiveLayerInfos.size();
}
- auto refreshRateSelector() { return holdRefreshRateSelector(); }
-
size_t getNumActiveLayers() NO_THREAD_SAFETY_ANALYSIS {
return mLayerHistory.mActiveLayerInfos.size();
}
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index ff79ce0..35c037c 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -842,7 +842,8 @@
sp<DisplayDevice> display = sp<DisplayDevice>::make(mCreationArgs);
mFlinger.mutableDisplays().emplace_or_replace(mDisplayToken, display);
if (mFlinger.scheduler()) {
- mFlinger.scheduler()->registerDisplay(display);
+ mFlinger.scheduler()->registerDisplay(display->getPhysicalId(),
+ display->holdRefreshRateSelector());
}
DisplayDeviceState state;