[sf] additional fixes for framerate propagation in new fe

We have to update the parents when their child is reparented. The
parent doesn't have a change flag since a child in most cases does
not affect its parent. To ensure framerate gets updated correctly,
always update framerates if there are any hierarchy changes.

Also add more tests to exercise this path.

Test: presubmit
Test: atest android.graphics.cts.FrameRateOverrideTest
Bug: 238781169
Change-Id: I3eefdeefae4e18b5f2f65a6f341481e328f5d1c7

3 files changed, 106 insertions, 6 deletions

diff --git a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp
index 7e678b98fa..23cfe928f5 100644
--- a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp
+++ b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp
@@ -540,7 +540,7 @@ const LayerSnapshot& LayerSnapshotBuilder::updateSnapshotsInHierarchy(
                         primaryDisplayRotationFlags);
         snapshot->changes |= RequestedLayerState::Changes::Created;
     }
-    scheduler::LayerInfo::FrameRate oldFrameRate = snapshot->frameRate;
+
     if (traversalPath.isRelative()) {
         bool parentIsRelative = traversalPath.variant == LayerHierarchy::Variant::Relative;
         updateRelativeState(*snapshot, parentSnapshot, parentIsRelative, args);
@@ -561,9 +561,6 @@ const LayerSnapshot& LayerSnapshotBuilder::updateSnapshotsInHierarchy(
         updateFrameRateFromChildSnapshot(*snapshot, childSnapshot, args);
     }
 
-    if (oldFrameRate == snapshot->frameRate) {
-        snapshot->changes.clear(RequestedLayerState::Changes::FrameRate);
-    }
     return *snapshot;
 }
 
@@ -670,8 +667,10 @@ void LayerSnapshotBuilder::updateFrameRateFromChildSnapshot(LayerSnapshot& snaps
                                                             const LayerSnapshot& childSnapshot,
                                                             const Args& args) {
     if (args.forceUpdate == ForceUpdateFlags::NONE &&
-        !childSnapshot.changes.any(RequestedLayerState::Changes::FrameRate |
-                                   RequestedLayerState::Changes::Hierarchy)) {
+        !args.layerLifecycleManager.getGlobalChanges().any(
+                RequestedLayerState::Changes::Hierarchy) &&
+        !childSnapshot.changes.any(RequestedLayerState::Changes::FrameRate) &&
+        !snapshot.changes.any(RequestedLayerState::Changes::FrameRate)) {
         return;
     }
 
@@ -817,6 +816,8 @@ void LayerSnapshotBuilder::updateSnapshot(LayerSnapshot& snapshot, const Args& a
     }
 
     if (forceUpdate ||
+        args.layerLifecycleManager.getGlobalChanges().any(
+                RequestedLayerState::Changes::Hierarchy) ||
         snapshot.changes.any(RequestedLayerState::Changes::FrameRate |
                              RequestedLayerState::Changes::Hierarchy)) {
         snapshot.frameRate = (requested.requestedFrameRate.rate.isValid() ||
@@ -824,6 +825,7 @@ void LayerSnapshotBuilder::updateSnapshot(LayerSnapshot& snapshot, const Args& a
                                scheduler::LayerInfo::FrameRateCompatibility::NoVote))
                 ? requested.requestedFrameRate
                 : parentSnapshot.frameRate;
+        snapshot.changes |= RequestedLayerState::Changes::FrameRate;
     }
 
     if (forceUpdate || snapshot.clientChanges & layer_state_t::eFrameRateSelectionPriority) {
diff --git a/services/surfaceflinger/tests/unittests/LayerHierarchyTest.h b/services/surfaceflinger/tests/unittests/LayerHierarchyTest.h
index 3234483f14..e475b843c9 100644
--- a/services/surfaceflinger/tests/unittests/LayerHierarchyTest.h
+++ b/services/surfaceflinger/tests/unittests/LayerHierarchyTest.h
@@ -319,6 +319,20 @@ protected:
         mLifecycleManager.applyTransactions(transactions);
     }
 
+    void setFrameRate(uint32_t id, float frameRate, int8_t compatibility,
+                      int8_t changeFrameRateStrategy) {
+        std::vector<TransactionState> transactions;
+        transactions.emplace_back();
+        transactions.back().states.push_back({});
+
+        transactions.back().states.front().state.what = layer_state_t::eFrameRateChanged;
+        transactions.back().states.front().layerId = id;
+        transactions.back().states.front().state.frameRate = frameRate;
+        transactions.back().states.front().state.frameRateCompatibility = compatibility;
+        transactions.back().states.front().state.changeFrameRateStrategy = changeFrameRateStrategy;
+        mLifecycleManager.applyTransactions(transactions);
+    }
+
     LayerLifecycleManager mLifecycleManager;
 };
 
diff --git a/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp b/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp
index d6c4b7229e..a581d5b6ff 100644
--- a/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp
+++ b/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp
@@ -512,4 +512,88 @@ TEST_F(LayerSnapshotTest, frameRateSelectionPriorityPassedToChildLayers) {
     EXPECT_EQ(getSnapshot({.id = 1221})->frameRateSelectionPriority, 1);
 }
 
