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
diff --git a/services/surfaceflinger/BufferLayer.cpp b/services/surfaceflinger/BufferLayer.cpp
index eac3d95..b3f9792 100644
--- a/services/surfaceflinger/BufferLayer.cpp
+++ b/services/surfaceflinger/BufferLayer.cpp
@@ -424,24 +424,6 @@
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 5fed79f..b8d3f12 100644
--- a/services/surfaceflinger/BufferLayer.h
+++ b/services/surfaceflinger/BufferLayer.h
@@ -189,10 +189,6 @@
// 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 @@
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 63dd25f..51e85c4 100644
--- a/services/surfaceflinger/BufferQueueLayer.cpp
+++ b/services/surfaceflinger/BufferQueueLayer.cpp
@@ -215,21 +215,6 @@
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 3a34b95..b3b7948 100644
--- a/services/surfaceflinger/BufferQueueLayer.h
+++ b/services/surfaceflinger/BufferQueueLayer.h
@@ -117,8 +117,6 @@
// 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 89dfb6f..00a6d66 100644
--- a/services/surfaceflinger/BufferStateLayer.cpp
+++ b/services/surfaceflinger/BufferStateLayer.cpp
@@ -644,16 +644,6 @@
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 036e8d2..7a3da6f 100644
--- a/services/surfaceflinger/BufferStateLayer.h
+++ b/services/surfaceflinger/BufferStateLayer.h
@@ -156,8 +156,6 @@
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 85ff479..1c5d6ec 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -930,10 +930,6 @@
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 61cc8a2..e580a97 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3379,6 +3379,28 @@
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 @@
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 @@
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 @@
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 @@
: 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 a5b06df..a7cdcd1 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -844,6 +844,7 @@
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 f010786..fa1a5ed 100644
--- a/services/surfaceflinger/tests/LayerState_test.cpp
+++ b/services/surfaceflinger/tests/LayerState_test.cpp
@@ -114,34 +114,5 @@
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