Revert "SurfaceFlinger: more aggressive infrequent layer detection"
This reverts commit 1adbb727595c6da51bb088e4ce3f8070d9b32808.
Reason for revert: Causing regression with b/157096772
Change-Id: Ib2009f7a4ecbd268ea69304853a91bd22674ec1e
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
Bug: 155062712
Bug: 156654519
diff --git a/services/surfaceflinger/Scheduler/LayerHistoryV2.cpp b/services/surfaceflinger/Scheduler/LayerHistoryV2.cpp
index afc8c4f..359fe1b 100644
--- a/services/surfaceflinger/Scheduler/LayerHistoryV2.cpp
+++ b/services/surfaceflinger/Scheduler/LayerHistoryV2.cpp
@@ -90,7 +90,7 @@
void LayerHistoryV2::registerLayer(Layer* layer, float /*lowRefreshRate*/, float highRefreshRate,
LayerVoteType type) {
const nsecs_t highRefreshRatePeriod = static_cast<nsecs_t>(1e9f / highRefreshRate);
- auto info = std::make_unique<LayerInfoV2>(layer->getName(), highRefreshRatePeriod, type);
+ auto info = std::make_unique<LayerInfoV2>(highRefreshRatePeriod, type);
std::lock_guard lock(mLock);
mLayerInfos.emplace_back(layer, std::move(info));
}
diff --git a/services/surfaceflinger/Scheduler/LayerInfoV2.cpp b/services/surfaceflinger/Scheduler/LayerInfoV2.cpp
index c16387a..8a6d52a 100644
--- a/services/surfaceflinger/Scheduler/LayerInfoV2.cpp
+++ b/services/surfaceflinger/Scheduler/LayerInfoV2.cpp
@@ -27,10 +27,8 @@
namespace android::scheduler {
-LayerInfoV2::LayerInfoV2(const std::string& name, nsecs_t highRefreshRatePeriod,
- LayerHistory::LayerVoteType defaultVote)
- : mName(name),
- mHighRefreshRatePeriod(highRefreshRatePeriod),
+LayerInfoV2::LayerInfoV2(nsecs_t highRefreshRatePeriod, LayerHistory::LayerVoteType defaultVote)
+ : mHighRefreshRatePeriod(highRefreshRatePeriod),
mDefaultVote(defaultVote),
mLayerVote({defaultVote, 0.0f}) {}
@@ -50,23 +48,42 @@
}
}
-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;
- }
+bool LayerInfoV2::isFrameTimeValid(const FrameTimeData& frameTime) const {
+ return frameTime.queueTime >= std::chrono::duration_cast<std::chrono::nanoseconds>(
+ mFrameTimeValidSince.time_since_epoch())
+ .count();
+}
- 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 {
+ // Find the first valid frame time
+ auto it = mFrameTimes.begin();
+ for (; it != mFrameTimes.end(); ++it) {
+ if (isFrameTimeValid(*it)) {
+ break;
}
}
- ALOGV("%s infrequent (not enough frames %zu)", mName.c_str(), mFrameTimes.size());
- return false;
+ // If we know nothing about this layer we consider it as frequent as it might be the start
+ // of an animation.
+ if (std::distance(it, mFrameTimes.end()) < FREQUENT_LAYER_WINDOW_SIZE) {
+ return true;
+ }
+
+ // Find the first active frame
+ for (; it != mFrameTimes.end(); ++it) {
+ if (it->queueTime >= getActiveLayerThreshold(now)) {
+ break;
+ }
+ }
+
+ const auto numFrames = std::distance(it, mFrameTimes.end());
+ if (numFrames < FREQUENT_LAYER_WINDOW_SIZE) {
+ return false;
+ }
+
+ // Layer is considered frequent if the average frame rate is higher than the threshold
+ const auto totalTime = mFrameTimes.back().queueTime - it->queueTime;
+ return (1e9f * (numFrames - 1)) / totalTime >= MIN_FPS_FOR_FREQUENT_LAYER;
}
bool LayerInfoV2::hasEnoughDataForHeuristic() const {
@@ -75,6 +92,10 @@
return false;
}
+ if (!isFrameTimeValid(mFrameTimes.front())) {
+ return false;
+ }
+
if (mFrameTimes.size() < HISTORY_SIZE &&
mFrameTimes.back().queueTime - mFrameTimes.front().queueTime < HISTORY_TIME.count()) {
return false;
@@ -170,22 +191,18 @@
std::pair<LayerHistory::LayerVoteType, float> LayerInfoV2::getRefreshRate(nsecs_t now) {
if (mLayerVote.type != LayerHistory::LayerVoteType::Heuristic) {
- ALOGV("%s voted %d ", mName.c_str(), static_cast<int>(mLayerVote.type));
return {mLayerVote.type, mLayerVote.fps};
}
if (!isFrequent(now)) {
- ALOGV("%s is infrequent", mName.c_str());
return {LayerHistory::LayerVoteType::Min, 0};
}
auto refreshRate = calculateRefreshRateIfPossible();
if (refreshRate.has_value()) {
- ALOGV("%s calculated refresh rate: %.2f", mName.c_str(), refreshRate.value());
return {LayerHistory::LayerVoteType::Heuristic, refreshRate.value()};
}
- ALOGV("%s Max (can't resolve refresh rate", mName.c_str());
return {LayerHistory::LayerVoteType::Max, 0};
}
diff --git a/services/surfaceflinger/Scheduler/LayerInfoV2.h b/services/surfaceflinger/Scheduler/LayerInfoV2.h
index ccd6be4..47945d1 100644
--- a/services/surfaceflinger/Scheduler/LayerInfoV2.h
+++ b/services/surfaceflinger/Scheduler/LayerInfoV2.h
@@ -54,8 +54,7 @@
friend class LayerHistoryTestV2;
public:
- LayerInfoV2(const std::string& name, nsecs_t highRefreshRatePeriod,
- LayerHistory::LayerVoteType defaultVote);
+ LayerInfoV2(nsecs_t highRefreshRatePeriod, LayerHistory::LayerVoteType defaultVote);
LayerInfoV2(const LayerInfo&) = delete;
LayerInfoV2& operator=(const LayerInfoV2&) = delete;
@@ -84,7 +83,11 @@
nsecs_t getLastUpdatedTime() const { return mLastUpdatedTime; }
void clearHistory() {
- mFrameTimes.clear();
+ // Mark mFrameTimeValidSince to now to ignore all previous frame times.
+ // We are not deleting the old frame to keep track of whether we should treat the first
+ // buffer as Max as we don't know anything about this layer or Min as this layer is
+ // posting infrequent updates.
+ mFrameTimeValidSince = std::chrono::steady_clock::now();
mLastReportedRefreshRate = 0.0f;
}
@@ -101,8 +104,7 @@
std::optional<float> calculateRefreshRateIfPossible();
std::pair<nsecs_t, bool> calculateAverageFrameTime() const;
bool isRefreshRateStable(nsecs_t averageFrameTime, bool missingPresentTime) const;
-
- const std::string mName;
+ bool isFrameTimeValid(const FrameTimeData&) const;
// Used for sanitizing the heuristic data
const nsecs_t mHighRefreshRatePeriod;
@@ -119,6 +121,8 @@
} mLayerVote;
std::deque<FrameTimeData> mFrameTimes;
+ std::chrono::time_point<std::chrono::steady_clock> mFrameTimeValidSince =
+ std::chrono::steady_clock::now();
static constexpr size_t HISTORY_SIZE = 90;
static constexpr std::chrono::nanoseconds HISTORY_TIME = 1s;
};
diff --git a/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp b/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp
index e181c12..651283f 100644
--- a/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp
+++ b/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp
@@ -103,7 +103,7 @@
ATRACE_CALL();
ALOGV("getRefreshRateForContent %zu layers", layers.size());
- if (touchConsidered) *touchConsidered = false;
+ *touchConsidered = false;
std::lock_guard lock(mLock);
int noVoteLayers = 0;
@@ -134,8 +134,7 @@
// Consider the touch event if there are no Explicit* layers. Otherwise wait until after we've
// selected a refresh rate to see if we should apply touch boost.
if (touchActive && !hasExplicitVoteLayers) {
- ALOGV("TouchBoost - choose %s", getMaxRefreshRateByPolicyLocked().getName().c_str());
- if (touchConsidered) *touchConsidered = true;
+ *touchConsidered = true;
return getMaxRefreshRateByPolicyLocked();
}
@@ -155,7 +154,6 @@
// Only if all layers want Min we should return Min
if (noVoteLayers + minVoteLayers == layers.size()) {
- ALOGV("all layers Min - choose %s", getMinRefreshRateByPolicyLocked().getName().c_str());
return getMinRefreshRateByPolicyLocked();
}
@@ -253,11 +251,9 @@
return 1.0f / iter;
}();
- ALOGV("%s (%s, weight %.2f) %.2fHz gives %s score of %.2f", layer.name.c_str(),
- layer.vote == LayerVoteType::ExplicitExactOrMultiple
- ? "ExplicitExactOrMultiple"
- : "Heuristic",
- weight, 1e9f / layerPeriod, scores[i].first->name.c_str(), layerScore);
+ ALOGV("%s (ExplicitExactOrMultiple, weight %.2f) %.2fHz gives %s score of %.2f",
+ layer.name.c_str(), weight, 1e9f / layerPeriod, scores[i].first->name.c_str(),
+ layerScore);
scores[i].second += weight * layerScore;
continue;
}
@@ -290,8 +286,7 @@
if (touchActive && explicitDefaultVoteLayers == 0 &&
bestRefreshRate->fps < touchRefreshRate.fps) {
- if (touchConsidered) *touchConsidered = true;
- ALOGV("TouchBoost - choose %s", touchRefreshRate.getName().c_str());
+ *touchConsidered = true;
return touchRefreshRate;
}
diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp
index 7c2af23..dcce08d 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.cpp
+++ b/services/surfaceflinger/Scheduler/Scheduler.cpp
@@ -527,9 +527,7 @@
void Scheduler::touchTimerCallback(TimerState state) {
const TouchState touch = state == TimerState::Reset ? TouchState::Active : TouchState::Inactive;
- if (handleTimerStateChanged(&mFeatures.touch, touch, true /* eventOnContentDetection */)) {
- mLayerHistory->clear();
- }
+ handleTimerStateChanged(&mFeatures.touch, touch, true /* eventOnContentDetection */);
ATRACE_INT("TouchState", static_cast<int>(touch));
}
@@ -552,19 +550,18 @@
}
template <class T>
-bool Scheduler::handleTimerStateChanged(T* currentState, T newState, bool eventOnContentDetection) {
+void Scheduler::handleTimerStateChanged(T* currentState, T newState, bool eventOnContentDetection) {
ConfigEvent event = ConfigEvent::None;
HwcConfigIndexType newConfigId;
- bool touchConsidered = false;
{
std::lock_guard<std::mutex> lock(mFeatureStateLock);
if (*currentState == newState) {
- return touchConsidered;
+ return;
}
*currentState = newState;
- newConfigId = calculateRefreshRateConfigIndexType(&touchConsidered);
+ newConfigId = calculateRefreshRateConfigIndexType();
if (mFeatures.configId == newConfigId) {
- return touchConsidered;
+ return;
}
mFeatures.configId = newConfigId;
if (eventOnContentDetection && !mFeatures.contentRequirements.empty()) {
@@ -573,12 +570,10 @@
}
const RefreshRate& newRefreshRate = mRefreshRateConfigs.getRefreshRateFromConfigId(newConfigId);
mSchedulerCallback.changeRefreshRate(newRefreshRate, event);
- return touchConsidered;
}
-HwcConfigIndexType Scheduler::calculateRefreshRateConfigIndexType(bool* touchConsidered) {
+HwcConfigIndexType Scheduler::calculateRefreshRateConfigIndexType() {
ATRACE_CALL();
- if (touchConsidered) *touchConsidered = false;
// If Display Power is not in normal operation we want to be in performance mode. When coming
// back to normal mode, a grace period is given with DisplayPowerTimer.
@@ -613,9 +608,18 @@
.getConfigId();
}
- return mRefreshRateConfigs
- .getBestRefreshRate(mFeatures.contentRequirements, touchActive, idle, touchConsidered)
- .getConfigId();
+ bool touchConsidered;
+ const auto& ret = mRefreshRateConfigs
+ .getBestRefreshRate(mFeatures.contentRequirements, touchActive, idle,
+ &touchConsidered)
+ .getConfigId();
+ if (touchConsidered) {
+ // Clear layer history if refresh rate was selected based on touch to allow
+ // the hueristic to pick up with the new rate.
+ mLayerHistory->clear();
+ }
+
+ return ret;
}
std::optional<HwcConfigIndexType> Scheduler::getPreferredConfigId() {
diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h
index 066e9ca..9cea4cc 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.h
+++ b/services/surfaceflinger/Scheduler/Scheduler.h
@@ -178,15 +178,14 @@
// handles various timer features to change the refresh rate.
template <class T>
- bool handleTimerStateChanged(T* currentState, T newState, bool eventOnContentDetection);
+ void handleTimerStateChanged(T* currentState, T newState, bool eventOnContentDetection);
void setVsyncPeriod(nsecs_t period);
// This function checks whether individual features that are affecting the refresh rate
// selection were initialized, prioritizes them, and calculates the HwcConfigIndexType
// for the suggested refresh rate.
- HwcConfigIndexType calculateRefreshRateConfigIndexType(bool* touchConsidered = nullptr)
- REQUIRES(mFeatureStateLock);
+ HwcConfigIndexType calculateRefreshRateConfigIndexType() REQUIRES(mFeatureStateLock);
// Stores EventThread associated with a given VSyncSource, and an initial EventThreadConnection.
struct Connection {
diff --git a/services/surfaceflinger/tests/unittests/LayerHistoryTestV2.cpp b/services/surfaceflinger/tests/unittests/LayerHistoryTestV2.cpp
index d55648a..431cf0f 100644
--- a/services/surfaceflinger/tests/unittests/LayerHistoryTestV2.cpp
+++ b/services/surfaceflinger/tests/unittests/LayerHistoryTestV2.cpp
@@ -103,22 +103,12 @@
EXPECT_TRUE(history().summarize(time).empty());
EXPECT_EQ(0, activeLayerCount());
- // 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++) {
+ for (int i = 0; i < PRESENT_TIME_HISTORY_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(1, activeLayerCount());
- EXPECT_EQ(1, frequentLayerCount(time));
}
// Max is returned since we have enough history but there is no timestamp votes.
@@ -127,7 +117,6 @@
ASSERT_EQ(1, history().summarize(time).size());
EXPECT_EQ(LayerHistory::LayerVoteType::Max, history().summarize(time)[0].vote);
EXPECT_EQ(1, activeLayerCount());
- EXPECT_EQ(1, frequentLayerCount(time));
}
}
@@ -145,7 +134,7 @@
auto summary = history().summarize(time);
ASSERT_EQ(1, history().summarize(time).size());
// Layer is still considered inactive so we expect to get Min
- EXPECT_EQ(LayerHistory::LayerVoteType::Min, history().summarize(time)[0].vote);
+ EXPECT_EQ(LayerHistory::LayerVoteType::Max, history().summarize(time)[0].vote);
EXPECT_EQ(1, activeLayerCount());
EXPECT_CALL(*layer, isVisible()).WillRepeatedly(Return(false));
@@ -475,15 +464,28 @@
nsecs_t time = systemTime();
- // The first few updates are considered infrequent
+ // the very first updates makes the layer frequent
for (int i = 0; i < FREQUENT_LAYER_WINDOW_SIZE - 1; i++) {
- history().record(layer.get(), 0, time);
+ history().record(layer.get(), time, time);
+ time += MAX_FREQUENT_LAYER_PERIOD_NS.count();
+
+ EXPECT_EQ(1, layerCount());
ASSERT_EQ(1, history().summarize(time).size());
- EXPECT_EQ(LayerHistory::LayerVoteType::Min, history().summarize(time)[0].vote);
+ EXPECT_EQ(LayerHistory::LayerVoteType::Max, history().summarize(time)[0].vote);
EXPECT_EQ(1, activeLayerCount());
- EXPECT_EQ(0, frequentLayerCount(time));
+ EXPECT_EQ(1, frequentLayerCount(time));
}
+ // the next update with the MAX_FREQUENT_LAYER_PERIOD_NS will get us to infrequent
+ history().record(layer.get(), time, time);
+ time += MAX_FREQUENT_LAYER_PERIOD_NS.count();
+
+ 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));
+
// advance the time for the previous frame to be inactive
time += MAX_ACTIVE_LAYER_PERIOD_NS.count();
@@ -526,11 +528,9 @@
nsecs_t time = systemTime();
- // Post a few buffers to the layers to make them active
- for (int i = 0; i < FREQUENT_LAYER_WINDOW_SIZE; i++) {
- history().record(explicitVisiblelayer.get(), time, time);
- history().record(explicitInvisiblelayer.get(), time, time);
- }
+ // Post a buffer to the layers to make them active
+ history().record(explicitVisiblelayer.get(), time, time);
+ history().record(explicitInvisiblelayer.get(), time, time);
EXPECT_EQ(2, layerCount());
ASSERT_EQ(1, history().summarize(time).size());