+TEST_F(LayerSnapshotTest, framerate) {
+    setFrameRate(11, 244.f, 0, 0);
+
+    UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
+    // verify parent is gets no vote
+    EXPECT_FALSE(getSnapshot({.id = 1})->frameRate.rate.isValid());
+    EXPECT_EQ(getSnapshot({.id = 1})->frameRate.type,
+              scheduler::LayerInfo::FrameRateCompatibility::NoVote);
+    EXPECT_TRUE(getSnapshot({.id = 1})->changes.test(RequestedLayerState::Changes::FrameRate));
+
+    // verify layer and children get the requested votes
+    EXPECT_TRUE(getSnapshot({.id = 11})->frameRate.rate.isValid());
+    EXPECT_EQ(getSnapshot({.id = 11})->frameRate.rate.getValue(), 244.f);
+    EXPECT_EQ(getSnapshot({.id = 11})->frameRate.type,
+              scheduler::LayerInfo::FrameRateCompatibility::Default);
+    EXPECT_TRUE(getSnapshot({.id = 11})->changes.test(RequestedLayerState::Changes::FrameRate));
+
+    EXPECT_TRUE(getSnapshot({.id = 111})->frameRate.rate.isValid());
+    EXPECT_EQ(getSnapshot({.id = 111})->frameRate.rate.getValue(), 244.f);
+    EXPECT_EQ(getSnapshot({.id = 111})->frameRate.type,
+              scheduler::LayerInfo::FrameRateCompatibility::Default);
+    EXPECT_TRUE(getSnapshot({.id = 111})->changes.test(RequestedLayerState::Changes::FrameRate));
+
+    // reparent and verify the child gets the new parent's framerate
+    reparentLayer(122, 11);
+
+    std::vector<uint32_t> expected = {1, 11, 111, 122, 1221, 12, 121, 13, 2};
+    UPDATE_AND_VERIFY(mSnapshotBuilder, expected);
+    // verify parent is gets no vote
+    EXPECT_FALSE(getSnapshot({.id = 1})->frameRate.rate.isValid());
+    EXPECT_EQ(getSnapshot({.id = 1})->frameRate.type,
+              scheduler::LayerInfo::FrameRateCompatibility::NoVote);
+
+    // verify layer and children get the requested votes
+    EXPECT_TRUE(getSnapshot({.id = 11})->frameRate.rate.isValid());
+    EXPECT_EQ(getSnapshot({.id = 11})->frameRate.rate.getValue(), 244.f);
+    EXPECT_EQ(getSnapshot({.id = 11})->frameRate.type,
+              scheduler::LayerInfo::FrameRateCompatibility::Default);
+
+    EXPECT_TRUE(getSnapshot({.id = 111})->frameRate.rate.isValid());
+    EXPECT_EQ(getSnapshot({.id = 111})->frameRate.rate.getValue(), 244.f);
+    EXPECT_EQ(getSnapshot({.id = 111})->frameRate.type,
+              scheduler::LayerInfo::FrameRateCompatibility::Default);
+
+    EXPECT_TRUE(getSnapshot({.id = 122})->frameRate.rate.isValid());
+    EXPECT_EQ(getSnapshot({.id = 122})->frameRate.rate.getValue(), 244.f);
+    EXPECT_EQ(getSnapshot({.id = 122})->frameRate.type,
+              scheduler::LayerInfo::FrameRateCompatibility::Default);
+    EXPECT_TRUE(getSnapshot({.id = 122})->changes.test(RequestedLayerState::Changes::FrameRate));
+
+    // reparent and verify the new parent gets no vote
+    reparentLayer(11, 2);
+    expected = {1, 12, 121, 13, 2, 11, 111, 122, 1221};
+    UPDATE_AND_VERIFY(mSnapshotBuilder, expected);
+
+    // verify old parent has invalid framerate (default)
+    EXPECT_FALSE(getSnapshot({.id = 1})->frameRate.rate.isValid());
+    EXPECT_EQ(getSnapshot({.id = 1})->frameRate.type,
+              scheduler::LayerInfo::FrameRateCompatibility::Default);
+    EXPECT_TRUE(getSnapshot({.id = 1})->changes.test(RequestedLayerState::Changes::FrameRate));
+
+    // verify new parent get no vote
+    EXPECT_FALSE(getSnapshot({.id = 2})->frameRate.rate.isValid());
+    EXPECT_EQ(getSnapshot({.id = 2})->frameRate.type,
+              scheduler::LayerInfo::FrameRateCompatibility::NoVote);
+    EXPECT_TRUE(getSnapshot({.id = 2})->changes.test(RequestedLayerState::Changes::FrameRate));
+
+    // verify layer and children get the requested votes (unchanged)
+    EXPECT_TRUE(getSnapshot({.id = 11})->frameRate.rate.isValid());
+    EXPECT_EQ(getSnapshot({.id = 11})->frameRate.rate.getValue(), 244.f);
+    EXPECT_EQ(getSnapshot({.id = 11})->frameRate.type,
+              scheduler::LayerInfo::FrameRateCompatibility::Default);
+
+    EXPECT_TRUE(getSnapshot({.id = 111})->frameRate.rate.isValid());
+    EXPECT_EQ(getSnapshot({.id = 111})->frameRate.rate.getValue(), 244.f);
+    EXPECT_EQ(getSnapshot({.id = 111})->frameRate.type,
+              scheduler::LayerInfo::FrameRateCompatibility::Default);
+
+    EXPECT_TRUE(getSnapshot({.id = 122})->frameRate.rate.isValid());
+    EXPECT_EQ(getSnapshot({.id = 122})->frameRate.rate.getValue(), 244.f);
+    EXPECT_EQ(getSnapshot({.id = 122})->frameRate.type,
+              scheduler::LayerInfo::FrameRateCompatibility::Default);
+}
+
 } // namespace android::surfaceflinger::frontend