SurfaceFlinger: get nextPredictedPresentTime directly from frame timeline

ag/13715677 moved the call to check whether a frame is early or not
before a transaction is applied.
This created a bug in BufferStateLayer::nextPredictedPresentTime since
it is checking the drawing state surface frame, which is not valid since
the transaction is not applied yet. This CL is fixing this by pasing the
vsync id itself, and getting the expected present time base on that
vsync id.

Change-Id: I0f95f2a3a2efff921964a6fb5f9b50e0fcc65a85
Test: launch an app and observe systraces
Bug: 181978893

9 files changed, 32 insertions, 23 deletions

diff --git a/services/surfaceflinger/BufferLayer.cpp b/services/surfaceflinger/BufferLayer.cpp
index a96dffdcb7..eac3d95d8a 100644
--- a/services/surfaceflinger/BufferLayer.cpp
+++ b/services/surfaceflinger/BufferLayer.cpp
@@ -424,12 +424,12 @@ bool BufferLayer::shouldPresentNow(nsecs_t expectedPresentTime) const {
     return isBufferDue(expectedPresentTime);
 }
 
-bool BufferLayer::frameIsEarly(nsecs_t expectedPresentTime) const {
+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();
+    const auto presentTime = nextPredictedPresentTime(vsyncId);
     if (!presentTime.has_value()) {
         return false;
     }
diff --git a/services/surfaceflinger/BufferLayer.h b/services/surfaceflinger/BufferLayer.h
index 2118f4ad18..d215298eda 100644
--- a/services/surfaceflinger/BufferLayer.h
+++ b/services/surfaceflinger/BufferLayer.h
@@ -191,7 +191,7 @@ protected:
 
     // Returns true if the next frame is considered too early to present
     // at the given expectedPresentTime
-    bool frameIsEarly(nsecs_t expectedPresentTime) const;
+    bool frameIsEarly(nsecs_t expectedPresentTime, int64_t vsyncId) const;
 
     std::atomic<bool> mAutoRefresh{false};
     std::atomic<bool> mSidebandStreamChanged{false};
@@ -238,7 +238,7 @@ private:
     FloatRect computeSourceBounds(const FloatRect& parentBounds) const override;
 
     // Returns the predicted present time of the next frame if available
-    virtual std::optional<nsecs_t> nextPredictedPresentTime() const = 0;
+    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
diff --git a/services/surfaceflinger/BufferQueueLayer.cpp b/services/surfaceflinger/BufferQueueLayer.cpp
index 3615a0209f..63dd25f72f 100644
--- a/services/surfaceflinger/BufferQueueLayer.cpp
+++ b/services/surfaceflinger/BufferQueueLayer.cpp
@@ -215,7 +215,7 @@ bool BufferQueueLayer::hasFrameUpdate() const {
     return mQueuedFrames > 0;
 }
 
-std::optional<nsecs_t> BufferQueueLayer::nextPredictedPresentTime() const {
+std::optional<nsecs_t> BufferQueueLayer::nextPredictedPresentTime(int64_t /*vsyncId*/) const {
     Mutex::Autolock lock(mQueueItemLock);
     if (mQueueItems.empty()) {
         return std::nullopt;
diff --git a/services/surfaceflinger/BufferQueueLayer.h b/services/surfaceflinger/BufferQueueLayer.h
index 0ea02e19a7..3a34b952f2 100644
--- a/services/surfaceflinger/BufferQueueLayer.h
+++ b/services/surfaceflinger/BufferQueueLayer.h
@@ -117,7 +117,7 @@ private:
     // Temporary - Used only for LEGACY camera mode.
     uint32_t getProducerStickyTransform() const;
 
-    std::optional<nsecs_t> nextPredictedPresentTime() const override;
+    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 296085a0b5..58221e71ab 100644
--- a/services/surfaceflinger/BufferStateLayer.cpp
+++ b/services/surfaceflinger/BufferStateLayer.cpp
@@ -605,13 +605,14 @@ bool BufferStateLayer::hasFrameUpdate() const {
     return mCurrentStateModified && (c.buffer != nullptr || c.bgColorLayer != nullptr);
 }
 
-std::optional<nsecs_t> BufferStateLayer::nextPredictedPresentTime() const {
-    const State& drawingState(getDrawingState());
-    if (!drawingState.isAutoTimestamp || !drawingState.bufferSurfaceFrameTX) {
+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 drawingState.bufferSurfaceFrameTX->getPredictions().presentTime;
+    return prediction->presentTime;
 }
 
 status_t BufferStateLayer::updateTexImage(bool& /*recomputeVisibleRegions*/, nsecs_t latchTime,
diff --git a/services/surfaceflinger/BufferStateLayer.h b/services/surfaceflinger/BufferStateLayer.h
index 6b422ea204..db0ebfc479 100644
--- a/services/surfaceflinger/BufferStateLayer.h
+++ b/services/surfaceflinger/BufferStateLayer.h
@@ -156,7 +156,7 @@ private:
 
     bool bufferNeedsFiltering() const override;
 
-    std::optional<nsecs_t> nextPredictedPresentTime() const override;
+    std::optional<nsecs_t> nextPredictedPresentTime(int64_t vsyncId) const override;
 
     static const std::array<float, 16> IDENTITY_MATRIX;
 
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index ce97155eaa..34a9f39b6d 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -922,7 +922,9 @@ public:
 
     pid_t getOwnerPid() { return mOwnerPid; }
 
-    virtual bool frameIsEarly(nsecs_t /*expectedPresentTime*/) const { return false; }
+    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
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index ad91183f18..e27b8dd01a 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3315,10 +3315,10 @@ void SurfaceFlinger::flushTransactionQueues() {
 
                 while (!transactionQueue.empty()) {
                     const auto& transaction = transactionQueue.front();
-                    if (!transactionIsReadyToBeApplied(transaction.isAutoTimestamp,
+                    if (!transactionIsReadyToBeApplied(transaction.frameTimelineInfo,
+                                                       transaction.isAutoTimestamp,
                                                        transaction.desiredPresentTime,
-                                                       transaction.states,
-                                                       pendingBuffers)) {
+                                                       transaction.states, pendingBuffers)) {
                         setTransactionFlags(eTransactionFlushNeeded);
                         break;
                     }
@@ -3342,10 +3342,10 @@ void SurfaceFlinger::flushTransactionQueues() {
                 const auto& transaction = mTransactionQueue.front();
                 bool pendingTransactions = mPendingTransactionQueues.find(transaction.applyToken) !=
                         mPendingTransactionQueues.end();
-                if (!transactionIsReadyToBeApplied(transaction.isAutoTimestamp,
+                if (!transactionIsReadyToBeApplied(transaction.frameTimelineInfo,
+                                                   transaction.isAutoTimestamp,
                                                    transaction.desiredPresentTime,
-                                                   transaction.states,
-                                                   pendingBuffers) ||
+                                                   transaction.states, pendingBuffers) ||
                     pendingTransactions) {
                     mPendingTransactionQueues[transaction.applyToken].push(transaction);
                 } else {
@@ -3375,7 +3375,8 @@ bool SurfaceFlinger::transactionFlushNeeded() {
 }
 
 bool SurfaceFlinger::transactionIsReadyToBeApplied(
-        bool isAutoTimestamp, int64_t desiredPresentTime, const Vector<ComposerState>& states,
+        const FrameTimelineInfo& info, bool isAutoTimestamp, int64_t desiredPresentTime,
+        const Vector<ComposerState>& states,
         std::unordered_set<sp<IBinder>, ISurfaceComposer::SpHash<IBinder>>& pendingBuffers) {
     const nsecs_t expectedPresentTime = mExpectedPresentTime.load();
     bool ready = true;
@@ -3391,6 +3392,7 @@ bool SurfaceFlinger::transactionIsReadyToBeApplied(
         if (!(s.what & layer_state_t::eAcquireFenceChanged)) {
             continue;
         }
+
         if (s.acquireFence && s.acquireFence->getStatus() == Fence::Status::Unsignaled) {
             ready = false;
         }
@@ -3405,7 +3407,10 @@ bool SurfaceFlinger::transactionIsReadyToBeApplied(
         if (!layer) {
             continue;
         }
-        if (layer->frameIsEarly(expectedPresentTime)) {
+
+        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()");
             return false;
         }
@@ -3415,8 +3420,8 @@ bool SurfaceFlinger::transactionIsReadyToBeApplied(
             ready = false;
         }
 
-        // If backpressure is enabled and we already have a buffer to commit, keep the transaction
-        // in the queue.
+        // If backpressure is enabled and we already have a buffer to commit, keep the
+        // transaction in the queue.
         const bool hasPendingBuffer = pendingBuffers.find(s.surface) != pendingBuffers.end();
         if (layer->backpressureEnabled() && hasPendingBuffer && isAutoTimestamp) {
             ready = false;
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 6434ca29ff..6d3b4efa84 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -800,7 +800,8 @@ private:
     void commitTransaction() REQUIRES(mStateLock);
     void commitOffscreenLayers();
     bool transactionIsReadyToBeApplied(
-            bool isAutoTimestamp, int64_t desiredPresentTime, const Vector<ComposerState>& states,
+            const FrameTimelineInfo& info, bool isAutoTimestamp, int64_t desiredPresentTime,
+            const Vector<ComposerState>& states,
             std::unordered_set<sp<IBinder>, ISurfaceComposer::SpHash<IBinder>>& pendingBuffers)
             REQUIRES(mStateLock);
     uint32_t setDisplayStateLocked(const DisplayState& s) REQUIRES(mStateLock);