SurfaceFlinger: check transaction timings even if it doesn't have a buffer
Bufferless transactions should also not be applied if they are considered
early.
Bug: 181978893
Test: launch an app and observe systraces
Test: adb shell /data/nativetest64/SurfaceFlinger_test/SurfaceFlinger_test
Change-Id: Ie6cff82f8316c1a299d4c9f151eaf637cb60e154
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index e27b8dd..548d45d 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3389,18 +3389,16 @@
for (const ComposerState& state : states) {
const layer_state_t& s = state.state;
- if (!(s.what & layer_state_t::eAcquireFenceChanged)) {
- continue;
- }
-
- if (s.acquireFence && s.acquireFence->getStatus() == Fence::Status::Unsignaled) {
+ const bool acquireFenceChanged = (s.what & layer_state_t::eAcquireFenceChanged);
+ if (acquireFenceChanged && s.acquireFence &&
+ s.acquireFence->getStatus() == Fence::Status::Unsignaled) {
ready = false;
}
sp<Layer> layer = nullptr;
if (s.surface) {
layer = fromHandleLocked(s.surface).promote();
- } else {
+ } else if (acquireFenceChanged) {
ALOGW("Transaction with buffer, but no Layer?");
continue;
}
@@ -3420,11 +3418,14 @@
ready = false;
}
- // 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;
+ if (acquireFenceChanged) {
+ // 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;
+ }
+ pendingBuffers.insert(s.surface);
}
pendingBuffers.insert(s.surface);
}
diff --git a/services/surfaceflinger/tests/LayerCallback_test.cpp b/services/surfaceflinger/tests/LayerCallback_test.cpp
index 6d28e62..aa1cce2 100644
--- a/services/surfaceflinger/tests/LayerCallback_test.cpp
+++ b/services/surfaceflinger/tests/LayerCallback_test.cpp
@@ -18,6 +18,10 @@
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wconversion"
+#include <sys/epoll.h>
+
+#include <gui/DisplayEventReceiver.h>
+
#include "LayerTransactionTest.h"
#include "utils/CallbackUtils.h"
@@ -30,6 +34,24 @@
class LayerCallbackTest : public LayerTransactionTest {
public:
+ void SetUp() override {
+ LayerTransactionTest::SetUp();
+
+ EXPECT_EQ(NO_ERROR, mDisplayEventReceiver.initCheck());
+
+ mEpollFd = epoll_create1(EPOLL_CLOEXEC);
+ EXPECT_GT(mEpollFd, 1);
+
+ epoll_event event;
+ event.events = EPOLLIN;
+ EXPECT_EQ(0, epoll_ctl(mEpollFd, EPOLL_CTL_ADD, mDisplayEventReceiver.getFd(), &event));
+ }
+
+ void TearDown() override {
+ close(mEpollFd);
+ LayerTransactionTest::TearDown();
+ }
+
virtual sp<SurfaceControl> createBufferStateLayer() {
return createLayer(mClient, "test", 0, 0, ISurfaceComposerClient::eFXSurfaceBufferState);
}
@@ -82,6 +104,35 @@
ASSERT_NO_FATAL_FAILURE(helper.verifyFinalState());
}
}
+
+ DisplayEventReceiver mDisplayEventReceiver;
+ int mEpollFd;
+
+ struct Vsync {
+ int64_t vsyncId = FrameTimelineInfo::INVALID_VSYNC_ID;
+ nsecs_t expectedPresentTime = std::numeric_limits<nsecs_t>::max();
+ };
+
+ Vsync waitForNextVsync() {
+ mDisplayEventReceiver.requestNextVsync();
+ epoll_event epollEvent;
+ Vsync vsync;
+ EXPECT_EQ(1, epoll_wait(mEpollFd, &epollEvent, 1, 1000))
+ << "Timeout waiting for vsync event";
+ DisplayEventReceiver::Event event;
+ while (mDisplayEventReceiver.getEvents(&event, 1) > 0) {
+ if (event.header.type != DisplayEventReceiver::DISPLAY_EVENT_VSYNC) {
+ continue;
+ }
+
+ vsync = {event.vsync.vsyncId, event.vsync.expectedVSyncTimestamp};
+ }
+
+ EXPECT_GE(vsync.vsyncId, 1);
+ EXPECT_GT(event.vsync.expectedVSyncTimestamp, systemTime());
+
+ return vsync;
+ }
};
TEST_F(LayerCallbackTest, BufferColor) {
@@ -873,6 +924,29 @@
expected.addExpectedPresentTime(systemTime());
EXPECT_NO_FATAL_FAILURE(waitForCallback(callback, expected, true));
}
+
+TEST_F(LayerCallbackTest, ExpectedPresentTime) {
+ sp<SurfaceControl> layer;
+ ASSERT_NO_FATAL_FAILURE(layer = createBufferStateLayer());
+
+ Transaction transaction;
+ CallbackHelper callback;
+ int err = fillTransaction(transaction, &callback, layer);
+ if (err) {
+ GTEST_SUCCEED() << "test not supported";
+ return;
+ }
+
+ const Vsync vsync = waitForNextVsync();
+ transaction.setFrameTimelineInfo({vsync.vsyncId, 0});
+ transaction.apply();
+
+ ExpectedResult expected;
+ expected.addSurface(ExpectedResult::Transaction::PRESENTED, layer);
+ expected.addExpectedPresentTimeForVsyncId(vsync.expectedPresentTime);
+ EXPECT_NO_FATAL_FAILURE(waitForCallback(callback, expected, true));
+}
+
} // namespace android
// TODO(b/129481165): remove the #pragma below and fix conversion issues
diff --git a/services/surfaceflinger/tests/utils/CallbackUtils.h b/services/surfaceflinger/tests/utils/CallbackUtils.h
index 1318deb..459b35c 100644
--- a/services/surfaceflinger/tests/utils/CallbackUtils.h
+++ b/services/surfaceflinger/tests/utils/CallbackUtils.h
@@ -81,6 +81,10 @@
mExpectedPresentTime = expectedPresentTime;
}
+ void addExpectedPresentTimeForVsyncId(nsecs_t expectedPresentTime) {
+ mExpectedPresentTimeForVsyncId = expectedPresentTime;
+ }
+
void verifyCallbackData(const CallbackData& callbackData) const {
const auto& [latchTime, presentFence, surfaceControlStats] = callbackData;
if (mTransactionResult == ExpectedResult::Transaction::PRESENTED) {
@@ -93,6 +97,11 @@
// misses vsync and we have to wait another 33.3ms
ASSERT_LE(presentFence->getSignalTime(),
mExpectedPresentTime + nsecs_t(66.666666 * 1e6));
+ } else if (mExpectedPresentTimeForVsyncId >= 0) {
+ ASSERT_EQ(presentFence->wait(3000), NO_ERROR);
+ // We give 4ms for prediction error
+ ASSERT_GE(presentFence->getSignalTime(),
+ mExpectedPresentTimeForVsyncId - 4'000'000);
}
} else {
ASSERT_EQ(presentFence, nullptr) << "transaction shouldn't have been presented";
@@ -151,6 +160,7 @@
};
ExpectedResult::Transaction mTransactionResult = ExpectedResult::Transaction::NOT_PRESENTED;
nsecs_t mExpectedPresentTime = -1;
+ nsecs_t mExpectedPresentTimeForVsyncId = -1;
std::unordered_map<sp<SurfaceControl>, ExpectedSurfaceResult, SCHash> mExpectedSurfaceResults;
};