Fix NoVote/NoPreference on LayerHistory

Previous CL ag/30285441 was supposed to allow game default override if
the layervote is simply "NoPreference" category. However there was a bug
with the CL.

This CL fixes this and also removes the early skip of NoVote, in order
for explicit NoVote layers to not affect frame rate scoring even if
surface has drawing.

Bug: 378455432
Test: atest libsurfaceflinger_unittest
Test: manual test game with overlay
Test: manual test video with overlay
Test: manual test on both MRR and ARR
Flag: EXEMPT bugfix
Change-Id: Iba36cc89597f544bdc3311424f6e5d04439d7dc7

2 files changed, 216 insertions, 4 deletions

diff --git a/services/surfaceflinger/Scheduler/LayerHistory.cpp b/services/surfaceflinger/Scheduler/LayerHistory.cpp
index e45bdfce13..171342d5e8 100644
--- a/services/surfaceflinger/Scheduler/LayerHistory.cpp
+++ b/services/surfaceflinger/Scheduler/LayerHistory.cpp
@@ -308,11 +308,14 @@ void LayerHistory::partitionLayers(nsecs_t now, bool isVrrDevice) {
                 const auto setFrameRateVoteType =
                         info->isVisible() ? voteType : LayerVoteType::NoVote;
 
-                const bool hasSetFrameRateOpinion = frameRate.isValid() && !frameRate.isNoVote();
+                const bool hasSetFrameRateOpinion =
+                        frameRate.isValuelessType() || frameRate.vote.rate.isValid();
                 const bool hasCategoryOpinion =
                         frameRate.category != FrameRateCategory::NoPreference &&
                         frameRate.category != FrameRateCategory::Default;
-                const bool hasFrameRateOpinion = hasSetFrameRateOpinion || hasCategoryOpinion;
+                const bool hasFrameRateOpinionAboveGameDefault =
+                        hasSetFrameRateOpinion || hasCategoryOpinion;
+                const bool hasFrameRateOpinionArr = frameRate.isValid() && !frameRate.isNoVote();
 
                 if (gameModeFrameRateOverride.isValid()) {
                     info->setLayerVote({gameFrameRateOverrideVoteType, gameModeFrameRateOverride});
@@ -321,7 +324,8 @@ void LayerHistory::partitionLayers(nsecs_t now, bool isVrrDevice) {
                         trace(*info, gameFrameRateOverrideVoteType,
                               gameModeFrameRateOverride.getIntValue());
                     }
-                } else if (hasFrameRateOpinion && frameRate.isVoteValidForMrr(isVrrDevice)) {
+                } else if (hasFrameRateOpinionAboveGameDefault &&
+                           frameRate.isVoteValidForMrr(isVrrDevice)) {
                     info->setLayerVote({setFrameRateVoteType,
                                         isValuelessVote ? 0_Hz : frameRate.vote.rate,
                                         frameRate.vote.seamlessness, frameRate.category});
@@ -337,8 +341,18 @@ void LayerHistory::partitionLayers(nsecs_t now, bool isVrrDevice) {
                         trace(*info, gameFrameRateOverrideVoteType,
                               gameDefaultFrameRateOverride.getIntValue());
                     }
+                } else if (hasFrameRateOpinionArr && frameRate.isVoteValidForMrr(isVrrDevice)) {
+                    // This allows NoPreference votes on ARR devices after considering the
+                    // gameDefaultFrameRateOverride (above).
+                    info->setLayerVote({setFrameRateVoteType,
+                                        isValuelessVote ? 0_Hz : frameRate.vote.rate,
+                                        frameRate.vote.seamlessness, frameRate.category});
+                    if (CC_UNLIKELY(mTraceEnabled)) {
+                        trace(*info, gameFrameRateOverrideVoteType,
+                              frameRate.vote.rate.getIntValue());
+                    }
                 } else {
-                    if (hasFrameRateOpinion && !frameRate.isVoteValidForMrr(isVrrDevice)) {
+                    if (hasFrameRateOpinionArr && !frameRate.isVoteValidForMrr(isVrrDevice)) {
                         SFTRACE_FORMAT_INSTANT("Reset layer to ignore explicit vote on MRR %s: %s "
                                                "%s %s",
                                                info->getName().c_str(),
diff --git a/services/surfaceflinger/tests/unittests/LayerHistoryIntegrationTest.cpp b/services/surfaceflinger/tests/unittests/LayerHistoryIntegrationTest.cpp
index de37b6342c..767000e6b2 100644
--- a/services/surfaceflinger/tests/unittests/LayerHistoryIntegrationTest.cpp
+++ b/services/surfaceflinger/tests/unittests/LayerHistoryIntegrationTest.cpp
@@ -715,6 +715,204 @@ TEST_F(LayerHistoryIntegrationTest, oneLayerCategoryNoPreference) {
     EXPECT_EQ(0, frequentLayerCount(time));
 }
 
+// Tests MRR NoPreference-only vote, no game default override. Expects vote reset.
+TEST_F(LayerHistoryIntegrationTest, oneLayerCategoryNoPreference_mrr) {
+    SET_FLAG_FOR_TEST(flags::frame_rate_category_mrr, false);
+    SET_FLAG_FOR_TEST(flags::game_default_frame_rate, true);
+    SET_FLAG_FOR_TEST(flags::vrr_config, true);
+
+    const LayerHistory::LayerVoteType defaultVote = LayerHistory::LayerVoteType::Min;
+
+    auto layer = createLegacyAndFrontedEndLayer(1);
+    setDefaultLayerVote(layer.get(), defaultVote);
+    showLayer(1);
+    setFrameRate(1, (0_Hz).getValue(), ANATIVEWINDOW_FRAME_RATE_COMPATIBILITY_DEFAULT,
+                 ANATIVEWINDOW_CHANGE_FRAME_RATE_ONLY_IF_SEAMLESS);
+    setFrameRateCategory(1, ANATIVEWINDOW_FRAME_RATE_CATEGORY_NO_PREFERENCE);
+
+    EXPECT_EQ(1u, layerCount());
+    EXPECT_EQ(0u, activeLayerCount());
+
+    nsecs_t time = systemTime();
+    for (size_t i = 0; i < PRESENT_TIME_HISTORY_SIZE; i++) {
+        setBufferWithPresentTime(layer, time);
+        time += HI_FPS_PERIOD;
+    }
+
+    EXPECT_EQ(1u, summarizeLayerHistory(time).size());
+    EXPECT_EQ(1u, activeLayerCount());
+    EXPECT_EQ(1, frequentLayerCount(time));
+    EXPECT_EQ(defaultVote, summarizeLayerHistory(time)[0].vote);
+    EXPECT_EQ(0_Hz, summarizeLayerHistory(time)[0].desiredRefreshRate);
+    EXPECT_EQ(FrameRateCategory::Default, summarizeLayerHistory(time)[0].frameRateCategory);
+}
+
+// Tests VRR NoPreference-only vote, no game default override. Expects NoPreference, *not* vote
+// reset.
+TEST_F(LayerHistoryIntegrationTest, oneLayerCategoryNoPreference_vrr) {
+    SET_FLAG_FOR_TEST(flags::frame_rate_category_mrr, false);
+    SET_FLAG_FOR_TEST(flags::game_default_frame_rate, true);
+    SET_FLAG_FOR_TEST(flags::vrr_config, true);
+    mSelector->setActiveMode(kVrrModeId, HI_FPS);
+
+    const LayerHistory::LayerVoteType defaultVote = LayerHistory::LayerVoteType::Min;
+
+    auto layer = createLegacyAndFrontedEndLayer(1);
+    setDefaultLayerVote(layer.get(), defaultVote);
+    showLayer(1);
+    setFrameRate(1, (0_Hz).getValue(), ANATIVEWINDOW_FRAME_RATE_COMPATIBILITY_DEFAULT,
+                 ANATIVEWINDOW_CHANGE_FRAME_RATE_ONLY_IF_SEAMLESS);
+    setFrameRateCategory(1, ANATIVEWINDOW_FRAME_RATE_CATEGORY_NO_PREFERENCE);
+
+    EXPECT_EQ(1u, layerCount());
+    EXPECT_EQ(0u, activeLayerCount());
+
+    nsecs_t time = systemTime();
+    for (size_t i = 0; i < PRESENT_TIME_HISTORY_SIZE; i++) {
+        setBufferWithPresentTime(layer, time);
+        time += HI_FPS_PERIOD;
+    }
+
+    EXPECT_EQ(1u, summarizeLayerHistory(time).size());
+    EXPECT_EQ(1u, activeLayerCount());
+    EXPECT_EQ(1, frequentLayerCount(time));
+    EXPECT_EQ(LayerHistory::LayerVoteType::ExplicitCategory, summarizeLayerHistory(time)[0].vote);
+    EXPECT_EQ(0_Hz, summarizeLayerHistory(time)[0].desiredRefreshRate);
+    EXPECT_EQ(FrameRateCategory::NoPreference, summarizeLayerHistory(time)[0].frameRateCategory);
+}
+
+TEST_F(LayerHistoryIntegrationTest, oneLayerCategoryNoPreferenceWithGameDefault_vrr) {
+    SET_FLAG_FOR_TEST(flags::frame_rate_category_mrr, false);
+    SET_FLAG_FOR_TEST(flags::game_default_frame_rate, true);
+    SET_FLAG_FOR_TEST(flags::vrr_config, true);
+    mSelector->setActiveMode(kVrrModeId, HI_FPS);
+
+    const Fps gameDefaultFrameRate = Fps::fromValue(30.0f);
+    const uid_t uid = 456;
+
+    history().updateGameDefaultFrameRateOverride(
+            FrameRateOverride({uid, gameDefaultFrameRate.getValue()}));
+
+    auto layer = createLegacyAndFrontedEndLayerWithUid(1, gui::Uid(uid));
+    showLayer(1);
+    setFrameRate(1, (0_Hz).getValue(), ANATIVEWINDOW_FRAME_RATE_COMPATIBILITY_DEFAULT,
+                 ANATIVEWINDOW_CHANGE_FRAME_RATE_ONLY_IF_SEAMLESS);
+    setFrameRateCategory(1, ANATIVEWINDOW_FRAME_RATE_CATEGORY_NO_PREFERENCE);
+
+    EXPECT_EQ(1u, layerCount());
+    EXPECT_EQ(0u, activeLayerCount());
+
+    nsecs_t time = systemTime();
+    for (size_t i = 0; i < PRESENT_TIME_HISTORY_SIZE; i++) {
+        setBufferWithPresentTime(layer, time);
+        time += HI_FPS_PERIOD;
+    }
+
+    EXPECT_EQ(1u, summarizeLayerHistory(time).size());
+    EXPECT_EQ(1u, activeLayerCount());
+    EXPECT_EQ(1, frequentLayerCount(time));
+    EXPECT_EQ(LayerHistory::LayerVoteType::ExplicitDefault, summarizeLayerHistory(time)[0].vote);
+    EXPECT_EQ(gameDefaultFrameRate, summarizeLayerHistory(time)[0].desiredRefreshRate);
+    EXPECT_EQ(FrameRateCategory::Default, summarizeLayerHistory(time)[0].frameRateCategory);
+}
+
+TEST_F(LayerHistoryIntegrationTest, oneLayerCategoryNoPreferenceWithGameDefault_mrr) {
+    SET_FLAG_FOR_TEST(flags::frame_rate_category_mrr, false);
+    SET_FLAG_FOR_TEST(flags::game_default_frame_rate, true);
+    SET_FLAG_FOR_TEST(flags::vrr_config, true);
+
+    const Fps gameDefaultFrameRate = Fps::fromValue(30.0f);
+    const uid_t uid = 456;
+
+    history().updateGameDefaultFrameRateOverride(
+            FrameRateOverride({uid, gameDefaultFrameRate.getValue()}));
+
+    auto layer = createLegacyAndFrontedEndLayerWithUid(1, gui::Uid(uid));
+    showLayer(1);
+    setFrameRate(1, (0_Hz).getValue(), ANATIVEWINDOW_FRAME_RATE_COMPATIBILITY_DEFAULT,
+                 ANATIVEWINDOW_CHANGE_FRAME_RATE_ONLY_IF_SEAMLESS);
+    setFrameRateCategory(1, ANATIVEWINDOW_FRAME_RATE_CATEGORY_NO_PREFERENCE);
+
+    EXPECT_EQ(1u, layerCount());
+    EXPECT_EQ(0u, activeLayerCount());
+
+    nsecs_t time = systemTime();
+    for (size_t i = 0; i < PRESENT_TIME_HISTORY_SIZE; i++) {
+        setBufferWithPresentTime(layer, time);
+        time += HI_FPS_PERIOD;
+    }
+
+    EXPECT_EQ(1u, summarizeLayerHistory(time).size());
+    EXPECT_EQ(1u, activeLayerCount());
+    EXPECT_EQ(1, frequentLayerCount(time));
+    EXPECT_EQ(LayerHistory::LayerVoteType::ExplicitDefault, summarizeLayerHistory(time)[0].vote);
+    EXPECT_EQ(gameDefaultFrameRate, summarizeLayerHistory(time)[0].desiredRefreshRate);
+    EXPECT_EQ(FrameRateCategory::Default, summarizeLayerHistory(time)[0].frameRateCategory);
+}
+
+TEST_F(LayerHistoryIntegrationTest, oneLayerNoVoteWithGameDefault_vrr) {
+    SET_FLAG_FOR_TEST(flags::frame_rate_category_mrr, false);
+    SET_FLAG_FOR_TEST(flags::game_default_frame_rate, true);
+    SET_FLAG_FOR_TEST(flags::vrr_config, true);
+    mSelector->setActiveMode(kVrrModeId, HI_FPS);
+
+    const Fps gameDefaultFrameRate = Fps::fromValue(30.0f);
+    const uid_t uid = 456;
+
+    history().updateGameDefaultFrameRateOverride(
+            FrameRateOverride({uid, gameDefaultFrameRate.getValue()}));
+
+    auto layer = createLegacyAndFrontedEndLayerWithUid(1, gui::Uid(uid));
+    showLayer(1);
+    setFrameRate(1, (0_Hz).getValue(), ANATIVEWINDOW_FRAME_RATE_NO_VOTE,
+                 ANATIVEWINDOW_CHANGE_FRAME_RATE_ONLY_IF_SEAMLESS);
+
+    EXPECT_EQ(1u, layerCount());
+    EXPECT_EQ(0u, activeLayerCount());
+
+    nsecs_t time = systemTime();
+    for (size_t i = 0; i < PRESENT_TIME_HISTORY_SIZE; i++) {
+        setBufferWithPresentTime(layer, time);
+        time += HI_FPS_PERIOD;
+    }
+
+    // Expect NoVote to be skipped in summarize.
+    EXPECT_EQ(0u, summarizeLayerHistory(time).size());
+    EXPECT_EQ(1u, activeLayerCount());
+    EXPECT_EQ(1, frequentLayerCount(time));
+}
+
+TEST_F(LayerHistoryIntegrationTest, oneLayerNoVoteWithGameDefault_mrr) {
+    SET_FLAG_FOR_TEST(flags::frame_rate_category_mrr, false);
+    SET_FLAG_FOR_TEST(flags::game_default_frame_rate, true);
+    SET_FLAG_FOR_TEST(flags::vrr_config, true);
+
+    const Fps gameDefaultFrameRate = Fps::fromValue(30.0f);
+    const uid_t uid = 456;
+
+    history().updateGameDefaultFrameRateOverride(
+            FrameRateOverride({uid, gameDefaultFrameRate.getValue()}));
+
+    auto layer = createLegacyAndFrontedEndLayerWithUid(1, gui::Uid(uid));
+    showLayer(1);
+    setFrameRate(1, (0_Hz).getValue(), ANATIVEWINDOW_FRAME_RATE_NO_VOTE,
+                 ANATIVEWINDOW_CHANGE_FRAME_RATE_ONLY_IF_SEAMLESS);
+
+    EXPECT_EQ(1u, layerCount());
+    EXPECT_EQ(0u, activeLayerCount());
+
+    nsecs_t time = systemTime();
+    for (size_t i = 0; i < PRESENT_TIME_HISTORY_SIZE; i++) {
+        setBufferWithPresentTime(layer, time);
+        time += HI_FPS_PERIOD;
+    }
+
+    // Expect NoVote to be skipped in summarize.
+    EXPECT_EQ(0u, summarizeLayerHistory(time).size());
+    EXPECT_EQ(1u, activeLayerCount());
+    EXPECT_EQ(1, frequentLayerCount(time));
+}
+
 TEST_F(LayerHistoryIntegrationTest, oneLayerExplicitVoteWithCategory) {
     SET_FLAG_FOR_TEST(flags::frame_rate_category_mrr, true);