SF: Pull CompositorTiming constructor to libgui

Clean up the snapping logic and add tests.

Bug: 185535769
Test: libgui_test
Change-Id: I08302d7c08a0932787e66203873b6aa6ff9f7a78

11 files changed, 198 insertions, 80 deletions

diff --git a/libs/gui/Android.bp b/libs/gui/Android.bp
index 9086f1ce62..828da2e090 100644
--- a/libs/gui/Android.bp
+++ b/libs/gui/Android.bp
@@ -193,6 +193,7 @@ cc_library_shared {
         "BufferHubConsumer.cpp",
         "BufferHubProducer.cpp",
         "BufferItemConsumer.cpp",
+        "CompositorTiming.cpp",
         "ConsumerBase.cpp",
         "CpuConsumer.cpp",
         "DebugEGLImageTracker.cpp",
diff --git a/libs/gui/CompositorTiming.cpp b/libs/gui/CompositorTiming.cpp
new file mode 100644
index 0000000000..50f7b252b6
--- /dev/null
+++ b/libs/gui/CompositorTiming.cpp
@@ -0,0 +1,54 @@
+/*
+ * Copyright 2022 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.
+ */
+
+#define LOG_TAG "CompositorTiming"
+
+#include <cutils/compiler.h>
+#include <gui/CompositorTiming.h>
+#include <log/log.h>
+
+namespace android::gui {
+
+CompositorTiming::CompositorTiming(nsecs_t vsyncDeadline, nsecs_t vsyncPeriod, nsecs_t vsyncPhase,
+                                   nsecs_t presentLatency) {
+    if (CC_UNLIKELY(vsyncPeriod <= 0)) {
+        ALOGE("Invalid VSYNC period");
+        return;
+    }
+
+    const nsecs_t idealLatency = [=] {
+        // Modulo rounds toward 0 not INT64_MIN, so treat signs separately.
+        if (vsyncPhase < 0) return -vsyncPhase % vsyncPeriod;
+
+        const nsecs_t latency = (vsyncPeriod - vsyncPhase) % vsyncPeriod;
+        return latency > 0 ? latency : vsyncPeriod;
+    }();
+
+    // Snap the latency to a value that removes scheduling jitter from the composite and present
+    // times, which often have >1ms of jitter. Reducing jitter is important if an app attempts to
+    // extrapolate something like user input to an accurate present time. Snapping also allows an
+    // app to precisely calculate vsyncPhase with (presentLatency % interval).
+    const nsecs_t bias = vsyncPeriod / 2;
+    const nsecs_t extraVsyncs = (presentLatency - idealLatency + bias) / vsyncPeriod;
+    const nsecs_t snappedLatency =
+            extraVsyncs > 0 ? idealLatency + extraVsyncs * vsyncPeriod : idealLatency;
+
+    this->deadline = vsyncDeadline - idealLatency;
+    this->interval = vsyncPeriod;
+    this->presentLatency = snappedLatency;
+}
+
+} // namespace android::gui
diff --git a/libs/gui/include/gui/CompositorTiming.h b/libs/gui/include/gui/CompositorTiming.h
new file mode 100644
index 0000000000..cb8ca7a15c
--- /dev/null
+++ b/libs/gui/include/gui/CompositorTiming.h
@@ -0,0 +1,43 @@
+/*
+ * Copyright 2022 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
+
+#include <utils/Timers.h>
+
+namespace android::gui {
+
+// Expected timing of the next composited frame, based on the timing of the latest frames.
+struct CompositorTiming {
+    static constexpr nsecs_t kDefaultVsyncPeriod = 16'666'667;
+
+    CompositorTiming() = default;
+    CompositorTiming(nsecs_t vsyncDeadline, nsecs_t vsyncPeriod, nsecs_t vsyncPhase,
+                     nsecs_t presentLatency);
+
+    // Time point when compositing is expected to start.
+    nsecs_t deadline = 0;
+
+    // Duration between consecutive frames. In other words, the VSYNC period.
+    nsecs_t interval = kDefaultVsyncPeriod;
+
+    // Duration between composite start and present. For missed frames, the extra latency is rounded
+    // to a multiple of the VSYNC period, such that the remainder (presentLatency % interval) always
+    // evaluates to the VSYNC phase offset.
+    nsecs_t presentLatency = kDefaultVsyncPeriod;
+};
+
+} // namespace android::gui
diff --git a/libs/gui/include/gui/FrameTimestamps.h b/libs/gui/include/gui/FrameTimestamps.h
index f73bc3b329..c08a9b1b6c 100644
--- a/libs/gui/include/gui/FrameTimestamps.h
+++ b/libs/gui/include/gui/FrameTimestamps.h
@@ -19,6 +19,7 @@
 
 #include <android/gui/FrameEvent.h>
 
+#include <gui/CompositorTiming.h>
 #include <ui/FenceTime.h>
 #include <utils/Flattenable.h>
 #include <utils/StrongPointer.h>
@@ -33,6 +34,7 @@ namespace android {
 struct FrameEvents;
 class FrameEventHistoryDelta;
 
+using gui::CompositorTiming;
 using gui::FrameEvent;
 
 // A collection of timestamps corresponding to a single frame.
@@ -83,12 +85,6 @@ struct FrameEvents {
     std::shared_ptr<FenceTime> releaseFence{FenceTime::NO_FENCE};
 };
 
-struct CompositorTiming {
-    nsecs_t deadline{0};
-    nsecs_t interval{16666667};
-    nsecs_t presentLatency{16666667};
-};
-
 // A short history of frames that are synchronized between the consumer and
 // producer via deltas.
 class FrameEventHistory {
diff --git a/libs/gui/tests/Android.bp b/libs/gui/tests/Android.bp
index fc68ad27c5..7b37b25a61 100644
--- a/libs/gui/tests/Android.bp
+++ b/libs/gui/tests/Android.bp
@@ -24,6 +24,7 @@ cc_test {
         "BLASTBufferQueue_test.cpp",
         "BufferItemConsumer_test.cpp",
         "BufferQueue_test.cpp",
+        "CompositorTiming_test.cpp",
         "CpuConsumer_test.cpp",
         "EndToEndNativeInputTest.cpp",
         "DisplayInfo_test.cpp",
diff --git a/libs/gui/tests/CompositorTiming_test.cpp b/libs/gui/tests/CompositorTiming_test.cpp
new file mode 100644
index 0000000000..d8bb21d582
--- /dev/null
+++ b/libs/gui/tests/CompositorTiming_test.cpp
@@ -0,0 +1,61 @@
+/*
+ * Copyright 2022 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.
+ */
+
+#include <gtest/gtest.h>
+#include <gui/CompositorTiming.h>
+
+namespace android::test {
+namespace {
+
+constexpr nsecs_t kMillisecond = 1'000'000;
+constexpr nsecs_t kVsyncPeriod = 8'333'333;
+constexpr nsecs_t kVsyncPhase = -2'166'667;
+constexpr nsecs_t kIdealLatency = -kVsyncPhase;
+
+} // namespace
+
+TEST(CompositorTimingTest, InvalidVsyncPeriod) {
+    const nsecs_t vsyncDeadline = systemTime();
+    constexpr nsecs_t kInvalidVsyncPeriod = -1;
+
+    const gui::CompositorTiming timing(vsyncDeadline, kInvalidVsyncPeriod, kVsyncPhase,
+                                       kIdealLatency);
+
+    EXPECT_EQ(timing.deadline, 0);
+    EXPECT_EQ(timing.interval, gui::CompositorTiming::kDefaultVsyncPeriod);
+    EXPECT_EQ(timing.presentLatency, gui::CompositorTiming::kDefaultVsyncPeriod);
+}
+
+TEST(CompositorTimingTest, PresentLatencySnapping) {
+    for (nsecs_t presentDelay = 0, compositeTime = systemTime(); presentDelay < 10 * kVsyncPeriod;
+         presentDelay += kMillisecond, compositeTime += kVsyncPeriod) {
+        const nsecs_t presentLatency = kIdealLatency + presentDelay;
+        const nsecs_t vsyncDeadline = compositeTime + presentLatency + kVsyncPeriod;
+
+        const gui::CompositorTiming timing(vsyncDeadline, kVsyncPeriod, kVsyncPhase,
+                                           presentLatency);
+
+        EXPECT_EQ(timing.deadline, compositeTime + presentDelay + kVsyncPeriod);
+        EXPECT_EQ(timing.interval, kVsyncPeriod);
+
+        // The presentDelay should be rounded to a multiple of the VSYNC period, such that the
+        // remainder (presentLatency % interval) always evaluates to the VSYNC phase offset.
+        EXPECT_GE(timing.presentLatency, kIdealLatency);
+        EXPECT_EQ(timing.presentLatency % timing.interval, kIdealLatency);
+    }
+}
+
+} // namespace android::test
diff --git a/libs/gui/tests/Surface_test.cpp b/libs/gui/tests/Surface_test.cpp
index 7eff3b318f..391a0aa7d4 100644
--- a/libs/gui/tests/Surface_test.cpp
+++ b/libs/gui/tests/Surface_test.cpp
@@ -1087,20 +1087,30 @@ class GetFrameTimestampsTest : public ::testing::Test {
 protected:
     struct FenceAndFenceTime {
         explicit FenceAndFenceTime(FenceToFenceTimeMap& fenceMap)
-           : mFence(new Fence),
-             mFenceTime(fenceMap.createFenceTimeForTest(mFence)) {}
-        sp<Fence> mFence { nullptr };
-        std::shared_ptr<FenceTime> mFenceTime { nullptr };
+              : mFenceTime(fenceMap.createFenceTimeForTest(mFence)) {}
+
+        sp<Fence> mFence = sp<Fence>::make();
+        std::shared_ptr<FenceTime> mFenceTime;
     };
 
+    static CompositorTiming makeCompositorTiming(nsecs_t deadline = 1'000'000'000,
+                                                 nsecs_t interval = 16'666'667,
+                                                 nsecs_t presentLatency = 50'000'000) {
+        CompositorTiming timing;
+        timing.deadline = deadline;
+        timing.interval = interval;
+        timing.presentLatency = presentLatency;
+        return timing;
+    }
+
     struct RefreshEvents {
         RefreshEvents(FenceToFenceTimeMap& fenceMap, nsecs_t refreshStart)
-          : mFenceMap(fenceMap),
-            kCompositorTiming(
-                {refreshStart, refreshStart + 1, refreshStart + 2 }),
-            kStartTime(refreshStart + 3),
-            kGpuCompositionDoneTime(refreshStart + 4),
-            kPresentTime(refreshStart + 5) {}
+              : mFenceMap(fenceMap),
+                kCompositorTiming(
+                        makeCompositorTiming(refreshStart, refreshStart + 1, refreshStart + 2)),
+                kStartTime(refreshStart + 3),
+                kGpuCompositionDoneTime(refreshStart + 4),
+                kPresentTime(refreshStart + 5) {}
 
         void signalPostCompositeFences() {
             mFenceMap.signalAllForTest(
@@ -1110,8 +1120,8 @@ protected:
 
         FenceToFenceTimeMap& mFenceMap;
 
-        FenceAndFenceTime mGpuCompositionDone { mFenceMap };
-        FenceAndFenceTime mPresent { mFenceMap };
+        FenceAndFenceTime mGpuCompositionDone{mFenceMap};
+        FenceAndFenceTime mPresent{mFenceMap};
 
         const CompositorTiming kCompositorTiming;
 
@@ -1377,11 +1387,7 @@ TEST_F(GetFrameTimestampsTest, DefaultDisabled) {
 // This test verifies that the frame timestamps are retrieved if explicitly
 // enabled via native_window_enable_frame_timestamps.
 TEST_F(GetFrameTimestampsTest, EnabledSimple) {
-    CompositorTiming initialCompositorTiming {
-        1000000000, // 1s deadline
-        16666667, // 16ms interval
-        50000000, // 50ms present latency
-    };
+    const CompositorTiming initialCompositorTiming = makeCompositorTiming();
     mCfeh->initializeCompositorTiming(initialCompositorTiming);
 
     enableFrameTimestamps();
@@ -1520,11 +1526,7 @@ TEST_F(GetFrameTimestampsTest, SnapToNextTickOverflow) {
 // This verifies the compositor timing is updated by refresh events
 // and piggy backed on a queue, dequeue, and enabling of timestamps..
 TEST_F(GetFrameTimestampsTest, CompositorTimingUpdatesBasic) {
-    CompositorTiming initialCompositorTiming {
-        1000000000, // 1s deadline
-        16666667, // 16ms interval
-        50000000, // 50ms present latency
-    };
+    const CompositorTiming initialCompositorTiming = makeCompositorTiming();
     mCfeh->initializeCompositorTiming(initialCompositorTiming);
 
     enableFrameTimestamps();
@@ -1605,11 +1607,7 @@ TEST_F(GetFrameTimestampsTest, CompositorTimingUpdatesBasic) {
 // This verifies the compositor deadline properly snaps to the the next
 // deadline based on the current time.
 TEST_F(GetFrameTimestampsTest, CompositorTimingDeadlineSnaps) {
-    CompositorTiming initialCompositorTiming {
-        1000000000, // 1s deadline
-        16666667, // 16ms interval
-        50000000, // 50ms present latency
-    };
+    const CompositorTiming initialCompositorTiming = makeCompositorTiming();
     mCfeh->initializeCompositorTiming(initialCompositorTiming);
 
     enableFrameTimestamps();
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 9f1cf9c362..bb58eb48b6 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2344,39 +2344,6 @@ nsecs_t SurfaceFlinger::trackPresentLatency(nsecs_t compositeTime,
     return compositeToPresentLatency;
 }
 
-CompositorTiming SurfaceFlinger::makeCompositorTiming(nsecs_t vsyncDeadline, nsecs_t vsyncPeriod,
-                                                      nsecs_t compositeToPresentLatency) {
-    // Avoid division by 0 by defaulting to 60Hz
-    vsyncPeriod = vsyncPeriod ?: (60_Hz).getPeriodNsecs();
-
-    // Integer division and modulo round toward 0 not -inf, so we need to
-    // treat negative and positive offsets differently.
-    nsecs_t idealLatency = (mVsyncConfiguration->getCurrentConfigs().late.sfOffset > 0)
-            ? (vsyncPeriod -
-               (mVsyncConfiguration->getCurrentConfigs().late.sfOffset % vsyncPeriod))
-            : ((-mVsyncConfiguration->getCurrentConfigs().late.sfOffset) % vsyncPeriod);
-
-    // Just in case mVsyncConfiguration->getCurrentConfigs().late.sf == -vsyncInterval.
-    if (idealLatency <= 0) {
-        idealLatency = vsyncPeriod;
-    }
-
-    // Snap the latency to a value that removes scheduling jitter from the
-    // composition and present times, which often have >1ms of jitter.
-    // Reducing jitter is important if an app attempts to extrapolate
-    // something (such as user input) to an accurate diasplay time.
-    // Snapping also allows an app to precisely calculate
-    // mVsyncConfiguration->getCurrentConfigs().late.sf with (presentLatency % interval).
-    const nsecs_t bias = vsyncPeriod / 2;
-    const int64_t extraVsyncs = ((compositeToPresentLatency - idealLatency + bias) / vsyncPeriod);
-    const nsecs_t snappedCompositeToPresentLatency =
-            (extraVsyncs > 0) ? idealLatency + (extraVsyncs * vsyncPeriod) : idealLatency;
-
-    return {.deadline = vsyncDeadline - idealLatency,
-            .interval = vsyncPeriod,
-            .presentLatency = snappedCompositeToPresentLatency};
-}
-
 bool SurfaceFlinger::isHdrLayer(Layer* layer) const {
     // Treat all layers as non-HDR if:
     // 1. They do not have a valid HDR dataspace. Currently we treat those as PQ or HLG. and
@@ -2463,7 +2430,7 @@ void SurfaceFlinger::postComposition() {
 
     // We use the CompositionEngine::getLastFrameRefreshTimestamp() which might
     // be sampled a little later than when we started doing work for this frame,
-    // but that should be okay since makeCompositorTiming has snapping logic.
+    // but that should be okay since CompositorTiming has snapping logic.
     const nsecs_t compositeTime = mCompositionEngine->getLastFrameRefreshTimestamp();
     const nsecs_t presentLatency =
             trackPresentLatency(compositeTime, mPreviousPresentFences[0].fenceTime);
@@ -2471,9 +2438,10 @@ void SurfaceFlinger::postComposition() {
     const auto& schedule = mScheduler->getVsyncSchedule();
     const TimePoint vsyncDeadline = schedule.vsyncDeadlineAfter(TimePoint::fromNs(now));
     const Period vsyncPeriod = schedule.period();
+    const nsecs_t vsyncPhase = mVsyncConfiguration->getCurrentConfigs().late.sfOffset;
 
-    const CompositorTiming compositorTiming =
-            makeCompositorTiming(vsyncDeadline.ns(), vsyncPeriod.ns(), presentLatency);
+    const CompositorTiming compositorTiming(vsyncDeadline.ns(), vsyncPeriod.ns(), vsyncPhase,
+                                            presentLatency);
 
     for (const auto& layer: mLayersWithQueuedFrames) {
         layer->onPostComposition(display, glCompositionDoneFenceTime,
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 0e5386fa81..71b9d5eea1 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -31,6 +31,7 @@
 #include <ftl/future.h>
 #include <ftl/small_map.h>
 #include <gui/BufferQueue.h>
+#include <gui/CompositorTiming.h>
 #include <gui/FrameTimestamps.h>
 #include <gui/ISurfaceComposer.h>
 #include <gui/ITransactionCompletedListener.h>
@@ -958,9 +959,6 @@ private:
     // Returns the composite-to-present latency of the latest presented frame.
     nsecs_t trackPresentLatency(nsecs_t compositeTime, std::shared_ptr<FenceTime> presentFenceTime);
 
-    CompositorTiming makeCompositorTiming(nsecs_t vsyncDeadline, nsecs_t vsyncPeriod,
-                                          nsecs_t compositeToPresentLatency);
-
     void postFrame() REQUIRES(kMainThreadContext);
 
     /*
diff --git a/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h b/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h
index 69ef2bf18b..40281a7dd0 100644
--- a/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h
+++ b/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h
@@ -630,9 +630,6 @@ public:
         mFlinger->postComposition();
 
         mFlinger->trackPresentLatency(mFdp.ConsumeIntegral<nsecs_t>(), FenceTime::NO_FENCE);
-        mFlinger->makeCompositorTiming(mFdp.ConsumeIntegral<nsecs_t>(),
-                                       mFdp.ConsumeIntegral<nsecs_t>(),
-                                       mFdp.ConsumeIntegral<nsecs_t>());
 
         FTL_FAKE_GUARD(kMainThreadContext, mFlinger->postFrame());
         mFlinger->calculateExpectedPresentTime({});
diff --git a/services/surfaceflinger/fuzzer/surfaceflinger_layer_fuzzer.cpp b/services/surfaceflinger/fuzzer/surfaceflinger_layer_fuzzer.cpp
index 4aef0173b6..b0a8a5143d 100644
--- a/services/surfaceflinger/fuzzer/surfaceflinger_layer_fuzzer.cpp
+++ b/services/surfaceflinger/fuzzer/surfaceflinger_layer_fuzzer.cpp
@@ -117,17 +117,18 @@ void LayerFuzzer::invokeBufferStateLayer() {
     sp<Fence> fence = sp<Fence>::make();
     const std::shared_ptr<FenceTime> fenceTime = std::make_shared<FenceTime>(fence);
 
-    const CompositorTiming compositor = {mFdp.ConsumeIntegral<int64_t>(),
-                                         mFdp.ConsumeIntegral<int64_t>(),
-                                         mFdp.ConsumeIntegral<int64_t>()};
+    const CompositorTiming compositorTiming(mFdp.ConsumeIntegral<int64_t>(),
+                                            mFdp.ConsumeIntegral<int64_t>(),
+                                            mFdp.ConsumeIntegral<int64_t>(),
+                                            mFdp.ConsumeIntegral<int64_t>());
 
     layer->onLayerDisplayed(ftl::yield<FenceResult>(fence).share());
     layer->onLayerDisplayed(
             ftl::yield<FenceResult>(base::unexpected(mFdp.ConsumeIntegral<status_t>())).share());
 
     layer->releasePendingBuffer(mFdp.ConsumeIntegral<int64_t>());
-    layer->finalizeFrameEventHistory(fenceTime, compositor);
-    layer->onPostComposition(nullptr, fenceTime, fenceTime, compositor);
+    layer->finalizeFrameEventHistory(fenceTime, compositorTiming);
+    layer->onPostComposition(nullptr, fenceTime, fenceTime, compositorTiming);
     layer->isBufferDue(mFdp.ConsumeIntegral<int64_t>());
 
     layer->setTransform(mFdp.ConsumeIntegral<uint32_t>());