SurfaceFlinger: remove SurfaceControl level vsyncId setting

FrameTimelineInfo can be set on the entire transaction, or for an
individual SurfaceControl. Later in the code the FrameTimelineInfo
is unified based on the most recent vsyncId. For this reason we are
removing the setting of a FrameTimelineInfo on a SurfaceControl and
instead we use the transaction's one.

Test: adb shell /data/nativetest64/SurfaceFlinger_test/SurfaceFlinger_test
Bug: 181978893
Bug: 169901895
Change-Id: Id4a8e46d57fbda66f6d478be82313482053dce20

15 files changed, 38 insertions, 138 deletions

diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp
index f778232803..5b1b975283 100644
--- a/libs/gui/BLASTBufferQueue.cpp
+++ b/libs/gui/BLASTBufferQueue.cpp
@@ -401,7 +401,7 @@ void BLASTBufferQueue::processNextBufferLocked(bool useNextTransaction) {
     t->setFrameNumber(mSurfaceControl, bufferItem.mFrameNumber);
 
     if (!mNextFrameTimelineInfoQueue.empty()) {
-        t->setFrameTimelineInfo(mSurfaceControl, mNextFrameTimelineInfoQueue.front());
+        t->setFrameTimelineInfo(mNextFrameTimelineInfoQueue.front());
         mNextFrameTimelineInfoQueue.pop();
     }
 
diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp
index 7a18ca61fe..a84e741482 100644
--- a/libs/gui/LayerState.cpp
+++ b/libs/gui/LayerState.cpp
@@ -62,7 +62,6 @@ layer_state_t::layer_state_t()
         shouldBeSeamless(true),
         fixedTransformHint(ui::Transform::ROT_INVALID),
         frameNumber(0),
-        frameTimelineInfo(),
         autoRefresh(false),
         releaseBufferListener(nullptr) {
     matrix.dsdx = matrix.dtdy = 1.0f;
@@ -151,7 +150,6 @@ status_t layer_state_t::write(Parcel& output) const
     SAFE_PARCEL(output.writeBool, shouldBeSeamless);
     SAFE_PARCEL(output.writeUint32, fixedTransformHint);
     SAFE_PARCEL(output.writeUint64, frameNumber);
-    SAFE_PARCEL(frameTimelineInfo.write, output);
     SAFE_PARCEL(output.writeBool, autoRefresh);
     SAFE_PARCEL(output.writeStrongBinder, IInterface::asBinder(releaseBufferListener));
 
@@ -274,7 +272,6 @@ status_t layer_state_t::read(const Parcel& input)
     SAFE_PARCEL(input.readUint32, &tmpUint32);
     fixedTransformHint = static_cast<ui::Transform::RotationFlags>(tmpUint32);
     SAFE_PARCEL(input.readUint64, &frameNumber);
-    SAFE_PARCEL(frameTimelineInfo.read, input);
     SAFE_PARCEL(input.readBool, &autoRefresh);
 
     tmpBinder = nullptr;
@@ -543,10 +540,6 @@ void layer_state_t::merge(const layer_state_t& other) {
         what |= eFrameNumberChanged;
         frameNumber = other.frameNumber;
     }
-    if (other.what & eFrameTimelineInfoChanged) {
-        what |= eFrameTimelineInfoChanged;
-        frameTimelineInfo.merge(other.frameTimelineInfo);
-    }
     if (other.what & eAutoRefreshChanged) {
         what |= eAutoRefreshChanged;
         autoRefresh = other.autoRefresh;
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index 11fe49039d..eaccc5bab5 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -1630,20 +1630,7 @@ SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setFixed
 
 SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setFrameTimelineInfo(
         const FrameTimelineInfo& frameTimelineInfo) {
-    mFrameTimelineInfo = frameTimelineInfo;
-    return *this;
-}
-
-SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setFrameTimelineInfo(
-        const sp<SurfaceControl>& sc, const FrameTimelineInfo& frameTimelineInfo) {
-    layer_state_t* s = getLayerState(sc);
-    if (!s) {
-        mStatus = BAD_INDEX;
-        return *this;
-    }
-
-    s->what |= layer_state_t::eFrameTimelineInfoChanged;
-    s->frameTimelineInfo = frameTimelineInfo;
+    mFrameTimelineInfo.merge(frameTimelineInfo);
     return *this;
 }
 
diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h
index d68a9cfa4a..4a52168315 100644
--- a/libs/gui/include/gui/LayerState.h
+++ b/libs/gui/include/gui/LayerState.h
@@ -133,10 +133,9 @@ struct layer_state_t {
         eProducerDisconnect = 0x100'00000000,
         eFixedTransformHintChanged = 0x200'00000000,
         eFrameNumberChanged = 0x400'00000000,
-        eFrameTimelineInfoChanged = 0x800'00000000,
-        eBlurRegionsChanged = 0x1000'00000000,
-        eAutoRefreshChanged = 0x2000'00000000,
-        eStretchChanged = 0x4000'00000000,
+        eBlurRegionsChanged = 0x800'00000000,
+        eAutoRefreshChanged = 0x1000'00000000,
+        eStretchChanged = 0x2000'00000000,
     };
 
     layer_state_t();
@@ -239,8 +238,6 @@ struct layer_state_t {
     // graphics producer.
     uint64_t frameNumber;
 
-    FrameTimelineInfo frameTimelineInfo;
-
     // Indicates that the consumer should acquire the next frame as soon as it
     // can and not wait for a frame to become available. This is only relevant
     // in shared buffer mode.
diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h
index f29983cef7..ae7170e1aa 100644
--- a/libs/gui/include/gui/SurfaceComposerClient.h
+++ b/libs/gui/include/gui/SurfaceComposerClient.h
@@ -530,9 +530,6 @@ public:
         // to the transaction, and the input event id that identifies the input event that caused
         // the current frame.
         Transaction& setFrameTimelineInfo(const FrameTimelineInfo& frameTimelineInfo);
-        // Variant that only applies to a specific SurfaceControl.
-        Transaction& setFrameTimelineInfo(const sp<SurfaceControl>& sc,
-                                          const FrameTimelineInfo& frameTimelineInfo);
 
         // Indicates that the consumer should acquire the next frame as soon as it
         // can and not wait for a frame to become available. This is only relevant
diff --git a/services/surfaceflinger/BufferLayer.cpp b/services/surfaceflinger/BufferLayer.cpp
index eac3d95d8a..b3f97924bd 100644
--- a/services/surfaceflinger/BufferLayer.cpp
+++ b/services/surfaceflinger/BufferLayer.cpp
@@ -424,24 +424,6 @@ bool BufferLayer::shouldPresentNow(nsecs_t expectedPresentTime) const {
     return isBufferDue(expectedPresentTime);
 }
 
-bool BufferLayer::frameIsEarly(nsecs_t expectedPresentTime, int64_t vsyncId) const {
-    // TODO(b/169901895): kEarlyLatchVsyncThreshold should be based on the
-    // vsync period. We can do this change as soon as ag/13100772 is merged.
-    constexpr static std::chrono::nanoseconds kEarlyLatchVsyncThreshold = 5ms;
-
-    const auto presentTime = nextPredictedPresentTime(vsyncId);
-    if (!presentTime.has_value()) {
-        return false;
-    }
-
-    if (std::abs(*presentTime - expectedPresentTime) >= kEarlyLatchMaxThreshold.count()) {
-        return false;
-    }
-
-    return *presentTime >= expectedPresentTime &&
-            *presentTime - expectedPresentTime >= kEarlyLatchVsyncThreshold.count();
-}
-
 bool BufferLayer::latchBuffer(bool& recomputeVisibleRegions, nsecs_t latchTime,
                               nsecs_t expectedPresentTime) {
     ATRACE_CALL();
diff --git a/services/surfaceflinger/BufferLayer.h b/services/surfaceflinger/BufferLayer.h
index 5fed79ffed..b8d3f12322 100644
--- a/services/surfaceflinger/BufferLayer.h
+++ b/services/surfaceflinger/BufferLayer.h
@@ -189,10 +189,6 @@ protected:
     // specific logic
     virtual bool isBufferDue(nsecs_t /*expectedPresentTime*/) const = 0;
 
-    // Returns true if the next frame is considered too early to present
-    // at the given expectedPresentTime
-    bool frameIsEarly(nsecs_t expectedPresentTime, int64_t vsyncId) const;
-
     std::atomic<bool> mAutoRefresh{false};
     std::atomic<bool> mSidebandStreamChanged{false};
 
@@ -236,13 +232,6 @@ private:
     std::unique_ptr<compositionengine::LayerFECompositionState> mCompositionState;
 
     FloatRect computeSourceBounds(const FloatRect& parentBounds) const override;
-
-    // Returns the predicted present time of the next frame if available
-    virtual std::optional<nsecs_t> nextPredictedPresentTime(int64_t vsyncId) const = 0;
-
-    // The amount of time SF can delay a frame if it is considered early based
-    // on the VsyncModulator::VsyncConfig::appWorkDuration
-    static constexpr std::chrono::nanoseconds kEarlyLatchMaxThreshold = 100ms;
 };
 
 } // namespace android
diff --git a/services/surfaceflinger/BufferQueueLayer.cpp b/services/surfaceflinger/BufferQueueLayer.cpp
index 63dd25f72f..51e85c4dcc 100644
--- a/services/surfaceflinger/BufferQueueLayer.cpp
+++ b/services/surfaceflinger/BufferQueueLayer.cpp
@@ -215,21 +215,6 @@ bool BufferQueueLayer::hasFrameUpdate() const {
     return mQueuedFrames > 0;
 }
 
-std::optional<nsecs_t> BufferQueueLayer::nextPredictedPresentTime(int64_t /*vsyncId*/) const {
-    Mutex::Autolock lock(mQueueItemLock);
-    if (mQueueItems.empty()) {
-        return std::nullopt;
-    }
-
-    const auto& bufferData = mQueueItems[0];
-
-    if (!bufferData.item.mIsAutoTimestamp || !bufferData.surfaceFrame) {
-        return std::nullopt;
-    }
-
-    return bufferData.surfaceFrame->getPredictions().presentTime;
-}
-
 status_t BufferQueueLayer::updateTexImage(bool& recomputeVisibleRegions, nsecs_t latchTime,
                                           nsecs_t expectedPresentTime) {
     // This boolean is used to make sure that SurfaceFlinger's shadow copy
diff --git a/services/surfaceflinger/BufferQueueLayer.h b/services/surfaceflinger/BufferQueueLayer.h
index 3a34b952f2..b3b7948935 100644
--- a/services/surfaceflinger/BufferQueueLayer.h
+++ b/services/surfaceflinger/BufferQueueLayer.h
@@ -117,8 +117,6 @@ private:
     // Temporary - Used only for LEGACY camera mode.
     uint32_t getProducerStickyTransform() const;
 
-    std::optional<nsecs_t> nextPredictedPresentTime(int64_t vsyncId) const override;
-
     sp<BufferLayerConsumer> mConsumer;
     sp<IGraphicBufferProducer> mProducer;
 
diff --git a/services/surfaceflinger/BufferStateLayer.cpp b/services/surfaceflinger/BufferStateLayer.cpp
index 89dfb6fb6b..00a6d661c0 100644
--- a/services/surfaceflinger/BufferStateLayer.cpp
+++ b/services/surfaceflinger/BufferStateLayer.cpp
@@ -644,16 +644,6 @@ bool BufferStateLayer::hasFrameUpdate() const {
     return mCurrentStateModified && (c.buffer != nullptr || c.bgColorLayer != nullptr);
 }
 
-std::optional<nsecs_t> BufferStateLayer::nextPredictedPresentTime(int64_t vsyncId) const {
-    const auto prediction =
-            mFlinger->mFrameTimeline->getTokenManager()->getPredictionsForToken(vsyncId);
-    if (!prediction.has_value()) {
-        return std::nullopt;
-    }
-
-    return prediction->presentTime;
-}
-
 status_t BufferStateLayer::updateTexImage(bool& /*recomputeVisibleRegions*/, nsecs_t latchTime,
                                           nsecs_t /*expectedPresentTime*/) {
     const State& s(getDrawingState());
diff --git a/services/surfaceflinger/BufferStateLayer.h b/services/surfaceflinger/BufferStateLayer.h
index 036e8d2e85..7a3da6fec1 100644
--- a/services/surfaceflinger/BufferStateLayer.h
+++ b/services/surfaceflinger/BufferStateLayer.h
@@ -156,8 +156,6 @@ private:
 
     bool bufferNeedsFiltering() const override;
 
-    std::optional<nsecs_t> nextPredictedPresentTime(int64_t vsyncId) const override;
-
     static const std::array<float, 16> IDENTITY_MATRIX;
 
     std::unique_ptr<renderengine::Image> mTextureImage;
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index 85ff479344..1c5d6ecb03 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -930,10 +930,6 @@ public:
 
     pid_t getOwnerPid() { return mOwnerPid; }
 
-    virtual bool frameIsEarly(nsecs_t /*expectedPresentTime*/, int64_t /*vsyncId*/) const {
-        return false;
-    }
-
     // This layer is not a clone, but it's the parent to the cloned hierarchy. The
     // variable mClonedChild represents the top layer that will be cloned so this
     // layer will be the parent of mClonedChild.
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 61cc8a2ff0..e580a97b90 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3379,6 +3379,28 @@ bool SurfaceFlinger::transactionFlushNeeded() {
     return !mPendingTransactionQueues.empty() || !mTransactionQueue.empty();
 }
 
+bool SurfaceFlinger::frameIsEarly(nsecs_t expectedPresentTime, int64_t vsyncId) const {
+    // The amount of time SF can delay a frame if it is considered early based
+    // on the VsyncModulator::VsyncConfig::appWorkDuration
+    constexpr static std::chrono::nanoseconds kEarlyLatchMaxThreshold = 100ms;
+
+    const auto currentVsyncPeriod = mScheduler->getDisplayStatInfo(systemTime()).vsyncPeriod;
+    const auto earlyLatchVsyncThreshold = currentVsyncPeriod / 2;
+
+    const auto prediction = mFrameTimeline->getTokenManager()->getPredictionsForToken(vsyncId);
+    if (!prediction.has_value()) {
+        return false;
+    }
+
+    if (std::abs(prediction->presentTime - expectedPresentTime) >=
+        kEarlyLatchMaxThreshold.count()) {
+        return false;
+    }
+
+    return prediction->presentTime >= expectedPresentTime &&
+            prediction->presentTime - expectedPresentTime >= earlyLatchVsyncThreshold;
+}
+
 bool SurfaceFlinger::transactionIsReadyToBeApplied(
         const FrameTimelineInfo& info, bool isAutoTimestamp, int64_t desiredPresentTime,
         uid_t originUid, const Vector<ComposerState>& states,
@@ -3399,6 +3421,13 @@ bool SurfaceFlinger::transactionIsReadyToBeApplied(
         ready = false;
     }
 
+    // If the client didn't specify desiredPresentTime, use the vsyncId to determine the expected
+    // present time of this transaction.
+    if (isAutoTimestamp && frameIsEarly(expectedPresentTime, info.vsyncId)) {
+        ATRACE_NAME("frameIsEarly");
+        ready = false;
+    }
+
     for (const ComposerState& state : states) {
         const layer_state_t& s = state.state;
         const bool acquireFenceChanged = (s.what & layer_state_t::eAcquireFenceChanged);
@@ -3420,13 +3449,6 @@ bool SurfaceFlinger::transactionIsReadyToBeApplied(
 
         ATRACE_NAME(layer->getName().c_str());
 
-        const bool frameTimelineInfoChanged = (s.what & layer_state_t::eFrameTimelineInfoChanged);
-        const auto vsyncId = frameTimelineInfoChanged ? s.frameTimelineInfo.vsyncId : info.vsyncId;
-        if (isAutoTimestamp && layer->frameIsEarly(expectedPresentTime, vsyncId)) {
-            ATRACE_NAME("frameIsEarly()");
-            ready = false;
-        }
-
         if (acquireFenceChanged) {
             // If backpressure is enabled and we already have a buffer to commit, keep the
             // transaction in the queue.
@@ -3959,12 +3981,6 @@ uint32_t SurfaceFlinger::setClientStateLocked(
             flags |= eTraversalNeeded;
         }
     }
-    FrameTimelineInfo info;
-    if (what & layer_state_t::eFrameTimelineInfoChanged) {
-        info = s.frameTimelineInfo;
-    } else if (frameTimelineInfo.vsyncId != FrameTimelineInfo::INVALID_VSYNC_ID) {
-        info = frameTimelineInfo;
-    }
     if (what & layer_state_t::eFixedTransformHintChanged) {
         if (layer->setFixedTransformHint(s.fixedTransformHint)) {
             flags |= eTraversalNeeded | eTransformHintUpdateNeeded;
@@ -4027,12 +4043,12 @@ uint32_t SurfaceFlinger::setClientStateLocked(
                 : layer->getHeadFrameNumber(-1 /* expectedPresentTime */) + 1;
 
         if (layer->setBuffer(buffer, s.acquireFence, postTime, desiredPresentTime, isAutoTimestamp,
-                             s.cachedBuffer, frameNumber, dequeueBufferTimestamp, info,
+                             s.cachedBuffer, frameNumber, dequeueBufferTimestamp, frameTimelineInfo,
                              s.releaseBufferListener)) {
             flags |= eTraversalNeeded;
         }
-    } else if (info.vsyncId != FrameTimelineInfo::INVALID_VSYNC_ID) {
-        layer->setFrameTimelineVsyncForBufferlessTransaction(info, postTime);
+    } else if (frameTimelineInfo.vsyncId != FrameTimelineInfo::INVALID_VSYNC_ID) {
+        layer->setFrameTimelineVsyncForBufferlessTransaction(frameTimelineInfo, postTime);
     }
 
     if (layer->setTransactionCompletedListeners(callbackHandles)) flags |= eTraversalNeeded;
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index a5b06dfbd4..a7cdcd1719 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -844,6 +844,7 @@ private:
     uint32_t setDisplayStateLocked(const DisplayState& s) REQUIRES(mStateLock);
     uint32_t addInputWindowCommands(const InputWindowCommands& inputWindowCommands)
             REQUIRES(mStateLock);
+    bool frameIsEarly(nsecs_t expectedPresentTime, int64_t vsyncId) const;
     /*
      * Layer management
      */
diff --git a/services/surfaceflinger/tests/LayerState_test.cpp b/services/surfaceflinger/tests/LayerState_test.cpp
index f010786601..fa1a5ed6b0 100644
--- a/services/surfaceflinger/tests/LayerState_test.cpp
+++ b/services/surfaceflinger/tests/LayerState_test.cpp
@@ -114,34 +114,5 @@ TEST(LayerStateTest, ParcellingScreenCaptureResults) {
     ASSERT_EQ(results.result, results2.result);
 }
 
-/**
- * Parcel a layer_state_t struct, and then unparcel. Ensure that the object that was parceled
- * matches the object that's unparceled.
- */
-TEST(LayerStateTest, ParcelUnparcelLayerStateT) {
-    layer_state_t input;
-    input.frameTimelineInfo.vsyncId = 1;
-    input.frameTimelineInfo.inputEventId = 2;
-    Parcel p;
-    input.write(p);
-    layer_state_t output;
-    p.setDataPosition(0);
-    output.read(p);
-    ASSERT_EQ(input.frameTimelineInfo.vsyncId, output.frameTimelineInfo.vsyncId);
-    ASSERT_EQ(input.frameTimelineInfo.inputEventId, output.frameTimelineInfo.inputEventId);
-}
-
-TEST(LayerStateTest, LayerStateMerge_SelectsValidInputEvent) {
-    layer_state_t layer1;
-    layer1.frameTimelineInfo.inputEventId = android::os::IInputConstants::INVALID_INPUT_EVENT_ID;
-    layer_state_t layer2;
-    layer2.frameTimelineInfo.inputEventId = 1;
-    layer2.what |= layer_state_t::eFrameTimelineInfoChanged;
-
-    layer1.merge(layer2);
-
-    ASSERT_EQ(1, layer1.frameTimelineInfo.inputEventId);
-}
-
 } // namespace test
 } // namespace android