SF: fix layer activation with scheduler
When a layer frame rate vote changes (via setFrameRate call),
the layers needs to be activated in scheduler. This change fixes a
bug that was activating all layers instead of just the layers that
has a vote change
Bug: 163079696
Bug: 180014293
Test: run a test app that calls setFrameRate
Change-Id: Iaa538d04c535b185161a24a4bbadeff2ac99bcd8
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index 95ab394..141a112 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -1468,7 +1468,7 @@
// First traverse the tree and count how many layers has votes. In addition
// activate the layers in Scheduler's LayerHistory for it to check for changes
int layersWithVote = 0;
- traverseTree([&layersWithVote, this](Layer* layer) {
+ traverseTree([&layersWithVote](Layer* layer) {
const auto layerVotedWithDefaultCompatibility =
layer->mCurrentState.frameRate.rate.isValid() &&
layer->mCurrentState.frameRate.type == FrameRateCompatibility::Default;
@@ -1484,20 +1484,21 @@
layerVotedWithExactCompatibility) {
layersWithVote++;
}
-
- mFlinger->mScheduler->recordLayerHistory(layer, systemTime(),
- LayerHistory::LayerUpdateType::SetFrameRate);
});
// Now update the other layers
bool transactionNeeded = false;
- traverseTree([layersWithVote, &transactionNeeded](Layer* layer) {
- if (layer->mCurrentState.treeHasFrameRateVote != layersWithVote > 0) {
+ traverseTree([layersWithVote, &transactionNeeded, this](Layer* layer) {
+ const bool treeHasFrameRateVote = layersWithVote > 0;
+ if (layer->mCurrentState.treeHasFrameRateVote != treeHasFrameRateVote) {
layer->mCurrentState.sequence++;
- layer->mCurrentState.treeHasFrameRateVote = layersWithVote > 0;
+ layer->mCurrentState.treeHasFrameRateVote = treeHasFrameRateVote;
layer->mCurrentState.modified = true;
layer->setTransactionFlags(eTransactionNeeded);
transactionNeeded = true;
+
+ mFlinger->mScheduler->recordLayerHistory(layer, systemTime(),
+ LayerHistory::LayerUpdateType::SetFrameRate);
}
});
diff --git a/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp b/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp
index a6d07d0..5c8c2d8 100644
--- a/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp
@@ -81,7 +81,7 @@
std::string name() override { return "BufferStateLayer"; }
sp<Layer> createLayer(TestableSurfaceFlinger& flinger) override {
sp<Client> client;
- LayerCreationArgs args(flinger.flinger(), client, "buffer-queue-layer", WIDTH, HEIGHT,
+ LayerCreationArgs args(flinger.flinger(), client, "buffer-state-layer", WIDTH, HEIGHT,
LAYER_FLAGS, LayerMetadata());
return new BufferStateLayer(args);
}
@@ -188,7 +188,8 @@
.WillRepeatedly(Return(FakeHwcDisplayInjector::DEFAULT_VSYNC_PERIOD));
EXPECT_CALL(*vsyncTracker, nextAnticipatedVSyncTimeFrom(_)).WillRepeatedly(Return(0));
mFlinger.setupScheduler(std::move(vsyncController), std::move(vsyncTracker),
- std::move(eventThread), std::move(sfEventThread));
+ std::move(eventThread), std::move(sfEventThread), /*callback*/ nullptr,
+ /*hasMultipleModes*/ true);
}
void SetFrameRateTest::setupComposer(uint32_t virtualDisplayCount) {
@@ -488,5 +489,28 @@
EXPECT_FALSE(ValidateFrameRate(60.0f, ANATIVEWINDOW_FRAME_RATE_EXACT, ""));
}
+TEST_P(SetFrameRateTest, SetOnParentActivatesTree) {
+ const auto& layerFactory = GetParam();
+
+ auto parent = mLayers.emplace_back(layerFactory->createLayer(mFlinger));
+ if (!parent->isVisible()) {
+ // This is a hack as all the test layers except EffectLayer are not visible,
+ // but since the logic is unified in Layer, it should be fine.
+ return;
+ }
+
+ auto child = mLayers.emplace_back(layerFactory->createLayer(mFlinger));
+ addChild(parent, child);
+
+ parent->setFrameRate(FRAME_RATE_VOTE1);
+ commitTransaction();
+
+ const auto layerHistorySummary =
+ mFlinger.mutableScheduler().mutableLayerHistory()->summarize(0);
+ ASSERT_EQ(2u, layerHistorySummary.size());
+ EXPECT_TRUE(FRAME_RATE_VOTE1.rate.equalsWithMargin(layerHistorySummary[0].desiredRefreshRate));
+ EXPECT_TRUE(FRAME_RATE_VOTE1.rate.equalsWithMargin(layerHistorySummary[1].desiredRefreshRate));
+}
+
} // namespace
} // namespace android
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index 2ba6490..63baf7d 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -246,6 +246,8 @@
void resetScheduler(Scheduler* scheduler) { mFlinger->mScheduler.reset(scheduler); }
+ TestableScheduler& mutableScheduler() const { return *mScheduler; }
+
using CreateBufferQueueFunction = surfaceflinger::test::Factory::CreateBufferQueueFunction;
void setCreateBufferQueueFunction(CreateBufferQueueFunction f) {
mFactory.mCreateBufferQueue = f;