Fix FrameRateCategory::NoPreference in SF
Decided the behavior should not be the same as LayerVoteType::NoVote.
Bug: 300152572
Test: atest libsurfaceflinger_unittest
Change-Id: I2e0a0d4959f1fdba3f5f7d04ca15ea9045d4cd36
diff --git a/services/surfaceflinger/Scheduler/LayerHistory.cpp b/services/surfaceflinger/Scheduler/LayerHistory.cpp
index 23eb31f..6b5327a 100644
--- a/services/surfaceflinger/Scheduler/LayerHistory.cpp
+++ b/services/surfaceflinger/Scheduler/LayerHistory.cpp
@@ -41,7 +41,7 @@
bool isLayerActive(const LayerInfo& info, nsecs_t threshold) {
// Layers with an explicit frame rate or frame rate category are always kept active,
- // but ignore NoVote/NoPreference.
+ // but ignore NoVote.
if (info.getSetFrameRateVote().isValid() && !info.getSetFrameRateVote().isNoVote()) {
return true;
}
diff --git a/services/surfaceflinger/Scheduler/LayerInfo.cpp b/services/surfaceflinger/Scheduler/LayerInfo.cpp
index bffef9b..0784251 100644
--- a/services/surfaceflinger/Scheduler/LayerInfo.cpp
+++ b/services/surfaceflinger/Scheduler/LayerInfo.cpp
@@ -491,8 +491,7 @@
}
bool LayerInfo::FrameRate::isNoVote() const {
- return vote.type == FrameRateCompatibility::NoVote ||
- category == FrameRateCategory::NoPreference;
+ return vote.type == FrameRateCompatibility::NoVote;
}
bool LayerInfo::FrameRate::isValid() const {
diff --git a/services/surfaceflinger/Scheduler/LayerInfo.h b/services/surfaceflinger/Scheduler/LayerInfo.h
index 1e08ec8..7fe407f 100644
--- a/services/surfaceflinger/Scheduler/LayerInfo.h
+++ b/services/surfaceflinger/Scheduler/LayerInfo.h
@@ -73,7 +73,7 @@
FrameRateCategory category = FrameRateCategory::Default;
// Returns true if the layer explicitly should contribute to frame rate scoring.
- bool isNoVote() const { return RefreshRateSelector::isNoVote(type, category); }
+ bool isNoVote() const { return RefreshRateSelector::isNoVote(type); }
};
using RefreshRateVotes = ftl::SmallVector<LayerInfo::LayerVote, 2>;
diff --git a/services/surfaceflinger/Scheduler/RefreshRateSelector.cpp b/services/surfaceflinger/Scheduler/RefreshRateSelector.cpp
index 3ee6a4d..5a00972 100644
--- a/services/surfaceflinger/Scheduler/RefreshRateSelector.cpp
+++ b/services/surfaceflinger/Scheduler/RefreshRateSelector.cpp
@@ -599,7 +599,8 @@
layer.name.c_str(), ftl::enum_string(layer.vote).c_str(), layer.weight,
layer.desiredRefreshRate.getValue(),
ftl::enum_string(layer.frameRateCategory).c_str());
- if (layer.isNoVote() || layer.vote == LayerVoteType::Min) {
+ if (layer.isNoVote() || layer.frameRateCategory == FrameRateCategory::NoPreference ||
+ layer.vote == LayerVoteType::Min) {
continue;
}
diff --git a/services/surfaceflinger/Scheduler/RefreshRateSelector.h b/services/surfaceflinger/Scheduler/RefreshRateSelector.h
index 9bcbc0e..5d32414 100644
--- a/services/surfaceflinger/Scheduler/RefreshRateSelector.h
+++ b/services/surfaceflinger/Scheduler/RefreshRateSelector.h
@@ -187,14 +187,12 @@
bool operator!=(const LayerRequirement& other) const { return !(*this == other); }
- bool isNoVote() const { return RefreshRateSelector::isNoVote(vote, frameRateCategory); }
+ bool isNoVote() const { return RefreshRateSelector::isNoVote(vote); }
};
// Returns true if the layer explicitly instructs to not contribute to refresh rate selection.
// In other words, true if the layer should be ignored.
- static bool isNoVote(LayerVoteType vote, FrameRateCategory category) {
- return vote == LayerVoteType::NoVote || category == FrameRateCategory::NoPreference;
- }
+ static bool isNoVote(LayerVoteType vote) { return vote == LayerVoteType::NoVote; }
// Global state describing signals that affect refresh rate choice.
struct GlobalSignals {
diff --git a/services/surfaceflinger/tests/unittests/LayerHistoryTest.cpp b/services/surfaceflinger/tests/unittests/LayerHistoryTest.cpp
index b7996ce..c432ad0 100644
--- a/services/surfaceflinger/tests/unittests/LayerHistoryTest.cpp
+++ b/services/surfaceflinger/tests/unittests/LayerHistoryTest.cpp
@@ -494,14 +494,14 @@
time += HI_FPS_PERIOD;
}
- ASSERT_TRUE(summarizeLayerHistory(time).empty());
+ EXPECT_EQ(1, summarizeLayerHistory(time).size());
EXPECT_EQ(1, activeLayerCount());
EXPECT_EQ(1, frequentLayerCount(time));
// layer became inactive
time += MAX_ACTIVE_LAYER_PERIOD_NS.count();
- ASSERT_TRUE(summarizeLayerHistory(time).empty());
- EXPECT_EQ(0, activeLayerCount());
+ EXPECT_EQ(1, summarizeLayerHistory(time).size());
+ EXPECT_EQ(1, activeLayerCount());
EXPECT_EQ(0, frequentLayerCount(time));
}
diff --git a/services/surfaceflinger/tests/unittests/RefreshRateSelectorTest.cpp b/services/surfaceflinger/tests/unittests/RefreshRateSelectorTest.cpp
index 264b172..0b67137 100644
--- a/services/surfaceflinger/tests/unittests/RefreshRateSelectorTest.cpp
+++ b/services/surfaceflinger/tests/unittests/RefreshRateSelectorTest.cpp
@@ -1410,11 +1410,6 @@
TEST_P(RefreshRateSelectorTest, getBestFrameRateMode_withFrameRateCategory_30_60_90_120) {
auto selector = createSelector(makeModes(kMode30, kMode60, kMode90, kMode120), kModeId60);
- std::vector<LayerRequirement> layers = {{.vote = LayerVoteType::ExplicitDefault, .weight = 1.f},
- {.vote = LayerVoteType::ExplicitCategory,
- .weight = 1.f}};
- auto& lr = layers[0];
-
struct Case {
// Params
Fps desiredFrameRate = 0_Hz;
@@ -1431,7 +1426,7 @@
{0_Hz, FrameRateCategory::High, 90_Hz},
{0_Hz, FrameRateCategory::Normal, 60_Hz},
{0_Hz, FrameRateCategory::Low, 30_Hz},
- {0_Hz, FrameRateCategory::NoPreference, 60_Hz},
+ {0_Hz, FrameRateCategory::NoPreference, 30_Hz},
// Cases that have both desired frame rate and frame rate category requirements.
{24_Hz, FrameRateCategory::High, 120_Hz},
@@ -1444,34 +1439,41 @@
};
for (auto testCase : testCases) {
+ std::vector<LayerRequirement> layers;
ALOGI("**** %s: Testing desiredFrameRate=%s, frameRateCategory=%s", __func__,
to_string(testCase.desiredFrameRate).c_str(),
ftl::enum_string(testCase.frameRateCategory).c_str());
- lr.desiredRefreshRate = testCase.desiredFrameRate;
-
- std::stringstream ss;
- ss << to_string(testCase.desiredFrameRate)
- << ", category=" << ftl::enum_string(testCase.frameRateCategory);
- lr.name = ss.str();
+ if (testCase.desiredFrameRate.isValid()) {
+ std::stringstream ss;
+ ss << to_string(testCase.desiredFrameRate) << "ExplicitDefault";
+ LayerRequirement layer = {.name = ss.str(),
+ .vote = LayerVoteType::ExplicitDefault,
+ .desiredRefreshRate = testCase.desiredFrameRate,
+ .weight = 1.f};
+ layers.push_back(layer);
+ }
if (testCase.frameRateCategory != FrameRateCategory::Default) {
- layers[1].frameRateCategory = testCase.frameRateCategory;
+ std::stringstream ss;
+ ss << "ExplicitCategory (" << ftl::enum_string(testCase.frameRateCategory) << ")";
+ LayerRequirement layer = {.name = ss.str(),
+ .vote = LayerVoteType::ExplicitCategory,
+ .frameRateCategory = testCase.frameRateCategory,
+ .weight = 1.f};
+ layers.push_back(layer);
}
EXPECT_EQ(testCase.expectedFrameRate, selector.getBestFrameRateMode(layers)->getPeakFps())
- << "did not get expected frame rate for " << lr.name;
+ << "Did not get expected frame rate for frameRate="
+ << to_string(testCase.desiredFrameRate)
+ << " category=" << ftl::enum_string(testCase.frameRateCategory);
}
}
TEST_P(RefreshRateSelectorTest, getBestFrameRateMode_withFrameRateCategory_60_120) {
auto selector = createSelector(makeModes(kMode60, kMode120), kModeId60);
- std::vector<LayerRequirement> layers = {{.vote = LayerVoteType::ExplicitDefault, .weight = 1.f},
- {.vote = LayerVoteType::ExplicitCategory,
- .weight = 1.f}};
- auto& lr = layers[0];
-
struct Case {
// Params
Fps desiredFrameRate = 0_Hz;
@@ -1501,23 +1503,35 @@
};
for (auto testCase : testCases) {
+ std::vector<LayerRequirement> layers;
ALOGI("**** %s: Testing desiredFrameRate=%s, frameRateCategory=%s", __func__,
to_string(testCase.desiredFrameRate).c_str(),
ftl::enum_string(testCase.frameRateCategory).c_str());
- lr.desiredRefreshRate = testCase.desiredFrameRate;
-
- std::stringstream ss;
- ss << to_string(testCase.desiredFrameRate)
- << ", category=" << ftl::enum_string(testCase.frameRateCategory);
- lr.name = ss.str();
+ if (testCase.desiredFrameRate.isValid()) {
+ std::stringstream ss;
+ ss << to_string(testCase.desiredFrameRate) << "ExplicitDefault";
+ LayerRequirement layer = {.name = ss.str(),
+ .vote = LayerVoteType::ExplicitDefault,
+ .desiredRefreshRate = testCase.desiredFrameRate,
+ .weight = 1.f};
+ layers.push_back(layer);
+ }
if (testCase.frameRateCategory != FrameRateCategory::Default) {
- layers[1].frameRateCategory = testCase.frameRateCategory;
+ std::stringstream ss;
+ ss << "ExplicitCategory (" << ftl::enum_string(testCase.frameRateCategory) << ")";
+ LayerRequirement layer = {.name = ss.str(),
+ .vote = LayerVoteType::ExplicitCategory,
+ .frameRateCategory = testCase.frameRateCategory,
+ .weight = 1.f};
+ layers.push_back(layer);
}
EXPECT_EQ(testCase.expectedFrameRate, selector.getBestFrameRateMode(layers)->getPeakFps())
- << "did not get expected frame rate for " << lr.name;
+ << "Did not get expected frame rate for frameRate="
+ << to_string(testCase.desiredFrameRate)
+ << " category=" << ftl::enum_string(testCase.frameRateCategory);
}
}