diff options
9 files changed, 113 insertions, 5 deletions
diff --git a/services/surfaceflinger/Android.bp b/services/surfaceflinger/Android.bp index 71c75f9521..9d0f2858ec 100644 --- a/services/surfaceflinger/Android.bp +++ b/services/surfaceflinger/Android.bp @@ -31,9 +31,6 @@ cc_defaults { "-Wconversion", "-DANDROID_UTILS_REF_BASE_DISABLE_IMPLICIT_CONSTRUCTION", ], - static_libs: [ - "libsurfaceflingerflags", - ], } cc_defaults { diff --git a/services/surfaceflinger/CompositionEngine/Android.bp b/services/surfaceflinger/CompositionEngine/Android.bp index 2dc7332ed7..370e4b66e8 100644 --- a/services/surfaceflinger/CompositionEngine/Android.bp +++ b/services/surfaceflinger/CompositionEngine/Android.bp @@ -63,6 +63,7 @@ cc_defaults { cc_library { name: "libcompositionengine", defaults: ["libcompositionengine_defaults"], + static_libs: ["libsurfaceflingerflags"], srcs: [ "src/planner/CachedSet.cpp", "src/planner/Flattener.cpp", @@ -107,6 +108,7 @@ cc_library { "libgtest", "libgmock", "libcompositionengine", + "libsurfaceflingerflags_test", ], local_include_dirs: ["include"], export_include_dirs: ["include"], @@ -141,6 +143,7 @@ cc_test { "librenderengine_mocks", "libgmock", "libgtest", + "libsurfaceflingerflags_test", ], // For some reason, libvulkan isn't picked up from librenderengine // Probably ASAN related? diff --git a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp index 1f922f11fb..c4c9fa56ff 100644 --- a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp +++ b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp @@ -28,10 +28,13 @@ #include "VSyncDispatchTimerQueue.h" #include "VSyncTracker.h" +#include <com_android_graphics_surfaceflinger_flags.h> + #undef LOG_TAG #define LOG_TAG "VSyncDispatch" namespace android::scheduler { +using namespace com::android::graphics::surfaceflinger; using base::StringAppendF; @@ -100,8 +103,14 @@ ScheduleResult VSyncDispatchTimerQueueEntry::schedule(VSyncDispatch::ScheduleTim mArmedInfo && (nextVsyncTime > (mArmedInfo->mActualVsyncTime + mMinVsyncDistance)); bool const wouldSkipAWakeup = mArmedInfo && ((nextWakeupTime > (mArmedInfo->mActualWakeupTime + mMinVsyncDistance))); - if (wouldSkipAVsyncTarget && wouldSkipAWakeup) { - return getExpectedCallbackTime(nextVsyncTime, timing); + if (flags::dont_skip_on_early()) { + if (wouldSkipAVsyncTarget || wouldSkipAWakeup) { + return getExpectedCallbackTime(mArmedInfo->mActualVsyncTime, timing); + } + } else { + if (wouldSkipAVsyncTarget && wouldSkipAWakeup) { + return getExpectedCallbackTime(nextVsyncTime, timing); + } } nextVsyncTime = adjustVsyncIfNeeded(tracker, nextVsyncTime); diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 2b703016ca..7e799bbe30 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -6446,6 +6446,8 @@ void SurfaceFlinger::dumpAllLocked(const DumpArgs& args, const std::string& comp mMisc2FlagEarlyBootValue == mMisc2FlagLateBootValue ? "stable" : "modified"); StringAppendF(&result, "VrrConfigFlagValue: %s\n", flagutils::vrrConfigEnabled() ? "true" : "false"); + StringAppendF(&result, "DontSkipOnEarlyFlagValue: %s\n", + flags::dont_skip_on_early() ? "true" : "false"); getRenderEngine().dump(result); diff --git a/services/surfaceflinger/surfaceflinger_flags.aconfig b/services/surfaceflinger/surfaceflinger_flags.aconfig index 0f26d566bc..becbb87bc9 100644 --- a/services/surfaceflinger/surfaceflinger_flags.aconfig +++ b/services/surfaceflinger/surfaceflinger_flags.aconfig @@ -38,3 +38,10 @@ flag { bug: "283055450" is_fixed_read_only: true } + +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" +} diff --git a/services/surfaceflinger/tests/Android.bp b/services/surfaceflinger/tests/Android.bp index 36b197234a..67f2dcade4 100644 --- a/services/surfaceflinger/tests/Android.bp +++ b/services/surfaceflinger/tests/Android.bp @@ -67,6 +67,7 @@ cc_test { static_libs: [ "liblayers_proto", "android.hardware.graphics.composer@2.1", + "libsurfaceflingerflags", ], shared_libs: [ "android.hardware.graphics.common@1.2", diff --git a/services/surfaceflinger/tests/unittests/Android.bp b/services/surfaceflinger/tests/unittests/Android.bp index c99809b60d..ec21eaf488 100644 --- a/services/surfaceflinger/tests/unittests/Android.bp +++ b/services/surfaceflinger/tests/unittests/Android.bp @@ -42,6 +42,12 @@ filegroup { ], } +cc_aconfig_library { + name: "libsurfaceflingerflags_test", + aconfig_declarations: "surfaceflinger_flags", + test: true, +} + cc_test { name: "libsurfaceflinger_unittest", defaults: [ @@ -168,6 +174,7 @@ cc_defaults { "libtimestats_proto", "libtonemap", "perfetto_trace_protos", + "libsurfaceflingerflags_test", ], shared_libs: [ "android.hardware.configstore-utils", diff --git a/services/surfaceflinger/tests/unittests/FlagUtils.h b/services/surfaceflinger/tests/unittests/FlagUtils.h new file mode 100644 index 0000000000..7103684577 --- /dev/null +++ b/services/surfaceflinger/tests/unittests/FlagUtils.h @@ -0,0 +1,36 @@ +/* + * Copyright 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#define SET_FLAG_FOR_TEST(name, value) TestFlagSetter _testflag_((name), (name), (value)) + +namespace android { +class TestFlagSetter { +public: + TestFlagSetter(bool (*getter)(), void((*setter)(bool)), bool flagValue) { + const bool initialValue = getter(); + setter(flagValue); + mResetFlagValue = [=] { setter(initialValue); }; + } + + ~TestFlagSetter() { mResetFlagValue(); } + +private: + std::function<void()> mResetFlagValue; +}; + +} // namespace android diff --git a/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp b/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp index 7af1da6a31..b8fdce1dce 100644 --- a/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp +++ b/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp @@ -30,13 +30,17 @@ #include <scheduler/TimeKeeper.h> +#include "FlagUtils.h" #include "Scheduler/VSyncDispatchTimerQueue.h" #include "Scheduler/VSyncTracker.h" +#include <com_android_graphics_surfaceflinger_flags.h> + using namespace testing; using namespace std::literals; namespace android::scheduler { +using namespace com::android::graphics::surfaceflinger; class MockVSyncTracker : public VSyncTracker { public: @@ -1061,6 +1065,48 @@ TEST_F(VSyncDispatchTimerQueueTest, updatesVsyncTimeForCloseWakeupTime) { EXPECT_THAT(cb.mReadyTime[0], Eq(2000)); } +TEST_F(VSyncDispatchTimerQueueTest, skipAVsyc) { + SET_FLAG_FOR_TEST(flags::dont_skip_on_early, false); + + EXPECT_CALL(mMockClock, alarmAt(_, 500)); + CountingCallback cb(mDispatch); + auto result = + mDispatch->schedule(cb, + {.workDuration = 500, .readyDuration = 0, .earliestVsync = 1000}); + EXPECT_TRUE(result.has_value()); + EXPECT_EQ(500, *result); + mMockClock.advanceBy(300); + + result = mDispatch->schedule(cb, + {.workDuration = 800, .readyDuration = 0, .earliestVsync = 1000}); + EXPECT_TRUE(result.has_value()); + EXPECT_EQ(1200, *result); + + advanceToNextCallback(); + ASSERT_THAT(cb.mCalls.size(), Eq(1)); +} + +TEST_F(VSyncDispatchTimerQueueTest, dontskipAVsyc) { + SET_FLAG_FOR_TEST(flags::dont_skip_on_early, true); + + EXPECT_CALL(mMockClock, alarmAt(_, 500)); + CountingCallback cb(mDispatch); + auto result = + mDispatch->schedule(cb, + {.workDuration = 500, .readyDuration = 0, .earliestVsync = 1000}); + EXPECT_TRUE(result.has_value()); + EXPECT_EQ(500, *result); + mMockClock.advanceBy(300); + + result = mDispatch->schedule(cb, + {.workDuration = 800, .readyDuration = 0, .earliestVsync = 1000}); + EXPECT_TRUE(result.has_value()); + EXPECT_EQ(200, *result); + + advanceToNextCallback(); + ASSERT_THAT(cb.mCalls.size(), Eq(1)); +} + class VSyncDispatchTimerQueueEntryTest : public testing::Test { protected: nsecs_t const mPeriod = 1000; |