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
diff --git a/services/surfaceflinger/BufferLayer.cpp b/services/surfaceflinger/BufferLayer.cpp
index a96dffd..eac3d95 100644
--- a/services/surfaceflinger/BufferLayer.cpp
+++ b/services/surfaceflinger/BufferLayer.cpp
@@ -424,12 +424,12 @@
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 2118f4a..d215298 100644
--- a/services/surfaceflinger/BufferLayer.h
+++ b/services/surfaceflinger/BufferLayer.h
@@ -191,7 +191,7 @@
// 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 @@
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 3615a02..63dd25f 100644
--- a/services/surfaceflinger/BufferQueueLayer.cpp
+++ b/services/surfaceflinger/BufferQueueLayer.cpp
@@ -215,7 +215,7 @@
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 0ea02e1..3a34b95 100644
--- a/services/surfaceflinger/BufferQueueLayer.h
+++ b/services/surfaceflinger/BufferQueueLayer.h
@@ -117,7 +117,7 @@
// 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 296085a..58221e7 100644
--- a/services/surfaceflinger/BufferStateLayer.cpp
+++ b/services/surfaceflinger/BufferStateLayer.cpp
@@ -605,13 +605,14 @@
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 6b422ea..db0ebfc 100644
--- a/services/surfaceflinger/BufferStateLayer.h
+++ b/services/surfaceflinger/BufferStateLayer.h
@@ -156,7 +156,7 @@
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 ce97155..34a9f39 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -922,7 +922,9 @@
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 ad91183..e27b8dd 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3315,10 +3315,10 @@
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 @@
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::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 @@
if (!(s.what & layer_state_t::eAcquireFenceChanged)) {
continue;
}
+
if (s.acquireFence && s.acquireFence->getStatus() == Fence::Status::Unsignaled) {
ready = false;
}
@@ -3405,7 +3407,10 @@
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 @@
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 6434ca2..6d3b4ef 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -800,7 +800,8 @@
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);