SF: replace dont_skip_on_early flag
.. with a new flag due to a Gantry bug: b/323967908
Bug: 273702768
Change-Id: I126bc6c4927c6ae8d1cd54316babe1590d3dde28
Test: presubmit
diff --git a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp
index 963f9e9..f5aaf06 100644
--- a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp
+++ b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp
@@ -107,7 +107,7 @@
mArmedInfo && (nextVsyncTime > (mArmedInfo->mActualVsyncTime + mMinVsyncDistance));
bool const wouldSkipAWakeup =
mArmedInfo && ((nextWakeupTime > (mArmedInfo->mActualWakeupTime + mMinVsyncDistance)));
- if (FlagManager::getInstance().dont_skip_on_early()) {
+ if (FlagManager::getInstance().dont_skip_on_early_ro()) {
if (wouldSkipAVsyncTarget || wouldSkipAWakeup) {
nextVsyncTime = mArmedInfo->mActualVsyncTime;
} else {
diff --git a/services/surfaceflinger/common/FlagManager.cpp b/services/surfaceflinger/common/FlagManager.cpp
index 5579648..f7adc0e 100644
--- a/services/surfaceflinger/common/FlagManager.cpp
+++ b/services/surfaceflinger/common/FlagManager.cpp
@@ -108,7 +108,6 @@
DUMP_SERVER_FLAG(use_skia_tracing);
/// Trunk stable server flags ///
- DUMP_SERVER_FLAG(dont_skip_on_early);
DUMP_SERVER_FLAG(refresh_rate_overlay_on_external_display);
/// Trunk stable readonly flags ///
@@ -131,6 +130,7 @@
DUMP_READ_ONLY_FLAG(vulkan_renderengine);
DUMP_READ_ONLY_FLAG(renderable_buffer_usage);
DUMP_READ_ONLY_FLAG(restore_blur_step);
+ DUMP_READ_ONLY_FLAG(dont_skip_on_early_ro);
#undef DUMP_READ_ONLY_FLAG
#undef DUMP_SERVER_FLAG
#undef DUMP_FLAG_INTERVAL
@@ -209,15 +209,9 @@
FLAG_MANAGER_READ_ONLY_FLAG(vulkan_renderengine, "debug.renderengine.vulkan")
FLAG_MANAGER_READ_ONLY_FLAG(renderable_buffer_usage, "")
FLAG_MANAGER_READ_ONLY_FLAG(restore_blur_step, "debug.renderengine.restore_blur_step")
+FLAG_MANAGER_READ_ONLY_FLAG(dont_skip_on_early_ro, "")
/// Trunk stable server flags ///
FLAG_MANAGER_SERVER_FLAG(refresh_rate_overlay_on_external_display, "")
-/// Exceptions ///
-bool FlagManager::dont_skip_on_early() const {
- // Even though this is a server writable flag, we do call it before boot completed, but that's
- // fine since the decision is done per frame. We can't do caching though.
- return flags::dont_skip_on_early();
-}
-
} // namespace android
diff --git a/services/surfaceflinger/common/include/common/FlagManager.h b/services/surfaceflinger/common/include/common/FlagManager.h
index d806ee9..18f623f 100644
--- a/services/surfaceflinger/common/include/common/FlagManager.h
+++ b/services/surfaceflinger/common/include/common/FlagManager.h
@@ -48,7 +48,6 @@
bool use_skia_tracing() const;
/// Trunk stable server flags ///
- bool dont_skip_on_early() const;
bool refresh_rate_overlay_on_external_display() const;
/// Trunk stable readonly flags ///
@@ -71,6 +70,7 @@
bool vulkan_renderengine() const;
bool renderable_buffer_usage() const;
bool restore_blur_step() const;
+ bool dont_skip_on_early_ro() const;
protected:
// overridden for unit tests
diff --git a/services/surfaceflinger/surfaceflinger_flags.aconfig b/services/surfaceflinger/surfaceflinger_flags.aconfig
index f644893..f5ec1ee 100644
--- a/services/surfaceflinger/surfaceflinger_flags.aconfig
+++ b/services/surfaceflinger/surfaceflinger_flags.aconfig
@@ -34,13 +34,6 @@
}
flag {
- name: "dont_skip_on_early"
- namespace: "core_graphics"
- description: "This flag is guarding the behaviour where SurfaceFlinger is trying to opportunistically present a frame when the configuration change from late to early"
- bug: "273702768"
-}
-
-flag {
name: "multithreaded_present"
namespace: "core_graphics"
description: "Controls whether to offload present calls to another thread"
@@ -190,3 +183,14 @@
purpose: PURPOSE_BUGFIX
}
}
+
+flag {
+ name: "dont_skip_on_early_ro"
+ namespace: "core_graphics"
+ description: "This flag is guarding the behaviour where SurfaceFlinger is trying to opportunistically present a frame when the configuration change from late to early"
+ bug: "273702768"
+ is_fixed_read_only: true
+ metadata {
+ purpose: PURPOSE_BUGFIX
+ }
+}
diff --git a/services/surfaceflinger/tests/unittests/FlagManagerTest.cpp b/services/surfaceflinger/tests/unittests/FlagManagerTest.cpp
index eddebe9..299e8aa 100644
--- a/services/surfaceflinger/tests/unittests/FlagManagerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/FlagManagerTest.cpp
@@ -169,18 +169,4 @@
}
}
-TEST_F(FlagManagerTest, dontSkipOnEarlyIsNotCached) {
- EXPECT_CALL(mFlagManager, getBoolProperty).WillRepeatedly(Return(std::nullopt));
-
- const auto initialValue = flags::dont_skip_on_early();
-
- flags::dont_skip_on_early(true);
- EXPECT_EQ(true, mFlagManager.dont_skip_on_early());
-
- flags::dont_skip_on_early(false);
- EXPECT_EQ(false, mFlagManager.dont_skip_on_early());
-
- flags::dont_skip_on_early(initialValue);
-}
-
} // namespace android
diff --git a/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp b/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp
index 4bf58de..c9a4094 100644
--- a/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp
@@ -734,7 +734,7 @@
// b/1450138150
TEST_F(VSyncDispatchTimerQueueTest, doesNotMoveCallbackBackwardsAndSkipAScheduledTargetVSync) {
- SET_FLAG_FOR_TEST(flags::dont_skip_on_early, false);
+ SET_FLAG_FOR_TEST(flags::dont_skip_on_early_ro, false);
EXPECT_CALL(mMockClock, alarmAt(_, 500));
CountingCallback cb(mDispatch);
@@ -754,7 +754,7 @@
// b/1450138150
TEST_F(VSyncDispatchTimerQueueTest, movesCallbackBackwardsAndSkipAScheduledTargetVSync) {
- SET_FLAG_FOR_TEST(flags::dont_skip_on_early, true);
+ SET_FLAG_FOR_TEST(flags::dont_skip_on_early_ro, true);
Sequence seq;
EXPECT_CALL(mMockClock, alarmAt(_, 500)).InSequence(seq);
@@ -821,7 +821,7 @@
}
TEST_F(VSyncDispatchTimerQueueTest, scheduleUpdatesDoesNotAffectSchedulingState) {
- SET_FLAG_FOR_TEST(flags::dont_skip_on_early, false);
+ SET_FLAG_FOR_TEST(flags::dont_skip_on_early_ro, false);
EXPECT_CALL(mMockClock, alarmAt(_, 600));
@@ -839,7 +839,7 @@
}
TEST_F(VSyncDispatchTimerQueueTest, scheduleUpdatesDoesAffectSchedulingState) {
- SET_FLAG_FOR_TEST(flags::dont_skip_on_early, true);
+ SET_FLAG_FOR_TEST(flags::dont_skip_on_early_ro, true);
Sequence seq;
EXPECT_CALL(mMockClock, alarmAt(_, 600)).InSequence(seq);
@@ -1051,7 +1051,7 @@
}
TEST_F(VSyncDispatchTimerQueueTest, updatesVsyncTimeForCloseWakeupTime) {
- SET_FLAG_FOR_TEST(flags::dont_skip_on_early, false);
+ SET_FLAG_FOR_TEST(flags::dont_skip_on_early_ro, false);
Sequence seq;
EXPECT_CALL(mMockClock, alarmAt(_, 600)).InSequence(seq);
@@ -1074,7 +1074,7 @@
}
TEST_F(VSyncDispatchTimerQueueTest, doesNotUpdatesVsyncTimeForCloseWakeupTime) {
- SET_FLAG_FOR_TEST(flags::dont_skip_on_early, true);
+ SET_FLAG_FOR_TEST(flags::dont_skip_on_early_ro, true);
Sequence seq;
EXPECT_CALL(mMockClock, alarmAt(_, 600)).InSequence(seq);
@@ -1098,7 +1098,7 @@
}
TEST_F(VSyncDispatchTimerQueueTest, skipAVsyc) {
- SET_FLAG_FOR_TEST(flags::dont_skip_on_early, false);
+ SET_FLAG_FOR_TEST(flags::dont_skip_on_early_ro, false);
EXPECT_CALL(mMockClock, alarmAt(_, 500));
CountingCallback cb(mDispatch);
@@ -1117,7 +1117,7 @@
}
TEST_F(VSyncDispatchTimerQueueTest, dontskipAVsyc) {
- SET_FLAG_FOR_TEST(flags::dont_skip_on_early, true);
+ SET_FLAG_FOR_TEST(flags::dont_skip_on_early_ro, true);
Sequence seq;
EXPECT_CALL(mMockClock, alarmAt(_, 500)).InSequence(seq);