From 0884711a6034150851dd8088302eaa0df9e10281 Mon Sep 17 00:00:00 2001 From: Rachel Lee Date: Thu, 16 Feb 2023 11:07:48 -0800 Subject: Assert kFrameTimelinesLength for sync. Want to keep the value in sync with java definition. Test: change kFrameTimelinesLength and m to observe assert Bug: 258694738 Change-Id: Id6f53a809015ded83fc15fa8a99c11573559e3b0 --- libs/gui/VsyncEventData.cpp | 3 +++ 1 file changed, 3 insertions(+) (limited to 'libs/gui/VsyncEventData.cpp') diff --git a/libs/gui/VsyncEventData.cpp b/libs/gui/VsyncEventData.cpp index 23f0921e99..76c60c23c4 100644 --- a/libs/gui/VsyncEventData.cpp +++ b/libs/gui/VsyncEventData.cpp @@ -23,6 +23,9 @@ namespace android::gui { +static_assert(VsyncEventData::kFrameTimelinesLength == 7, + "Must update value in DisplayEventReceiver.java#FRAME_TIMELINES_LENGTH (and here)"); + int64_t VsyncEventData::preferredVsyncId() const { return frameTimelines[preferredFrameTimelineIndex].vsyncId; } -- cgit v1.2.3-59-g8ed1b From 0655a9157bc7ab4771202d2d4e206f91d5352481 Mon Sep 17 00:00:00 2001 From: Rachel Lee Date: Thu, 20 Apr 2023 19:54:18 -0700 Subject: Fix Choreographer affecting ASurfaceControlTest Fixes `testSurfaceTransaction_setFrameTimeline_notPreferredIndex` case on devices that do not have high refresh rates (e.g. max 60Hz). This occurred because the transaction-ready logic in SF does not consider transactions to be too early if the expected presentation time is over 100ms in the future, so the frame would just be deemed ready and presented asap. Thus, no longer provide frame timelines that are far into the future, which are not useful as well. Test: atest ASurfaceControlTest Test: atest ChoreographerTest Test: atest ChoreographerNativeTest Test: atest DisplayEventStructLayoutTest Test: atest ParcelableVsyncEventData Test: atest libsurfacefligner_unittest Fixes: 270612751 Change-Id: Ic05717bc153a9b07409b8d7912a1c40e1e31a57e --- libs/gui/VsyncEventData.cpp | 15 ++- libs/gui/include/gui/VsyncEventData.h | 5 +- libs/gui/tests/DisplayEventStructLayout_test.cpp | 1 + libs/gui/tests/VsyncEventData_test.cpp | 2 + libs/nativedisplay/AChoreographer.cpp | 2 +- services/surfaceflinger/Scheduler/EventThread.cpp | 38 +++--- .../Scheduler/include/scheduler/VsyncConfig.h | 6 + services/surfaceflinger/SurfaceFlinger.cpp | 6 +- .../tests/DisplayEventReceiver_test.cpp | 7 +- .../tests/unittests/EventThreadTest.cpp | 132 +++++++++++++++++---- 10 files changed, 166 insertions(+), 48 deletions(-) (limited to 'libs/gui/VsyncEventData.cpp') diff --git a/libs/gui/VsyncEventData.cpp b/libs/gui/VsyncEventData.cpp index 76c60c23c4..91fc9c0ffe 100644 --- a/libs/gui/VsyncEventData.cpp +++ b/libs/gui/VsyncEventData.cpp @@ -46,11 +46,15 @@ status_t ParcelableVsyncEventData::readFromParcel(const Parcel* parcel) { SAFE_PARCEL(parcel->readInt64, &vsync.frameInterval); - uint64_t uintPreferredFrameTimelineIndex; - SAFE_PARCEL(parcel->readUint64, &uintPreferredFrameTimelineIndex); + uint32_t uintPreferredFrameTimelineIndex; + SAFE_PARCEL(parcel->readUint32, &uintPreferredFrameTimelineIndex); vsync.preferredFrameTimelineIndex = static_cast(uintPreferredFrameTimelineIndex); - for (int i = 0; i < VsyncEventData::kFrameTimelinesLength; i++) { + uint32_t uintFrameTimelinesLength; + SAFE_PARCEL(parcel->readUint32, &uintFrameTimelinesLength); + vsync.frameTimelinesLength = static_cast(uintFrameTimelinesLength); + + for (size_t i = 0; i < vsync.frameTimelinesLength; i++) { SAFE_PARCEL(parcel->readInt64, &vsync.frameTimelines[i].vsyncId); SAFE_PARCEL(parcel->readInt64, &vsync.frameTimelines[i].deadlineTimestamp); SAFE_PARCEL(parcel->readInt64, &vsync.frameTimelines[i].expectedPresentationTime); @@ -60,8 +64,9 @@ status_t ParcelableVsyncEventData::readFromParcel(const Parcel* parcel) { } status_t ParcelableVsyncEventData::writeToParcel(Parcel* parcel) const { SAFE_PARCEL(parcel->writeInt64, vsync.frameInterval); - SAFE_PARCEL(parcel->writeUint64, vsync.preferredFrameTimelineIndex); - for (int i = 0; i < VsyncEventData::kFrameTimelinesLength; i++) { + SAFE_PARCEL(parcel->writeUint32, vsync.preferredFrameTimelineIndex); + SAFE_PARCEL(parcel->writeUint32, vsync.frameTimelinesLength); + for (size_t i = 0; i < vsync.frameTimelinesLength; i++) { SAFE_PARCEL(parcel->writeInt64, vsync.frameTimelines[i].vsyncId); SAFE_PARCEL(parcel->writeInt64, vsync.frameTimelines[i].deadlineTimestamp); SAFE_PARCEL(parcel->writeInt64, vsync.frameTimelines[i].expectedPresentationTime); diff --git a/libs/gui/include/gui/VsyncEventData.h b/libs/gui/include/gui/VsyncEventData.h index dfdae214d2..9ddb7cd82b 100644 --- a/libs/gui/include/gui/VsyncEventData.h +++ b/libs/gui/include/gui/VsyncEventData.h @@ -24,7 +24,7 @@ namespace android::gui { // Plain Old Data (POD) vsync data structure. For example, it can be easily used in the // DisplayEventReceiver::Event union. struct VsyncEventData { - // Max amount of frame timelines is arbitrarily set to be reasonable. + // Max capacity of frame timelines is arbitrarily set to be reasonable. static constexpr int64_t kFrameTimelinesLength = 7; // The current frame interval in ns when this frame was scheduled. @@ -33,6 +33,9 @@ struct VsyncEventData { // Index into the frameTimelines that represents the platform's preferred frame timeline. uint32_t preferredFrameTimelineIndex; + // Size of frame timelines provided by the platform; max is kFrameTimelinesLength. + uint32_t frameTimelinesLength; + struct alignas(8) FrameTimeline { // The Vsync Id corresponsing to this vsync event. This will be used to // populate ISurfaceComposer::setFrameTimelineVsync and diff --git a/libs/gui/tests/DisplayEventStructLayout_test.cpp b/libs/gui/tests/DisplayEventStructLayout_test.cpp index da88463d63..b7b4ab605a 100644 --- a/libs/gui/tests/DisplayEventStructLayout_test.cpp +++ b/libs/gui/tests/DisplayEventStructLayout_test.cpp @@ -35,6 +35,7 @@ TEST(DisplayEventStructLayoutTest, TestEventAlignment) { CHECK_OFFSET(DisplayEventReceiver::Event::VSync, count, 0); CHECK_OFFSET(DisplayEventReceiver::Event::VSync, vsyncData.frameInterval, 8); CHECK_OFFSET(DisplayEventReceiver::Event::VSync, vsyncData.preferredFrameTimelineIndex, 16); + CHECK_OFFSET(DisplayEventReceiver::Event::VSync, vsyncData.frameTimelinesLength, 20); CHECK_OFFSET(DisplayEventReceiver::Event::VSync, vsyncData.frameTimelines, 24); CHECK_OFFSET(DisplayEventReceiver::Event::VSync, vsyncData.frameTimelines[0].vsyncId, 24); CHECK_OFFSET(DisplayEventReceiver::Event::VSync, vsyncData.frameTimelines[0].deadlineTimestamp, diff --git a/libs/gui/tests/VsyncEventData_test.cpp b/libs/gui/tests/VsyncEventData_test.cpp index f114522951..6e039ee6e7 100644 --- a/libs/gui/tests/VsyncEventData_test.cpp +++ b/libs/gui/tests/VsyncEventData_test.cpp @@ -36,6 +36,7 @@ TEST(ParcelableVsyncEventData, Parcelling) { FrameTimeline timeline1 = FrameTimeline{4, 5, 6}; data.vsync.frameTimelines[0] = timeline0; data.vsync.frameTimelines[1] = timeline1; + data.vsync.frameTimelinesLength = 2; Parcel p; data.writeToParcel(&p); @@ -45,6 +46,7 @@ TEST(ParcelableVsyncEventData, Parcelling) { data2.readFromParcel(&p); ASSERT_EQ(data.vsync.frameInterval, data2.vsync.frameInterval); ASSERT_EQ(data.vsync.preferredFrameTimelineIndex, data2.vsync.preferredFrameTimelineIndex); + ASSERT_EQ(data.vsync.frameTimelinesLength, data2.vsync.frameTimelinesLength); for (int i = 0; i < VsyncEventData::kFrameTimelinesLength; i++) { ASSERT_EQ(data.vsync.frameTimelines[i].vsyncId, data2.vsync.frameTimelines[i].vsyncId); ASSERT_EQ(data.vsync.frameTimelines[i].deadlineTimestamp, diff --git a/libs/nativedisplay/AChoreographer.cpp b/libs/nativedisplay/AChoreographer.cpp index 66a40f1278..80b0041923 100644 --- a/libs/nativedisplay/AChoreographer.cpp +++ b/libs/nativedisplay/AChoreographer.cpp @@ -197,7 +197,7 @@ size_t AChoreographerFrameCallbackData_getFrameTimelinesLength( AChoreographerFrameCallbackData_to_ChoreographerFrameCallbackDataImpl(data); LOG_ALWAYS_FATAL_IF(!frameCallbackData->choreographer->inCallback(), "Data is only valid in callback"); - return VsyncEventData::kFrameTimelinesLength; + return frameCallbackData->vsyncEventData.frameTimelinesLength; } size_t AChoreographerFrameCallbackData_getPreferredFrameTimelineIndex( const AChoreographerFrameCallbackData* data) { diff --git a/services/surfaceflinger/Scheduler/EventThread.cpp b/services/surfaceflinger/Scheduler/EventThread.cpp index 74665a70aa..f79688dfe0 100644 --- a/services/surfaceflinger/Scheduler/EventThread.cpp +++ b/services/surfaceflinger/Scheduler/EventThread.cpp @@ -42,6 +42,7 @@ #include #include +#include #include "DisplayHardware/DisplayMode.h" #include "FrameTimeline.h" #include "VSyncDispatch.h" @@ -597,25 +598,34 @@ void EventThread::generateFrameTimeline(VsyncEventData& outVsyncEventData, nsecs nsecs_t timestamp, nsecs_t preferredExpectedPresentationTime, nsecs_t preferredDeadlineTimestamp) const { + uint32_t currentIndex = 0; // Add 1 to ensure the preferredFrameTimelineIndex entry (when multiplier == 0) is included. - for (int64_t multiplier = -VsyncEventData::kFrameTimelinesLength + 1, currentIndex = 0; + for (int64_t multiplier = -VsyncEventData::kFrameTimelinesLength + 1; currentIndex < VsyncEventData::kFrameTimelinesLength; multiplier++) { nsecs_t deadlineTimestamp = preferredDeadlineTimestamp + multiplier * frameInterval; - // Valid possible frame timelines must have future values. - if (deadlineTimestamp > timestamp) { - if (multiplier == 0) { - outVsyncEventData.preferredFrameTimelineIndex = currentIndex; - } - nsecs_t expectedPresentationTime = - preferredExpectedPresentationTime + multiplier * frameInterval; - outVsyncEventData.frameTimelines[currentIndex] = - {.vsyncId = - generateToken(timestamp, deadlineTimestamp, expectedPresentationTime), - .deadlineTimestamp = deadlineTimestamp, - .expectedPresentationTime = expectedPresentationTime}; - currentIndex++; + // Valid possible frame timelines must have future values, so find a later frame timeline. + if (deadlineTimestamp <= timestamp) { + continue; + } + + nsecs_t expectedPresentationTime = + preferredExpectedPresentationTime + multiplier * frameInterval; + if (expectedPresentationTime >= preferredExpectedPresentationTime + + scheduler::VsyncConfig::kEarlyLatchMaxThreshold.count()) { + break; } + + if (multiplier == 0) { + outVsyncEventData.preferredFrameTimelineIndex = currentIndex; + } + + outVsyncEventData.frameTimelines[currentIndex] = + {.vsyncId = generateToken(timestamp, deadlineTimestamp, expectedPresentationTime), + .deadlineTimestamp = deadlineTimestamp, + .expectedPresentationTime = expectedPresentationTime}; + currentIndex++; } + outVsyncEventData.frameTimelinesLength = currentIndex; } void EventThread::dispatchEvent(const DisplayEventReceiver::Event& event, diff --git a/services/surfaceflinger/Scheduler/include/scheduler/VsyncConfig.h b/services/surfaceflinger/Scheduler/include/scheduler/VsyncConfig.h index 3b1985f56d..47d95a8a9a 100644 --- a/services/surfaceflinger/Scheduler/include/scheduler/VsyncConfig.h +++ b/services/surfaceflinger/Scheduler/include/scheduler/VsyncConfig.h @@ -22,6 +22,8 @@ namespace android::scheduler { +using namespace std::chrono_literals; + // Phase offsets and work durations for SF and app deadlines from VSYNC. struct VsyncConfig { nsecs_t sfOffset; @@ -35,6 +37,10 @@ struct VsyncConfig { } bool operator!=(const VsyncConfig& other) const { return !(*this == other); } + + // The duration for which SF can delay a frame if it is considered early based on the + // VsyncConfig::appWorkDuration. + static constexpr std::chrono::nanoseconds kEarlyLatchMaxThreshold = 100ms; }; struct VsyncConfigSet { diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 48b41448a1..584b125f50 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -4395,10 +4395,8 @@ bool SurfaceFlinger::frameIsEarly(TimePoint expectedPresentTime, VsyncId vsyncId const auto predictedPresentTime = TimePoint::fromNs(prediction->presentTime); - // The duration for which SF can delay a frame if it is considered early based on the - // VsyncConfig::appWorkDuration. - if (constexpr std::chrono::nanoseconds kEarlyLatchMaxThreshold = 100ms; - std::chrono::abs(predictedPresentTime - expectedPresentTime) >= kEarlyLatchMaxThreshold) { + if (std::chrono::abs(predictedPresentTime - expectedPresentTime) >= + scheduler::VsyncConfig::kEarlyLatchMaxThreshold) { return false; } diff --git a/services/surfaceflinger/tests/DisplayEventReceiver_test.cpp b/services/surfaceflinger/tests/DisplayEventReceiver_test.cpp index 0df7e2fafa..761ac8c538 100644 --- a/services/surfaceflinger/tests/DisplayEventReceiver_test.cpp +++ b/services/surfaceflinger/tests/DisplayEventReceiver_test.cpp @@ -33,9 +33,14 @@ TEST_F(DisplayEventReceiverTest, getLatestVsyncEventData) { const VsyncEventData& vsyncEventData = parcelableVsyncEventData.vsync; EXPECT_NE(std::numeric_limits::max(), vsyncEventData.preferredFrameTimelineIndex); + EXPECT_GT(static_cast(vsyncEventData.frameTimelinesLength), 0) + << "Frame timelines length should be greater than 0"; + EXPECT_LE(static_cast(vsyncEventData.frameTimelinesLength), + VsyncEventData::kFrameTimelinesLength) + << "Frame timelines length should not exceed max capacity"; EXPECT_GT(vsyncEventData.frameTimelines[0].deadlineTimestamp, now) << "Deadline timestamp should be greater than frame time"; - for (size_t i = 0; i < VsyncEventData::kFrameTimelinesLength; i++) { + for (size_t i = 0; i < vsyncEventData.frameTimelinesLength; i++) { EXPECT_NE(gui::FrameTimelineInfo::INVALID_VSYNC_ID, vsyncEventData.frameTimelines[i].vsyncId); EXPECT_GT(vsyncEventData.frameTimelines[i].expectedPresentationTime, diff --git a/services/surfaceflinger/tests/unittests/EventThreadTest.cpp b/services/surfaceflinger/tests/unittests/EventThreadTest.cpp index f1cdca3ee1..6debbaae2d 100644 --- a/services/surfaceflinger/tests/unittests/EventThreadTest.cpp +++ b/services/surfaceflinger/tests/unittests/EventThreadTest.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include #include "AsyncCallRecorder.h" @@ -77,7 +78,7 @@ protected: EventThreadTest(); ~EventThreadTest() override; - void createThread(); + void setupEventThread(std::chrono::nanoseconds vsyncPeriod); sp createConnection(ConnectionEventRecorder& recorder, EventRegistrationFlags eventRegistration = {}, uid_t ownerUid = mConnectionUid); @@ -90,8 +91,9 @@ protected: nsecs_t expectedTimestamp, unsigned expectedCount); void expectVsyncEventReceivedByConnection(nsecs_t expectedTimestamp, unsigned expectedCount); void expectVsyncEventFrameTimelinesCorrect( - nsecs_t expectedTimestamp, - /*VSyncSource::VSyncData*/ gui::VsyncEventData::FrameTimeline preferredVsyncData); + nsecs_t expectedTimestamp, gui::VsyncEventData::FrameTimeline preferredVsyncData); + void expectVsyncEventDataFrameTimelinesValidLength(VsyncEventData vsyncEventData, + std::chrono::nanoseconds vsyncPeriod); void expectHotplugEventReceivedByConnection(PhysicalDisplayId expectedDisplayId, bool expectedConnected); void expectConfigChangedEventReceivedByConnection(PhysicalDisplayId expectedDisplayId, @@ -154,19 +156,6 @@ EventThreadTest::EventThreadTest() { .WillRepeatedly(Invoke(mVSyncCallbackUpdateRecorder.getInvocable())); EXPECT_CALL(mockDispatch, unregisterCallback(_)) .WillRepeatedly(Invoke(mVSyncCallbackUnregisterRecorder.getInvocable())); - - createThread(); - mConnection = - createConnection(mConnectionEventCallRecorder, - gui::ISurfaceComposer::EventRegistration::modeChanged | - gui::ISurfaceComposer::EventRegistration::frameRateOverride); - mThrottledConnection = createConnection(mThrottledConnectionEventCallRecorder, - gui::ISurfaceComposer::EventRegistration::modeChanged, - mThrottledConnectionUid); - - // A display must be connected for VSYNC events to be delivered. - mThread->onHotplugReceived(INTERNAL_DISPLAY_ID, true); - expectHotplugEventReceivedByConnection(INTERNAL_DISPLAY_ID, true); } EventThreadTest::~EventThreadTest() { @@ -179,14 +168,12 @@ EventThreadTest::~EventThreadTest() { EXPECT_TRUE(mVSyncCallbackUnregisterRecorder.waitForCall().has_value()); } -void EventThreadTest::createThread() { +void EventThreadTest::setupEventThread(std::chrono::nanoseconds vsyncPeriod) { const auto throttleVsync = [&](nsecs_t expectedVsyncTimestamp, uid_t uid) { mThrottleVsyncCallRecorder.getInvocable()(expectedVsyncTimestamp, uid); return (uid == mThrottledConnectionUid); }; - const auto getVsyncPeriod = [](uid_t uid) { - return VSYNC_PERIOD.count(); - }; + const auto getVsyncPeriod = [vsyncPeriod](uid_t uid) { return vsyncPeriod.count(); }; mTokenManager = std::make_unique(); mThread = std::make_unique("EventThreadTest", mVsyncSchedule, @@ -195,6 +182,18 @@ void EventThreadTest::createThread() { // EventThread should register itself as VSyncSource callback. EXPECT_TRUE(mVSyncCallbackRegisterRecorder.waitForCall().has_value()); + + mConnection = + createConnection(mConnectionEventCallRecorder, + gui::ISurfaceComposer::EventRegistration::modeChanged | + gui::ISurfaceComposer::EventRegistration::frameRateOverride); + mThrottledConnection = createConnection(mThrottledConnectionEventCallRecorder, + gui::ISurfaceComposer::EventRegistration::modeChanged, + mThrottledConnectionUid); + + // A display must be connected for VSYNC events to be delivered. + mThread->onHotplugReceived(INTERNAL_DISPLAY_ID, true); + expectHotplugEventReceivedByConnection(INTERNAL_DISPLAY_ID, true); } sp EventThreadTest::createConnection( @@ -259,7 +258,7 @@ void EventThreadTest::expectVsyncEventFrameTimelinesCorrect( ASSERT_TRUE(args.has_value()) << " did not receive an event for timestamp " << expectedTimestamp; const auto& event = std::get<0>(args.value()); - for (int i = 0; i < VsyncEventData::kFrameTimelinesLength; i++) { + for (int i = 0; i < event.vsync.vsyncData.frameTimelinesLength; i++) { auto prediction = mTokenManager->getPredictionsForToken( event.vsync.vsyncData.frameTimelines[i].vsyncId); EXPECT_TRUE(prediction.has_value()); @@ -293,6 +292,21 @@ void EventThreadTest::expectVsyncEventFrameTimelinesCorrect( } } +void EventThreadTest::expectVsyncEventDataFrameTimelinesValidLength( + VsyncEventData vsyncEventData, std::chrono::nanoseconds vsyncPeriod) { + float nonPreferredTimelinesAmount = + scheduler::VsyncConfig::kEarlyLatchMaxThreshold / vsyncPeriod; + EXPECT_LE(vsyncEventData.frameTimelinesLength, nonPreferredTimelinesAmount + 1) + << "Amount of non-preferred frame timelines too many;" + << " expected presentation time will be over threshold"; + EXPECT_LT(nonPreferredTimelinesAmount, VsyncEventData::kFrameTimelinesLength) + << "Amount of non-preferred frame timelines should be less than max capacity"; + EXPECT_GT(static_cast(vsyncEventData.frameTimelinesLength), 0) + << "Frame timelines length should be greater than 0"; + EXPECT_LT(vsyncEventData.preferredFrameTimelineIndex, vsyncEventData.frameTimelinesLength) + << "Preferred frame timeline index should be less than frame timelines length"; +} + void EventThreadTest::expectHotplugEventReceivedByConnection(PhysicalDisplayId expectedDisplayId, bool expectedConnected) { auto args = mConnectionEventCallRecorder.waitForCall(); @@ -343,6 +357,8 @@ using namespace testing; */ TEST_F(EventThreadTest, canCreateAndDestroyThreadWithNoEventsSent) { + setupEventThread(VSYNC_PERIOD); + EXPECT_FALSE(mVSyncCallbackRegisterRecorder.waitForCall(0us).has_value()); EXPECT_FALSE(mVSyncCallbackScheduleRecorder.waitForCall(0us).has_value()); EXPECT_FALSE(mVSyncCallbackUpdateRecorder.waitForCall(0us).has_value()); @@ -352,6 +368,8 @@ TEST_F(EventThreadTest, canCreateAndDestroyThreadWithNoEventsSent) { } TEST_F(EventThreadTest, vsyncRequestIsIgnoredIfDisplayIsDisconnected) { + setupEventThread(VSYNC_PERIOD); + mThread->onHotplugReceived(INTERNAL_DISPLAY_ID, false); expectHotplugEventReceivedByConnection(INTERNAL_DISPLAY_ID, false); @@ -363,6 +381,8 @@ TEST_F(EventThreadTest, vsyncRequestIsIgnoredIfDisplayIsDisconnected) { } TEST_F(EventThreadTest, requestNextVsyncPostsASingleVSyncEventToTheConnection) { + setupEventThread(VSYNC_PERIOD); + // Signal that we want the next vsync event to be posted to the connection mThread->requestNextVsync(mConnection); @@ -394,6 +414,8 @@ TEST_F(EventThreadTest, requestNextVsyncPostsASingleVSyncEventToTheConnection) { } TEST_F(EventThreadTest, requestNextVsyncEventFrameTimelinesCorrect) { + setupEventThread(VSYNC_PERIOD); + // Signal that we want the next vsync event to be posted to the connection mThread->requestNextVsync(mConnection); @@ -405,7 +427,34 @@ TEST_F(EventThreadTest, requestNextVsyncEventFrameTimelinesCorrect) { expectVsyncEventFrameTimelinesCorrect(123, {-1, 789, 456}); } +TEST_F(EventThreadTest, requestNextVsyncEventFrameTimelinesValidLength) { + // The VsyncEventData should not have kFrameTimelinesLength amount of valid frame timelines, due + // to longer vsync period and kEarlyLatchMaxThreshold. + // Use length-2 to avoid decimal truncation (e.g. 60Hz has 16.6... ms vsync period). + std::chrono::nanoseconds vsyncPeriod(scheduler::VsyncConfig::kEarlyLatchMaxThreshold / + (VsyncEventData::kFrameTimelinesLength - 2)); + setupEventThread(vsyncPeriod); + + // Signal that we want the next vsync event to be posted to the connection + mThread->requestNextVsync(mConnection); + + expectVSyncCallbackScheduleReceived(true); + + // Use the received callback to signal a vsync event. + // The throttler should receive the event, as well as the connection. + nsecs_t expectedTimestamp = 123; + onVSyncEvent(expectedTimestamp, 456, 789); + + auto args = mConnectionEventCallRecorder.waitForCall(); + ASSERT_TRUE(args.has_value()) << " did not receive an event for timestamp " + << expectedTimestamp; + const VsyncEventData vsyncEventData = std::get<0>(args.value()).vsync.vsyncData; + expectVsyncEventDataFrameTimelinesValidLength(vsyncEventData, vsyncPeriod); +} + TEST_F(EventThreadTest, getLatestVsyncEventData) { + setupEventThread(VSYNC_PERIOD); + const nsecs_t now = systemTime(); const nsecs_t preferredExpectedPresentationTime = now + 20000000; const nsecs_t preferredDeadline = preferredExpectedPresentationTime - kReadyDuration.count(); @@ -420,9 +469,10 @@ TEST_F(EventThreadTest, getLatestVsyncEventData) { // Check EventThread immediately requested a resync. EXPECT_TRUE(mResyncCallRecorder.waitForCall().has_value()); + expectVsyncEventDataFrameTimelinesValidLength(vsyncEventData, VSYNC_PERIOD); EXPECT_GT(vsyncEventData.frameTimelines[0].deadlineTimestamp, now) << "Deadline timestamp should be greater than frame time"; - for (size_t i = 0; i < VsyncEventData::kFrameTimelinesLength; i++) { + for (size_t i = 0; i < vsyncEventData.frameTimelinesLength; i++) { auto prediction = mTokenManager->getPredictionsForToken(vsyncEventData.frameTimelines[i].vsyncId); EXPECT_TRUE(prediction.has_value()); @@ -458,6 +508,8 @@ TEST_F(EventThreadTest, getLatestVsyncEventData) { } TEST_F(EventThreadTest, setVsyncRateZeroPostsNoVSyncEventsToThatConnection) { + setupEventThread(VSYNC_PERIOD); + // Create a first connection, register it, and request a vsync rate of zero. ConnectionEventRecorder firstConnectionEventRecorder{0}; sp firstConnection = createConnection(firstConnectionEventRecorder); @@ -485,6 +537,8 @@ TEST_F(EventThreadTest, setVsyncRateZeroPostsNoVSyncEventsToThatConnection) { } TEST_F(EventThreadTest, setVsyncRateOnePostsAllEventsToThatConnection) { + setupEventThread(VSYNC_PERIOD); + mThread->setVsyncRate(1, mConnection); // EventThread should enable vsync callbacks. @@ -508,6 +562,8 @@ TEST_F(EventThreadTest, setVsyncRateOnePostsAllEventsToThatConnection) { } TEST_F(EventThreadTest, setVsyncRateTwoPostsEveryOtherEventToThatConnection) { + setupEventThread(VSYNC_PERIOD); + mThread->setVsyncRate(2, mConnection); // EventThread should enable vsync callbacks. @@ -534,6 +590,8 @@ TEST_F(EventThreadTest, setVsyncRateTwoPostsEveryOtherEventToThatConnection) { } TEST_F(EventThreadTest, connectionsRemovedIfInstanceDestroyed) { + setupEventThread(VSYNC_PERIOD); + mThread->setVsyncRate(1, mConnection); // EventThread should enable vsync callbacks. @@ -551,6 +609,8 @@ TEST_F(EventThreadTest, connectionsRemovedIfInstanceDestroyed) { } TEST_F(EventThreadTest, connectionsRemovedIfEventDeliveryError) { + setupEventThread(VSYNC_PERIOD); + ConnectionEventRecorder errorConnectionEventRecorder{NO_MEMORY}; sp errorConnection = createConnection(errorConnectionEventRecorder); mThread->setVsyncRate(1, errorConnection); @@ -575,6 +635,8 @@ TEST_F(EventThreadTest, connectionsRemovedIfEventDeliveryError) { } TEST_F(EventThreadTest, tracksEventConnections) { + setupEventThread(VSYNC_PERIOD); + EXPECT_EQ(2, mThread->getEventThreadConnectionCount()); ConnectionEventRecorder errorConnectionEventRecorder{NO_MEMORY}; sp errorConnection = createConnection(errorConnectionEventRecorder); @@ -598,6 +660,8 @@ TEST_F(EventThreadTest, tracksEventConnections) { } TEST_F(EventThreadTest, eventsDroppedIfNonfatalEventDeliveryError) { + setupEventThread(VSYNC_PERIOD); + ConnectionEventRecorder errorConnectionEventRecorder{WOULD_BLOCK}; sp errorConnection = createConnection(errorConnectionEventRecorder); mThread->setVsyncRate(1, errorConnection); @@ -622,31 +686,43 @@ TEST_F(EventThreadTest, eventsDroppedIfNonfatalEventDeliveryError) { } TEST_F(EventThreadTest, setPhaseOffsetForwardsToVSyncSource) { + setupEventThread(VSYNC_PERIOD); + mThread->setDuration(321ns, 456ns); expectVSyncSetDurationCallReceived(321ns, 456ns); } TEST_F(EventThreadTest, postHotplugInternalDisconnect) { + setupEventThread(VSYNC_PERIOD); + mThread->onHotplugReceived(INTERNAL_DISPLAY_ID, false); expectHotplugEventReceivedByConnection(INTERNAL_DISPLAY_ID, false); } TEST_F(EventThreadTest, postHotplugInternalConnect) { + setupEventThread(VSYNC_PERIOD); + mThread->onHotplugReceived(INTERNAL_DISPLAY_ID, true); expectHotplugEventReceivedByConnection(INTERNAL_DISPLAY_ID, true); } TEST_F(EventThreadTest, postHotplugExternalDisconnect) { + setupEventThread(VSYNC_PERIOD); + mThread->onHotplugReceived(EXTERNAL_DISPLAY_ID, false); expectHotplugEventReceivedByConnection(EXTERNAL_DISPLAY_ID, false); } TEST_F(EventThreadTest, postHotplugExternalConnect) { + setupEventThread(VSYNC_PERIOD); + mThread->onHotplugReceived(EXTERNAL_DISPLAY_ID, true); expectHotplugEventReceivedByConnection(EXTERNAL_DISPLAY_ID, true); } TEST_F(EventThreadTest, postConfigChangedPrimary) { + setupEventThread(VSYNC_PERIOD); + const auto mode = DisplayMode::Builder(hal::HWConfigId(0)) .setPhysicalDisplayId(INTERNAL_DISPLAY_ID) .setId(DisplayModeId(7)) @@ -659,6 +735,8 @@ TEST_F(EventThreadTest, postConfigChangedPrimary) { } TEST_F(EventThreadTest, postConfigChangedExternal) { + setupEventThread(VSYNC_PERIOD); + const auto mode = DisplayMode::Builder(hal::HWConfigId(0)) .setPhysicalDisplayId(EXTERNAL_DISPLAY_ID) .setId(DisplayModeId(5)) @@ -671,6 +749,8 @@ TEST_F(EventThreadTest, postConfigChangedExternal) { } TEST_F(EventThreadTest, postConfigChangedPrimary64bit) { + setupEventThread(VSYNC_PERIOD); + const auto mode = DisplayMode::Builder(hal::HWConfigId(0)) .setPhysicalDisplayId(DISPLAY_ID_64BIT) .setId(DisplayModeId(7)) @@ -682,6 +762,8 @@ TEST_F(EventThreadTest, postConfigChangedPrimary64bit) { } TEST_F(EventThreadTest, suppressConfigChanged) { + setupEventThread(VSYNC_PERIOD); + ConnectionEventRecorder suppressConnectionEventRecorder{0}; sp suppressConnection = createConnection(suppressConnectionEventRecorder); @@ -701,6 +783,8 @@ TEST_F(EventThreadTest, suppressConfigChanged) { } TEST_F(EventThreadTest, postUidFrameRateMapping) { + setupEventThread(VSYNC_PERIOD); + const std::vector overrides = { {.uid = 1, .frameRateHz = 20}, {.uid = 3, .frameRateHz = 40}, @@ -712,6 +796,8 @@ TEST_F(EventThreadTest, postUidFrameRateMapping) { } TEST_F(EventThreadTest, suppressUidFrameRateMapping) { + setupEventThread(VSYNC_PERIOD); + const std::vector overrides = { {.uid = 1, .frameRateHz = 20}, {.uid = 3, .frameRateHz = 40}, @@ -730,6 +816,8 @@ TEST_F(EventThreadTest, suppressUidFrameRateMapping) { } TEST_F(EventThreadTest, requestNextVsyncWithThrottleVsyncDoesntPostVSync) { + setupEventThread(VSYNC_PERIOD); + // Signal that we want the next vsync event to be posted to the throttled connection mThread->requestNextVsync(mThrottledConnection); -- cgit v1.2.3-59-g8ed1b From 40aef42d44e1f0465b8dc17e3ecd8a1aee074236 Mon Sep 17 00:00:00 2001 From: Rachel Lee Date: Tue, 25 Apr 2023 14:35:47 -0700 Subject: Rename const to kFrameTimelinesCapacity From `kFrameTimelinesLength`. Bug: 270612751 Test: build Change-Id: I83e61a19254ab4a63479a2004124cfd2c082da35 --- libs/gui/VsyncEventData.cpp | 4 ++-- libs/gui/fuzzer/libgui_displayEvent_fuzzer.cpp | 2 +- libs/gui/include/gui/VsyncEventData.h | 6 +++--- libs/gui/tests/DisplayEventStructLayout_test.cpp | 8 ++++---- libs/gui/tests/VsyncEventData_test.cpp | 2 +- libs/nativedisplay/AChoreographer.cpp | 6 +++--- services/surfaceflinger/Scheduler/EventThread.cpp | 4 ++-- services/surfaceflinger/tests/DisplayEventReceiver_test.cpp | 2 +- services/surfaceflinger/tests/unittests/EventThreadTest.cpp | 10 +++++----- 9 files changed, 22 insertions(+), 22 deletions(-) (limited to 'libs/gui/VsyncEventData.cpp') diff --git a/libs/gui/VsyncEventData.cpp b/libs/gui/VsyncEventData.cpp index 91fc9c0ffe..8e00c2fe32 100644 --- a/libs/gui/VsyncEventData.cpp +++ b/libs/gui/VsyncEventData.cpp @@ -23,8 +23,8 @@ namespace android::gui { -static_assert(VsyncEventData::kFrameTimelinesLength == 7, - "Must update value in DisplayEventReceiver.java#FRAME_TIMELINES_LENGTH (and here)"); +static_assert(VsyncEventData::kFrameTimelinesCapacity == 7, + "Must update value in DisplayEventReceiver.java#FRAME_TIMELINES_CAPACITY (and here)"); int64_t VsyncEventData::preferredVsyncId() const { return frameTimelines[preferredFrameTimelineIndex].vsyncId; diff --git a/libs/gui/fuzzer/libgui_displayEvent_fuzzer.cpp b/libs/gui/fuzzer/libgui_displayEvent_fuzzer.cpp index 6d5ae49635..6e4f074825 100644 --- a/libs/gui/fuzzer/libgui_displayEvent_fuzzer.cpp +++ b/libs/gui/fuzzer/libgui_displayEvent_fuzzer.cpp @@ -51,7 +51,7 @@ DisplayEventReceiver::Event buildDisplayEvent(FuzzedDataProvider* fdp, uint32_t event.vsync.count = fdp->ConsumeIntegral(); event.vsync.vsyncData.frameInterval = fdp->ConsumeIntegral(); event.vsync.vsyncData.preferredFrameTimelineIndex = fdp->ConsumeIntegral(); - for (size_t idx = 0; idx < gui::VsyncEventData::kFrameTimelinesLength; ++idx) { + for (size_t idx = 0; idx < gui::VsyncEventData::kFrameTimelinesCapacity; ++idx) { event.vsync.vsyncData.frameTimelines[idx].vsyncId = fdp->ConsumeIntegral(); event.vsync.vsyncData.frameTimelines[idx].deadlineTimestamp = fdp->ConsumeIntegral(); diff --git a/libs/gui/include/gui/VsyncEventData.h b/libs/gui/include/gui/VsyncEventData.h index 9ddb7cd82b..b40a84099c 100644 --- a/libs/gui/include/gui/VsyncEventData.h +++ b/libs/gui/include/gui/VsyncEventData.h @@ -25,7 +25,7 @@ namespace android::gui { // DisplayEventReceiver::Event union. struct VsyncEventData { // Max capacity of frame timelines is arbitrarily set to be reasonable. - static constexpr int64_t kFrameTimelinesLength = 7; + static constexpr int64_t kFrameTimelinesCapacity = 7; // The current frame interval in ns when this frame was scheduled. int64_t frameInterval; @@ -33,7 +33,7 @@ struct VsyncEventData { // Index into the frameTimelines that represents the platform's preferred frame timeline. uint32_t preferredFrameTimelineIndex; - // Size of frame timelines provided by the platform; max is kFrameTimelinesLength. + // Size of frame timelines provided by the platform; max is kFrameTimelinesCapacity. uint32_t frameTimelinesLength; struct alignas(8) FrameTimeline { @@ -48,7 +48,7 @@ struct VsyncEventData { // The anticipated Vsync presentation time in nanos. int64_t expectedPresentationTime; - } frameTimelines[kFrameTimelinesLength]; // Sorted possible frame timelines. + } frameTimelines[kFrameTimelinesCapacity]; // Sorted possible frame timelines. // Gets the preferred frame timeline's vsync ID. int64_t preferredVsyncId() const; diff --git a/libs/gui/tests/DisplayEventStructLayout_test.cpp b/libs/gui/tests/DisplayEventStructLayout_test.cpp index b7b4ab605a..3949d70aac 100644 --- a/libs/gui/tests/DisplayEventStructLayout_test.cpp +++ b/libs/gui/tests/DisplayEventStructLayout_test.cpp @@ -45,16 +45,16 @@ TEST(DisplayEventStructLayoutTest, TestEventAlignment) { // Also test the offsets of the last frame timeline. A loop is not used because the non-const // index cannot be used in static_assert. const int lastFrameTimelineOffset = /* Start of array */ 24 + - (VsyncEventData::kFrameTimelinesLength - 1) * /* Size of FrameTimeline */ 24; + (VsyncEventData::kFrameTimelinesCapacity - 1) * /* Size of FrameTimeline */ 24; CHECK_OFFSET(DisplayEventReceiver::Event::VSync, - vsyncData.frameTimelines[VsyncEventData::kFrameTimelinesLength - 1].vsyncId, + vsyncData.frameTimelines[VsyncEventData::kFrameTimelinesCapacity - 1].vsyncId, lastFrameTimelineOffset); CHECK_OFFSET(DisplayEventReceiver::Event::VSync, - vsyncData.frameTimelines[VsyncEventData::kFrameTimelinesLength - 1] + vsyncData.frameTimelines[VsyncEventData::kFrameTimelinesCapacity - 1] .deadlineTimestamp, lastFrameTimelineOffset + 8); CHECK_OFFSET(DisplayEventReceiver::Event::VSync, - vsyncData.frameTimelines[VsyncEventData::kFrameTimelinesLength - 1] + vsyncData.frameTimelines[VsyncEventData::kFrameTimelinesCapacity - 1] .expectedPresentationTime, lastFrameTimelineOffset + 16); diff --git a/libs/gui/tests/VsyncEventData_test.cpp b/libs/gui/tests/VsyncEventData_test.cpp index 6e039ee6e7..a2138f2144 100644 --- a/libs/gui/tests/VsyncEventData_test.cpp +++ b/libs/gui/tests/VsyncEventData_test.cpp @@ -47,7 +47,7 @@ TEST(ParcelableVsyncEventData, Parcelling) { ASSERT_EQ(data.vsync.frameInterval, data2.vsync.frameInterval); ASSERT_EQ(data.vsync.preferredFrameTimelineIndex, data2.vsync.preferredFrameTimelineIndex); ASSERT_EQ(data.vsync.frameTimelinesLength, data2.vsync.frameTimelinesLength); - for (int i = 0; i < VsyncEventData::kFrameTimelinesLength; i++) { + for (int i = 0; i < VsyncEventData::kFrameTimelinesCapacity; i++) { ASSERT_EQ(data.vsync.frameTimelines[i].vsyncId, data2.vsync.frameTimelines[i].vsyncId); ASSERT_EQ(data.vsync.frameTimelines[i].deadlineTimestamp, data2.vsync.frameTimelines[i].deadlineTimestamp); diff --git a/libs/nativedisplay/AChoreographer.cpp b/libs/nativedisplay/AChoreographer.cpp index 80b0041923..8f005a56f8 100644 --- a/libs/nativedisplay/AChoreographer.cpp +++ b/libs/nativedisplay/AChoreographer.cpp @@ -213,7 +213,7 @@ AVsyncId AChoreographerFrameCallbackData_getFrameTimelineVsyncId( AChoreographerFrameCallbackData_to_ChoreographerFrameCallbackDataImpl(data); LOG_ALWAYS_FATAL_IF(!frameCallbackData->choreographer->inCallback(), "Data is only valid in callback"); - LOG_ALWAYS_FATAL_IF(index >= VsyncEventData::kFrameTimelinesLength, "Index out of bounds"); + LOG_ALWAYS_FATAL_IF(index >= VsyncEventData::kFrameTimelinesCapacity, "Index out of bounds"); return frameCallbackData->vsyncEventData.frameTimelines[index].vsyncId; } int64_t AChoreographerFrameCallbackData_getFrameTimelineExpectedPresentationTimeNanos( @@ -222,7 +222,7 @@ int64_t AChoreographerFrameCallbackData_getFrameTimelineExpectedPresentationTime AChoreographerFrameCallbackData_to_ChoreographerFrameCallbackDataImpl(data); LOG_ALWAYS_FATAL_IF(!frameCallbackData->choreographer->inCallback(), "Data is only valid in callback"); - LOG_ALWAYS_FATAL_IF(index >= VsyncEventData::kFrameTimelinesLength, "Index out of bounds"); + LOG_ALWAYS_FATAL_IF(index >= VsyncEventData::kFrameTimelinesCapacity, "Index out of bounds"); return frameCallbackData->vsyncEventData.frameTimelines[index].expectedPresentationTime; } int64_t AChoreographerFrameCallbackData_getFrameTimelineDeadlineNanos( @@ -231,7 +231,7 @@ int64_t AChoreographerFrameCallbackData_getFrameTimelineDeadlineNanos( AChoreographerFrameCallbackData_to_ChoreographerFrameCallbackDataImpl(data); LOG_ALWAYS_FATAL_IF(!frameCallbackData->choreographer->inCallback(), "Data is only valid in callback"); - LOG_ALWAYS_FATAL_IF(index >= VsyncEventData::kFrameTimelinesLength, "Index out of bounds"); + LOG_ALWAYS_FATAL_IF(index >= VsyncEventData::kFrameTimelinesCapacity, "Index out of bounds"); return frameCallbackData->vsyncEventData.frameTimelines[index].deadlineTimestamp; } diff --git a/services/surfaceflinger/Scheduler/EventThread.cpp b/services/surfaceflinger/Scheduler/EventThread.cpp index f79688dfe0..7b59ca5203 100644 --- a/services/surfaceflinger/Scheduler/EventThread.cpp +++ b/services/surfaceflinger/Scheduler/EventThread.cpp @@ -600,8 +600,8 @@ void EventThread::generateFrameTimeline(VsyncEventData& outVsyncEventData, nsecs nsecs_t preferredDeadlineTimestamp) const { uint32_t currentIndex = 0; // Add 1 to ensure the preferredFrameTimelineIndex entry (when multiplier == 0) is included. - for (int64_t multiplier = -VsyncEventData::kFrameTimelinesLength + 1; - currentIndex < VsyncEventData::kFrameTimelinesLength; multiplier++) { + for (int64_t multiplier = -VsyncEventData::kFrameTimelinesCapacity + 1; + currentIndex < VsyncEventData::kFrameTimelinesCapacity; multiplier++) { nsecs_t deadlineTimestamp = preferredDeadlineTimestamp + multiplier * frameInterval; // Valid possible frame timelines must have future values, so find a later frame timeline. if (deadlineTimestamp <= timestamp) { diff --git a/services/surfaceflinger/tests/DisplayEventReceiver_test.cpp b/services/surfaceflinger/tests/DisplayEventReceiver_test.cpp index 761ac8c538..4c26017241 100644 --- a/services/surfaceflinger/tests/DisplayEventReceiver_test.cpp +++ b/services/surfaceflinger/tests/DisplayEventReceiver_test.cpp @@ -36,7 +36,7 @@ TEST_F(DisplayEventReceiverTest, getLatestVsyncEventData) { EXPECT_GT(static_cast(vsyncEventData.frameTimelinesLength), 0) << "Frame timelines length should be greater than 0"; EXPECT_LE(static_cast(vsyncEventData.frameTimelinesLength), - VsyncEventData::kFrameTimelinesLength) + VsyncEventData::kFrameTimelinesCapacity) << "Frame timelines length should not exceed max capacity"; EXPECT_GT(vsyncEventData.frameTimelines[0].deadlineTimestamp, now) << "Deadline timestamp should be greater than frame time"; diff --git a/services/surfaceflinger/tests/unittests/EventThreadTest.cpp b/services/surfaceflinger/tests/unittests/EventThreadTest.cpp index 6debbaae2d..5fed9b45a6 100644 --- a/services/surfaceflinger/tests/unittests/EventThreadTest.cpp +++ b/services/surfaceflinger/tests/unittests/EventThreadTest.cpp @@ -299,7 +299,7 @@ void EventThreadTest::expectVsyncEventDataFrameTimelinesValidLength( EXPECT_LE(vsyncEventData.frameTimelinesLength, nonPreferredTimelinesAmount + 1) << "Amount of non-preferred frame timelines too many;" << " expected presentation time will be over threshold"; - EXPECT_LT(nonPreferredTimelinesAmount, VsyncEventData::kFrameTimelinesLength) + EXPECT_LT(nonPreferredTimelinesAmount, VsyncEventData::kFrameTimelinesCapacity) << "Amount of non-preferred frame timelines should be less than max capacity"; EXPECT_GT(static_cast(vsyncEventData.frameTimelinesLength), 0) << "Frame timelines length should be greater than 0"; @@ -428,11 +428,11 @@ TEST_F(EventThreadTest, requestNextVsyncEventFrameTimelinesCorrect) { } TEST_F(EventThreadTest, requestNextVsyncEventFrameTimelinesValidLength) { - // The VsyncEventData should not have kFrameTimelinesLength amount of valid frame timelines, due - // to longer vsync period and kEarlyLatchMaxThreshold. - // Use length-2 to avoid decimal truncation (e.g. 60Hz has 16.6... ms vsync period). + // The VsyncEventData should not have kFrameTimelinesCapacity amount of valid frame timelines, + // due to longer vsync period and kEarlyLatchMaxThreshold. Use length-2 to avoid decimal + // truncation (e.g. 60Hz has 16.6... ms vsync period). std::chrono::nanoseconds vsyncPeriod(scheduler::VsyncConfig::kEarlyLatchMaxThreshold / - (VsyncEventData::kFrameTimelinesLength - 2)); + (VsyncEventData::kFrameTimelinesCapacity - 2)); setupEventThread(vsyncPeriod); // Signal that we want the next vsync event to be posted to the connection -- cgit v1.2.3-59-g8ed1b