Revert "Revert "SF: opportunistically try to present the next vs..."
Revert submission 24994369-revert-24958537-b273702768-ERNZICNUUX
Reason for revert: Resubmit after fixing b/304324338
Reverted changes: /q/submissionid:24994369-revert-24958537-b273702768-ERNZICNUUX
Bug: 273702768
Change-Id: Icfb4cf6f69ae50f7bb0b5a185c9e68b43c946aa5
Test: manual
Test: presubmit
diff --git a/services/surfaceflinger/tests/Android.bp b/services/surfaceflinger/tests/Android.bp
index 36b1972..67f2dca 100644
--- a/services/surfaceflinger/tests/Android.bp
+++ b/services/surfaceflinger/tests/Android.bp
@@ -67,6 +67,7 @@
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 c99809b..91910c7 100644
--- a/services/surfaceflinger/tests/unittests/Android.bp
+++ b/services/surfaceflinger/tests/unittests/Android.bp
@@ -42,6 +42,13 @@
],
}
+cc_aconfig_library {
+ name: "libsurfaceflingerflags_test",
+ aconfig_declarations: "surfaceflinger_flags",
+ // TODO(b/304338314): uncomment the below line once the bug is fixed
+ // test: true,
+}
+
cc_test {
name: "libsurfaceflinger_unittest",
defaults: [
@@ -168,6 +175,7 @@
"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 0000000..7103684
--- /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 7af1da6..a1eda94 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,52 @@
EXPECT_THAT(cb.mReadyTime[0], Eq(2000));
}
+// TODO(b/304338314): Set the flag value instead of skipping the test
+TEST_F(VSyncDispatchTimerQueueTest, skipAVsyc) {
+ // SET_FLAG_FOR_TEST(flags::dont_skip_on_early, false);
+ if (flags::dont_skip_on_early()) GTEST_SKIP();
+
+ 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));
+}
+
+// TODO(b/304338314): Set the flag value instead of skipping the test
+TEST_F(VSyncDispatchTimerQueueTest, dontskipAVsyc) {
+ // SET_FLAG_FOR_TEST(flags::dont_skip_on_early, true);
+ if (!flags::dont_skip_on_early()) GTEST_SKIP();
+
+ 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;