Revert "SurfaceFlinger: tune infrequent detection logic more"
This reverts commit 39db2c9b075964d28818f187ba417dc13698b680.
Reason for revert: Causing regression with b/157096772
Change-Id: I058d6f8efa637cb36371276761281f51b7aa3dd3
Test: Play 24fps video in YouTube PIP mode and rotate the device - no jank
Test: Chrome playing video - no refresh rate switching
Test: Hide/Show keyboard when inputting text
Test: Running Hay Day and observing refresh rate
Bug: 157096772
diff --git a/services/surfaceflinger/Scheduler/LayerInfoV2.cpp b/services/surfaceflinger/Scheduler/LayerInfoV2.cpp
index 78f4433..c16387a 100644
--- a/services/surfaceflinger/Scheduler/LayerInfoV2.cpp
+++ b/services/surfaceflinger/Scheduler/LayerInfoV2.cpp
@@ -50,28 +50,23 @@
}
}
-bool LayerInfoV2::isFrequent(nsecs_t now) {
- mLastReportedIsFrequent = [&] {
- for (auto it = mFrameTimes.crbegin(); it != mFrameTimes.crend(); ++it) {
- if (now - it->queueTime >= MAX_FREQUENT_LAYER_PERIOD_NS.count()) {
- ALOGV("%s infrequent (last frame is %.2fms ago)", mName.c_str(),
- (now - mFrameTimes.back().queueTime) / 1e6f);
- return false;
- }
-
- const auto numFrames = std::distance(mFrameTimes.crbegin(), it + 1);
- if (numFrames >= FREQUENT_LAYER_WINDOW_SIZE) {
- ALOGV("%s frequent (burst of %zu frames)", mName.c_str(), numFrames);
- return true;
- }
+bool LayerInfoV2::isFrequent(nsecs_t now) const {
+ for (auto it = mFrameTimes.crbegin(); it != mFrameTimes.crend(); ++it) {
+ if (now - it->queueTime >= MAX_FREQUENT_LAYER_PERIOD_NS.count()) {
+ ALOGV("%s infrequent (last frame is %.2fms ago", mName.c_str(),
+ (now - mFrameTimes.back().queueTime) / 1e6f);
+ return false;
}
- ALOGV("%s %sfrequent (not enough frames %zu)", mName.c_str(),
- mLastReportedIsFrequent ? "" : "in", mFrameTimes.size());
- return mLastReportedIsFrequent;
- }();
+ const auto numFrames = std::distance(mFrameTimes.crbegin(), it + 1);
+ if (numFrames >= FREQUENT_LAYER_WINDOW_SIZE) {
+ ALOGV("%s frequent (burst of %zu frames", mName.c_str(), numFrames);
+ return true;
+ }
+ }
- return mLastReportedIsFrequent;
+ ALOGV("%s infrequent (not enough frames %zu)", mName.c_str(), mFrameTimes.size());
+ return false;
}
bool LayerInfoV2::hasEnoughDataForHeuristic() const {
diff --git a/services/surfaceflinger/Scheduler/LayerInfoV2.h b/services/surfaceflinger/Scheduler/LayerInfoV2.h
index 82da7e3..48bddf5 100644
--- a/services/surfaceflinger/Scheduler/LayerInfoV2.h
+++ b/services/surfaceflinger/Scheduler/LayerInfoV2.h
@@ -94,7 +94,7 @@
bool pendingConfigChange;
};
- bool isFrequent(nsecs_t now);
+ bool isFrequent(nsecs_t now) const;
bool hasEnoughDataForHeuristic() const;
std::optional<float> calculateRefreshRateIfPossible();
std::pair<nsecs_t, bool> calculateAverageFrameTime() const;
@@ -110,13 +110,6 @@
float mLastReportedRefreshRate = 0.0f;
- // Used to determine whether a layer should be considered frequent or
- // not when we don't have enough frames. This member will not be cleared
- // as part of clearHistory() to remember whether this layer was frequent
- // or not before we processed touch boost (or anything else that would
- // clear layer history).
- bool mLastReportedIsFrequent = true;
-
// Holds information about the layer vote
struct {
LayerHistory::LayerVoteType type;
diff --git a/services/surfaceflinger/tests/unittests/LayerHistoryTestV2.cpp b/services/surfaceflinger/tests/unittests/LayerHistoryTestV2.cpp
index 09ef06a..d55648a 100644
--- a/services/surfaceflinger/tests/unittests/LayerHistoryTestV2.cpp
+++ b/services/surfaceflinger/tests/unittests/LayerHistoryTestV2.cpp
@@ -103,8 +103,17 @@
EXPECT_TRUE(history().summarize(time).empty());
EXPECT_EQ(0, activeLayerCount());
- // The first few updates are considered frequent
- for (int i = 0; i < PRESENT_TIME_HISTORY_SIZE - 1; i++) {
+ // The first few updates are considered infrequent
+ for (int i = 0; i < FREQUENT_LAYER_WINDOW_SIZE - 1; i++) {
+ history().record(layer.get(), 0, time);
+ ASSERT_EQ(1, history().summarize(time).size());
+ EXPECT_EQ(LayerHistory::LayerVoteType::Min, history().summarize(time)[0].vote);
+ EXPECT_EQ(1, activeLayerCount());
+ EXPECT_EQ(0, frequentLayerCount(time));
+ }
+
+ // Max returned if active layers have insufficient history.
+ for (int i = 0; i < PRESENT_TIME_HISTORY_SIZE - FREQUENT_LAYER_WINDOW_SIZE - 1; i++) {
history().record(layer.get(), 0, time);
ASSERT_EQ(1, history().summarize(time).size());
EXPECT_EQ(LayerHistory::LayerVoteType::Max, history().summarize(time)[0].vote);
@@ -135,8 +144,8 @@
history().record(layer.get(), 0, time);
auto summary = history().summarize(time);
ASSERT_EQ(1, history().summarize(time).size());
- // Layer is still considered active so we expect to get Max
- EXPECT_EQ(LayerHistory::LayerVoteType::Max, history().summarize(time)[0].vote);
+ // Layer is still considered inactive so we expect to get Min
+ EXPECT_EQ(LayerHistory::LayerVoteType::Min, history().summarize(time)[0].vote);
EXPECT_EQ(1, activeLayerCount());
EXPECT_CALL(*layer, isVisible()).WillRepeatedly(Return(false));
@@ -466,13 +475,13 @@
nsecs_t time = systemTime();
- // The first few updates are considered frequent
- for (int i = 0; i < PRESENT_TIME_HISTORY_SIZE - 1; i++) {
+ // The first few updates are considered infrequent
+ for (int i = 0; i < FREQUENT_LAYER_WINDOW_SIZE - 1; i++) {
history().record(layer.get(), 0, time);
ASSERT_EQ(1, history().summarize(time).size());
- EXPECT_EQ(LayerHistory::LayerVoteType::Max, history().summarize(time)[0].vote);
+ EXPECT_EQ(LayerHistory::LayerVoteType::Min, history().summarize(time)[0].vote);
EXPECT_EQ(1, activeLayerCount());
- EXPECT_EQ(1, frequentLayerCount(time));
+ EXPECT_EQ(0, frequentLayerCount(time));
}
// advance the time for the previous frame to be inactive
@@ -499,36 +508,6 @@
EXPECT_EQ(LayerHistory::LayerVoteType::Max, history().summarize(time)[0].vote);
EXPECT_EQ(1, activeLayerCount());
EXPECT_EQ(1, frequentLayerCount(time));
-
- // advance the time for the previous frame to be inactive
- time += MAX_ACTIVE_LAYER_PERIOD_NS.count();
-
- // Now event if we post a quick few frame we should stay infrequent
- for (int i = 0; i < FREQUENT_LAYER_WINDOW_SIZE - 1; i++) {
- history().record(layer.get(), time, time);
- time += HI_FPS_PERIOD;
-
- EXPECT_EQ(1, layerCount());
- ASSERT_EQ(1, history().summarize(time).size());
- EXPECT_EQ(LayerHistory::LayerVoteType::Min, history().summarize(time)[0].vote);
- EXPECT_EQ(1, activeLayerCount());
- EXPECT_EQ(0, frequentLayerCount(time));
- }
-
- // clear the history
- history().clear();
-
- // Now event if we post a quick few frame we should stay infrequent
- for (int i = 0; i < FREQUENT_LAYER_WINDOW_SIZE - 1; i++) {
- history().record(layer.get(), time, time);
- time += HI_FPS_PERIOD;
-
- EXPECT_EQ(1, layerCount());
- ASSERT_EQ(1, history().summarize(time).size());
- EXPECT_EQ(LayerHistory::LayerVoteType::Min, history().summarize(time)[0].vote);
- EXPECT_EQ(1, activeLayerCount());
- EXPECT_EQ(0, frequentLayerCount(time));
- }
}
TEST_F(LayerHistoryTestV2, invisibleExplicitLayer) {