Fix framerate propagation from child to parent layers
We were not resetting the framerate vote of the parent layer
after the child frame rate vote was removed. Fix this and
add tests to verify this behavior does not regress.
Flag: EXEMPT (removing old flags)
Bug: 330785038
Test: presubmit
Change-Id: I88d60484d4dfdfa4b09c98484867a49545f60257
diff --git a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp
index 47fd700..c874db3 100644
--- a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp
+++ b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp
@@ -561,6 +561,7 @@
updateSnapshot(*snapshot, args, *layer, parentSnapshot, traversalPath);
}
+ bool childHasValidFrameRate = false;
for (auto& [childHierarchy, variant] : hierarchy.mChildren) {
LayerHierarchy::ScopedAddToTraversalPath addChildToPath(traversalPath,
childHierarchy->getLayer()->id,
@@ -568,7 +569,8 @@
const LayerSnapshot& childSnapshot =
updateSnapshotsInHierarchy(args, *childHierarchy, traversalPath, *snapshot,
depth + 1);
- updateFrameRateFromChildSnapshot(*snapshot, childSnapshot, args);
+ updateFrameRateFromChildSnapshot(*snapshot, childSnapshot, *childHierarchy->getLayer(),
+ args, &childHasValidFrameRate);
}
return *snapshot;
@@ -675,9 +677,10 @@
}
}
-void LayerSnapshotBuilder::updateFrameRateFromChildSnapshot(LayerSnapshot& snapshot,
- const LayerSnapshot& childSnapshot,
- const Args& args) {
+void LayerSnapshotBuilder::updateFrameRateFromChildSnapshot(
+ LayerSnapshot& snapshot, const LayerSnapshot& childSnapshot,
+ const RequestedLayerState& /* requestedChildState */, const Args& args,
+ bool* outChildHasValidFrameRate) {
if (args.forceUpdate == ForceUpdateFlags::NONE &&
!args.layerLifecycleManager.getGlobalChanges().any(
RequestedLayerState::Changes::Hierarchy) &&
@@ -687,7 +690,7 @@
}
using FrameRateCompatibility = scheduler::FrameRateCompatibility;
- if (snapshot.frameRate.isValid()) {
+ if (snapshot.inheritedFrameRate.isValid() || *outChildHasValidFrameRate) {
// we already have a valid framerate.
return;
}
@@ -704,13 +707,18 @@
const auto layerVotedWithExactCompatibility = childSnapshot.frameRate.vote.rate.isValid() &&
childSnapshot.frameRate.vote.type == FrameRateCompatibility::Exact;
- bool childHasValidFrameRate = layerVotedWithDefaultCompatibility || layerVotedWithNoVote ||
+ *outChildHasValidFrameRate |= layerVotedWithDefaultCompatibility || layerVotedWithNoVote ||
layerVotedWithCategory || layerVotedWithExactCompatibility;
// If we don't have a valid frame rate, but the children do, we set this
// layer as NoVote to allow the children to control the refresh rate
- if (childHasValidFrameRate) {
- snapshot.frameRate = scheduler::LayerInfo::FrameRate(Fps(), FrameRateCompatibility::NoVote);
+ static const auto noVote =
+ scheduler::LayerInfo::FrameRate(Fps(), FrameRateCompatibility::NoVote);
+ if (*outChildHasValidFrameRate) {
+ snapshot.frameRate = noVote;
+ snapshot.changes |= RequestedLayerState::Changes::FrameRate;
+ } else if (snapshot.frameRate != snapshot.inheritedFrameRate) {
+ snapshot.frameRate = snapshot.inheritedFrameRate;
snapshot.changes |= RequestedLayerState::Changes::FrameRate;
}
}
diff --git a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.h b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.h
index f3c56a4..207e23a 100644
--- a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.h
+++ b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.h
@@ -130,7 +130,9 @@
const RequestedLayerState& layer,
const LayerSnapshot& parentSnapshot);
void updateFrameRateFromChildSnapshot(LayerSnapshot& snapshot,
- const LayerSnapshot& childSnapshot, const Args& args);
+ const LayerSnapshot& childSnapshot,
+ const RequestedLayerState& requestedCHildState,
+ const Args& args, bool* outChildHasValidFrameRate);
void updateTouchableRegionCrop(const Args& args);
std::unordered_map<LayerHierarchy::TraversalPath, LayerSnapshot*,
diff --git a/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp b/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp
index 2bb864a..2860345 100644
--- a/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp
+++ b/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp
@@ -56,6 +56,17 @@
class LayerSnapshotTest : public LayerSnapshotTestBase {
protected:
+ const Layer::FrameRate FRAME_RATE_VOTE1 =
+ Layer::FrameRate(67_Hz, scheduler::FrameRateCompatibility::Default);
+ const Layer::FrameRate FRAME_RATE_VOTE2 =
+ Layer::FrameRate(14_Hz, scheduler::FrameRateCompatibility::Default);
+ const Layer::FrameRate FRAME_RATE_VOTE3 =
+ Layer::FrameRate(99_Hz, scheduler::FrameRateCompatibility::Default);
+ const Layer::FrameRate FRAME_RATE_TREE =
+ Layer::FrameRate(Fps(), scheduler::FrameRateCompatibility::NoVote);
+ const Layer::FrameRate FRAME_RATE_NO_VOTE =
+ Layer::FrameRate(Fps(), scheduler::FrameRateCompatibility::Default);
+
LayerSnapshotTest() : LayerSnapshotTestBase() {
UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
}
@@ -787,6 +798,155 @@
scheduler::FrameRateCompatibility::Default);
}
+TEST_F(LayerSnapshotTest, frameRateSetAndGet) {
+ setFrameRate(1, FRAME_RATE_VOTE1.vote.rate.getValue(), 0, 0);
+ UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
+ // verify parent is gets no vote
+ EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_VOTE1);
+}
+
+TEST_F(LayerSnapshotTest, frameRateSetAndGetParent) {
+ setFrameRate(111, FRAME_RATE_VOTE1.vote.rate.getValue(), 0, 0);
+ UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
+ EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_TREE);
+ EXPECT_EQ(getSnapshot({.id = 11})->frameRate, FRAME_RATE_TREE);
+ EXPECT_EQ(getSnapshot({.id = 111})->frameRate, FRAME_RATE_VOTE1);
+
+ setFrameRate(111, FRAME_RATE_NO_VOTE.vote.rate.getValue(), 0, 0);
+ UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
+ EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_NO_VOTE);
+ EXPECT_EQ(getSnapshot({.id = 11})->frameRate, FRAME_RATE_NO_VOTE);
+ EXPECT_EQ(getSnapshot({.id = 111})->frameRate, FRAME_RATE_NO_VOTE);
+}
+
+TEST_F(LayerSnapshotTest, frameRateSetAndGetParentAllVote) {
+ setFrameRate(1, FRAME_RATE_VOTE3.vote.rate.getValue(), 0, 0);
+ setFrameRate(11, FRAME_RATE_VOTE2.vote.rate.getValue(), 0, 0);
+ setFrameRate(111, FRAME_RATE_VOTE1.vote.rate.getValue(), 0, 0);
+ UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
+
+ EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_VOTE3);
+ EXPECT_EQ(getSnapshot({.id = 11})->frameRate, FRAME_RATE_VOTE2);
+ EXPECT_EQ(getSnapshot({.id = 111})->frameRate, FRAME_RATE_VOTE1);
+
+ setFrameRate(111, FRAME_RATE_NO_VOTE.vote.rate.getValue(), 0, 0);
+ UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
+ EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_VOTE3);
+ EXPECT_EQ(getSnapshot({.id = 11})->frameRate, FRAME_RATE_VOTE2);
+ EXPECT_EQ(getSnapshot({.id = 111})->frameRate, FRAME_RATE_VOTE2);
+
+ setFrameRate(11, FRAME_RATE_NO_VOTE.vote.rate.getValue(), 0, 0);
+ UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
+ EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_VOTE3);
+ EXPECT_EQ(getSnapshot({.id = 11})->frameRate, FRAME_RATE_VOTE3);
+ EXPECT_EQ(getSnapshot({.id = 111})->frameRate, FRAME_RATE_VOTE3);
+
+ setFrameRate(1, FRAME_RATE_NO_VOTE.vote.rate.getValue(), 0, 0);
+ UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
+ EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_NO_VOTE);
+ EXPECT_EQ(getSnapshot({.id = 11})->frameRate, FRAME_RATE_NO_VOTE);
+ EXPECT_EQ(getSnapshot({.id = 111})->frameRate, FRAME_RATE_NO_VOTE);
+}
+
+TEST_F(LayerSnapshotTest, frameRateSetAndGetChild) {
+ setFrameRate(1, FRAME_RATE_VOTE1.vote.rate.getValue(), 0, 0);
+ UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
+ EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_VOTE1);
+ EXPECT_EQ(getSnapshot({.id = 11})->frameRate, FRAME_RATE_VOTE1);
+ EXPECT_EQ(getSnapshot({.id = 111})->frameRate, FRAME_RATE_VOTE1);
+
+ setFrameRate(1, FRAME_RATE_NO_VOTE.vote.rate.getValue(), 0, 0);
+ UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
+ EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_NO_VOTE);
+ EXPECT_EQ(getSnapshot({.id = 11})->frameRate, FRAME_RATE_NO_VOTE);
+ EXPECT_EQ(getSnapshot({.id = 111})->frameRate, FRAME_RATE_NO_VOTE);
+}
+
+TEST_F(LayerSnapshotTest, frameRateSetAndGetChildAllVote) {
+ setFrameRate(1, FRAME_RATE_VOTE3.vote.rate.getValue(), 0, 0);
+ setFrameRate(11, FRAME_RATE_VOTE2.vote.rate.getValue(), 0, 0);
+ setFrameRate(111, FRAME_RATE_VOTE1.vote.rate.getValue(), 0, 0);
+ UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
+ EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_VOTE3);
+ EXPECT_EQ(getSnapshot({.id = 11})->frameRate, FRAME_RATE_VOTE2);
+ EXPECT_EQ(getSnapshot({.id = 111})->frameRate, FRAME_RATE_VOTE1);
+
+ setFrameRate(1, FRAME_RATE_NO_VOTE.vote.rate.getValue(), 0, 0);
+ UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
+ EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_TREE);
+ EXPECT_EQ(getSnapshot({.id = 11})->frameRate, FRAME_RATE_VOTE2);
+ EXPECT_EQ(getSnapshot({.id = 111})->frameRate, FRAME_RATE_VOTE1);
+
+ setFrameRate(11, FRAME_RATE_NO_VOTE.vote.rate.getValue(), 0, 0);
+ UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
+ EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_TREE);
+ EXPECT_EQ(getSnapshot({.id = 11})->frameRate, FRAME_RATE_TREE);
+ EXPECT_EQ(getSnapshot({.id = 111})->frameRate, FRAME_RATE_VOTE1);
+
+ setFrameRate(111, FRAME_RATE_NO_VOTE.vote.rate.getValue(), 0, 0);
+ UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
+ EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_NO_VOTE);
+ EXPECT_EQ(getSnapshot({.id = 11})->frameRate, FRAME_RATE_NO_VOTE);
+ EXPECT_EQ(getSnapshot({.id = 111})->frameRate, FRAME_RATE_NO_VOTE);
+}
+
+TEST_F(LayerSnapshotTest, frameRateSetAndGetChildAddAfterVote) {
+ setFrameRate(1, FRAME_RATE_VOTE1.vote.rate.getValue(), 0, 0);
+ reparentLayer(111, 2);
+ std::vector<uint32_t> traversalOrder = {1, 11, 12, 121, 122, 1221, 13, 2, 111};
+ UPDATE_AND_VERIFY(mSnapshotBuilder, traversalOrder);
+ EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_VOTE1);
+ EXPECT_EQ(getSnapshot({.id = 11})->frameRate, FRAME_RATE_VOTE1);
+ EXPECT_EQ(getSnapshot({.id = 111})->frameRate, FRAME_RATE_NO_VOTE);
+
+ reparentLayer(111, 11);
+ UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
+ EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_VOTE1);
+ EXPECT_EQ(getSnapshot({.id = 11})->frameRate, FRAME_RATE_VOTE1);
+ EXPECT_EQ(getSnapshot({.id = 111})->frameRate, FRAME_RATE_VOTE1);
+
+ setFrameRate(1, FRAME_RATE_NO_VOTE.vote.rate.getValue(), 0, 0);
+ UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
+ EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_NO_VOTE);
+ EXPECT_EQ(getSnapshot({.id = 11})->frameRate, FRAME_RATE_NO_VOTE);
+ EXPECT_EQ(getSnapshot({.id = 111})->frameRate, FRAME_RATE_NO_VOTE);
+}
+
+TEST_F(LayerSnapshotTest, frameRateSetAndGetChildRemoveAfterVote) {
+ setFrameRate(1, FRAME_RATE_VOTE1.vote.rate.getValue(), 0, 0);
+ UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
+ EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_VOTE1);
+ EXPECT_EQ(getSnapshot({.id = 11})->frameRate, FRAME_RATE_VOTE1);
+ EXPECT_EQ(getSnapshot({.id = 111})->frameRate, FRAME_RATE_VOTE1);
+
+ reparentLayer(111, 2);
+ std::vector<uint32_t> traversalOrder = {1, 11, 12, 121, 122, 1221, 13, 2, 111};
+ UPDATE_AND_VERIFY(mSnapshotBuilder, traversalOrder);
+ EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_VOTE1);
+ EXPECT_EQ(getSnapshot({.id = 11})->frameRate, FRAME_RATE_VOTE1);
+ EXPECT_EQ(getSnapshot({.id = 111})->frameRate, FRAME_RATE_NO_VOTE);
+
+ setFrameRate(1, FRAME_RATE_NO_VOTE.vote.rate.getValue(), 0, 0);
+ UPDATE_AND_VERIFY(mSnapshotBuilder, traversalOrder);
+ EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_NO_VOTE);
+ EXPECT_EQ(getSnapshot({.id = 11})->frameRate, FRAME_RATE_NO_VOTE);
+ EXPECT_EQ(getSnapshot({.id = 111})->frameRate, FRAME_RATE_NO_VOTE);
+}
+
+TEST_F(LayerSnapshotTest, frameRateAddChildForParentWithTreeVote) {
+ setFrameRate(11, FRAME_RATE_VOTE1.vote.rate.getValue(), 0, 0);
+ UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
+ EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_TREE);
+ EXPECT_EQ(getSnapshot({.id = 11})->frameRate, FRAME_RATE_VOTE1);
+ EXPECT_EQ(getSnapshot({.id = 12})->frameRate, FRAME_RATE_NO_VOTE);
+
+ setFrameRate(11, FRAME_RATE_NO_VOTE.vote.rate.getValue(), 0, 0);
+ UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
+ EXPECT_EQ(getSnapshot({.id = 1})->frameRate, FRAME_RATE_NO_VOTE);
+ EXPECT_EQ(getSnapshot({.id = 11})->frameRate, FRAME_RATE_NO_VOTE);
+ EXPECT_EQ(getSnapshot({.id = 12})->frameRate, FRAME_RATE_NO_VOTE);
+}
+
TEST_F(LayerSnapshotTest, translateDataspace) {
setDataspace(1, ui::Dataspace::UNKNOWN);
UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
@@ -965,7 +1125,7 @@
EXPECT_FALSE(getSnapshot({.id = 1})->frameRate.vote.rate.isValid());
EXPECT_EQ(getSnapshot({.id = 1})->frameRate.vote.type,
scheduler::FrameRateCompatibility::NoVote);
- EXPECT_FALSE(getSnapshot({.id = 1})->changes.test(RequestedLayerState::Changes::FrameRate));
+ EXPECT_TRUE(getSnapshot({.id = 1})->changes.test(RequestedLayerState::Changes::FrameRate));
// verify layer 12 and all descendants (121, 122, 1221) get the requested vote
EXPECT_FALSE(getSnapshot({.id = 12})->frameRate.vote.rate.isValid());
@@ -1056,7 +1216,7 @@
EXPECT_EQ(getSnapshot({.id = 1})->frameRate.vote.type,
scheduler::FrameRateCompatibility::NoVote);
EXPECT_EQ(getSnapshot({.id = 1})->frameRate.category, FrameRateCategory::Default);
- EXPECT_FALSE(getSnapshot({.id = 1})->changes.test(RequestedLayerState::Changes::FrameRate));
+ EXPECT_TRUE(getSnapshot({.id = 1})->changes.test(RequestedLayerState::Changes::FrameRate));
// verify layer 12 and all descendants (121, 122, 1221) get the requested vote
EXPECT_FALSE(getSnapshot({.id = 12})->frameRate.vote.rate.isValid());