Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 29d16cb5 authored by Ady Abraham's avatar Ady Abraham
Browse files

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
parent 43752eba
Loading
Loading
Loading
Loading
+12 −11
Original line number Diff line number Diff line
@@ -3389,18 +3389,16 @@ bool SurfaceFlinger::transactionIsReadyToBeApplied(

    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,6 +3418,7 @@ bool SurfaceFlinger::transactionIsReadyToBeApplied(
            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();
@@ -3428,6 +3427,8 @@ bool SurfaceFlinger::transactionIsReadyToBeApplied(
            }
            pendingBuffers.insert(s.surface);
        }
        pendingBuffers.insert(s.surface);
    }
    return ready;
}

+74 −0
Original line number Diff line number Diff line
@@ -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 @@ using android::hardware::graphics::common::V1_1::BufferUsage;

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 @@ public:
            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 @@ TEST_F(LayerCallbackTest, DesiredPresentTime_Past) {
    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
+10 −0
Original line number Diff line number Diff line
@@ -81,6 +81,10 @@ public:
        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 @@ public:
                // 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 @@ private:
    };
    ExpectedResult::Transaction mTransactionResult = ExpectedResult::Transaction::NOT_PRESENTED;
    nsecs_t mExpectedPresentTime = -1;
    nsecs_t mExpectedPresentTimeForVsyncId = -1;
    std::unordered_map<sp<SurfaceControl>, ExpectedSurfaceResult, SCHash> mExpectedSurfaceResults;
};