diff options
24 files changed, 229 insertions, 34 deletions
diff --git a/include/android/choreographer.h b/include/android/choreographer.h index bec3283cd8..2622a01935 100644 --- a/include/android/choreographer.h +++ b/include/android/choreographer.h @@ -318,7 +318,7 @@ size_t AChoreographerFrameCallbackData_getPreferredFrameTimelineIndex( /** * Gets the token used by the platform to identify the frame timeline at the given \c index. - * q + * * Available since API level 33. * * \param index index of a frame timeline, in \f( [0, FrameTimelinesLength) \f). See diff --git a/libs/gui/BufferStuffing.md b/libs/gui/BufferStuffing.md new file mode 100644 index 0000000000..6ed8ad988b --- /dev/null +++ b/libs/gui/BufferStuffing.md @@ -0,0 +1,7 @@ +### Buffer Stuffing and Recovery ### + +Buffer stuffing happens on the client side when SurfaceFlinger misses a frame, but the client continues producing buffers at the same rate. This could occur anytime when SurfaceFlinger does not meet the expected timeline’s deadline to finish composing a frame. As a result, SurfaceFlinger cannot yet release the buffer for the frame that it missed and the client has one less buffer to render into. The client may then run out of buffers or have to wait for buffer release callbacks, increasing the chances of janking when clients render multiple windows. + +Recovery is implemented by first detecting when buffer stuffing occurs and ensuring that the elevated buffer counts in the server are from a relevant SurfaceControl (is a ViewRootImpl). Other SurfaceControl buffer producers such as games, media, and camera have other reasons for expectedly increased buffer counts, which do not need buffer stuffing recovery. + +The actual recovery adjusts the animation timeline in the Choreographer so that the client deadlines for subsequent frames are moved forward in time by one frame. This approach adjusts the client buffer production timeline such that SurfaceFlinger does not fall behind when it misses a frame because the client will simply match its frame production rate with SurfaceFlinger. Ordinarily, buffer stuffing is problematic because the client continues producing buffers when SurfaceFlinger is behind. However, if the client delays producing its buffers to match SurfaceFlinger’s rate, the animation has new frame deadlines that can be reasonably met. The animation is effectively paused for one frame longer than originally intended, and continues the remainder of the animation normally.
\ No newline at end of file diff --git a/libs/gui/include/gui/DisplayEventReceiver.h b/libs/gui/include/gui/DisplayEventReceiver.h index 4dbf9e1929..40a6e79a24 100644 --- a/libs/gui/include/gui/DisplayEventReceiver.h +++ b/libs/gui/include/gui/DisplayEventReceiver.h @@ -119,7 +119,7 @@ public: HdcpLevelsChange hdcpLevelsChange; }; }; - static_assert(sizeof(Event) == 216); + static_assert(sizeof(Event) == 224); public: /* diff --git a/libs/gui/include/gui/IGraphicBufferProducer.h b/libs/gui/include/gui/IGraphicBufferProducer.h index 001e570982..a42ddc466c 100644 --- a/libs/gui/include/gui/IGraphicBufferProducer.h +++ b/libs/gui/include/gui/IGraphicBufferProducer.h @@ -414,7 +414,7 @@ public: uint64_t nextFrameNumber{0}; FrameEventHistoryDelta frameTimestamps; bool bufferReplaced{false}; - int maxBufferCount{0}; + int maxBufferCount{BufferQueueDefs::NUM_BUFFER_SLOTS}; status_t result{NO_ERROR}; }; diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h index 9098dffa8e..1c31e46cb4 100644 --- a/libs/gui/include/gui/LayerState.h +++ b/libs/gui/include/gui/LayerState.h @@ -171,6 +171,10 @@ struct layer_state_t { // Sets a property on this layer indicating that its visible region should be considered // when computing TrustedPresentation Thresholds. eCanOccludePresentation = 0x1000, + // Indicates that the SurfaceControl should recover from buffer stuffing when + // possible. This is the case when the SurfaceControl is the root SurfaceControl + // owned by ViewRootImpl. + eRecoverableFromBufferStuffing = 0x2000, }; enum { diff --git a/libs/gui/include/gui/VsyncEventData.h b/libs/gui/include/gui/VsyncEventData.h index b40a84099c..ced5130505 100644 --- a/libs/gui/include/gui/VsyncEventData.h +++ b/libs/gui/include/gui/VsyncEventData.h @@ -36,6 +36,9 @@ struct VsyncEventData { // Size of frame timelines provided by the platform; max is kFrameTimelinesCapacity. uint32_t frameTimelinesLength; + // Number of queued buffers to indicate if buffer stuffing mode is detected. + uint32_t numberQueuedBuffers; + 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 29eeaa806f..791f471a1d 100644 --- a/libs/gui/tests/DisplayEventStructLayout_test.cpp +++ b/libs/gui/tests/DisplayEventStructLayout_test.cpp @@ -36,15 +36,16 @@ TEST(DisplayEventStructLayoutTest, TestEventAlignment) { 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.numberQueuedBuffers, 24); + CHECK_OFFSET(DisplayEventReceiver::Event::VSync, vsyncData.frameTimelines, 32); + CHECK_OFFSET(DisplayEventReceiver::Event::VSync, vsyncData.frameTimelines[0].vsyncId, 32); CHECK_OFFSET(DisplayEventReceiver::Event::VSync, vsyncData.frameTimelines[0].deadlineTimestamp, - 32); + 40); CHECK_OFFSET(DisplayEventReceiver::Event::VSync, - vsyncData.frameTimelines[0].expectedPresentationTime, 40); + vsyncData.frameTimelines[0].expectedPresentationTime, 48); // 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 + + const int lastFrameTimelineOffset = /* Start of array */ 32 + (VsyncEventData::kFrameTimelinesCapacity - 1) * /* Size of FrameTimeline */ 24; CHECK_OFFSET(DisplayEventReceiver::Event::VSync, vsyncData.frameTimelines[VsyncEventData::kFrameTimelinesCapacity - 1].vsyncId, diff --git a/libs/nativewindow/ANativeWindow.cpp b/libs/nativewindow/ANativeWindow.cpp index ac3a832168..5ce4076476 100644 --- a/libs/nativewindow/ANativeWindow.cpp +++ b/libs/nativewindow/ANativeWindow.cpp @@ -222,6 +222,8 @@ int32_t ANativeWindow_setBuffersDataSpace(ANativeWindow* window, int32_t dataSpa static_cast<int>(HAL_DATASPACE_BT2020_ITU_HLG)); static_assert(static_cast<int>(ADATASPACE_DEPTH) == static_cast<int>(HAL_DATASPACE_DEPTH)); static_assert(static_cast<int>(ADATASPACE_DYNAMIC_DEPTH) == static_cast<int>(HAL_DATASPACE_DYNAMIC_DEPTH)); + static_assert(static_cast<int>(ADATASPACE_DISPLAY_BT2020) == + static_cast<int>(HAL_DATASPACE_DISPLAY_BT2020)); if (!window || !query(window, NATIVE_WINDOW_IS_VALID)) { return -EINVAL; diff --git a/libs/nativewindow/include/android/data_space.h b/libs/nativewindow/include/android/data_space.h index 8056d9ac4f..295a307c3c 100644 --- a/libs/nativewindow/include/android/data_space.h +++ b/libs/nativewindow/include/android/data_space.h @@ -578,6 +578,13 @@ enum ADataSpace : int32_t { */ ADATASPACE_BT2020_ITU_HLG = 302383104, // ADATASPACE_STANDARD_BT2020 | ADATASPACE_TRANSFER_HLG | // ADATASPACE_RANGE_LIMITED + /** + * sRGB-encoded BT. 2020 + * + * Uses full range, sRGB transfer and BT2020 standard. + */ + ADATASPACE_DISPLAY_BT2020 = 142999552, // ADATASPACE_STANDARD_BT2020 | ADATASPACE_TRANSFER_SRGB + // | ADATASPACE_RANGE_FULL /** * Depth diff --git a/libs/renderengine/skia/SkiaRenderEngine.cpp b/libs/renderengine/skia/SkiaRenderEngine.cpp index 5c46c9168d..a93f6c36ff 100644 --- a/libs/renderengine/skia/SkiaRenderEngine.cpp +++ b/libs/renderengine/skia/SkiaRenderEngine.cpp @@ -544,7 +544,8 @@ sk_sp<SkShader> SkiaRenderEngine::createRuntimeEffectShader( } if (graphicBuffer && parameters.layer.luts) { - shader = mLutShader.lutShader(shader, parameters.layer.luts); + shader = mLutShader.lutShader(shader, parameters.layer.luts, + toSkColorSpace(parameters.outputDataSpace)); } if (parameters.requiresLinearEffect) { diff --git a/libs/renderengine/skia/filters/LutShader.cpp b/libs/renderengine/skia/filters/LutShader.cpp index cea46ef40e..1e43ff3cd3 100644 --- a/libs/renderengine/skia/filters/LutShader.cpp +++ b/libs/renderengine/skia/filters/LutShader.cpp @@ -169,7 +169,8 @@ sk_sp<SkShader> LutShader::generateLutShader(sk_sp<SkShader> input, } sk_sp<SkShader> LutShader::lutShader(sk_sp<SkShader>& input, - std::shared_ptr<gui::DisplayLuts> displayLuts) { + std::shared_ptr<gui::DisplayLuts> displayLuts, + sk_sp<SkColorSpace> outColorSpace) { if (mBuilder == nullptr) { const static SkRuntimeEffect::Result instance = SkRuntimeEffect::MakeForShader(kShader); mBuilder = std::make_unique<SkRuntimeShaderBuilder>(instance.effect); @@ -179,14 +180,11 @@ sk_sp<SkShader> LutShader::lutShader(sk_sp<SkShader>& input, if (fd.ok()) { // de-gamma the image without changing the primaries SkImage* baseImage = input->isAImage((SkMatrix*)nullptr, (SkTileMode*)nullptr); - if (baseImage) { - sk_sp<SkColorSpace> baseColorSpace = - baseImage->colorSpace() ? baseImage->refColorSpace() : SkColorSpace::MakeSRGB(); - sk_sp<SkColorSpace> gainmapMathColorSpace = baseColorSpace->makeLinearGamma(); - auto colorXformSdrToGainmap = - SkColorFilterPriv::MakeColorSpaceXform(baseColorSpace, gainmapMathColorSpace); - input = input->makeWithColorFilter(colorXformSdrToGainmap); - } + sk_sp<SkColorSpace> baseColorSpace = baseImage && baseImage->colorSpace() + ? baseImage->refColorSpace() + : SkColorSpace::MakeSRGB(); + sk_sp<SkColorSpace> lutMathColorSpace = baseColorSpace->makeLinearGamma(); + input = input->makeWithWorkingColorSpace(lutMathColorSpace); auto& offsets = displayLuts->offsets; auto& lutProperties = displayLuts->lutProperties; @@ -223,16 +221,9 @@ sk_sp<SkShader> LutShader::lutShader(sk_sp<SkShader>& input, lutProperties[i].samplingKey); } - // re-gamma - baseImage = input->isAImage((SkMatrix*)nullptr, (SkTileMode*)nullptr); - if (baseImage) { - sk_sp<SkColorSpace> baseColorSpace = - baseImage->colorSpace() ? baseImage->refColorSpace() : SkColorSpace::MakeSRGB(); - sk_sp<SkColorSpace> gainmapMathColorSpace = baseColorSpace->makeLinearGamma(); - auto colorXformGainmapToDst = - SkColorFilterPriv::MakeColorSpaceXform(gainmapMathColorSpace, baseColorSpace); - input = input->makeWithColorFilter(colorXformGainmapToDst); - } + auto colorXformLutToDst = + SkColorFilterPriv::MakeColorSpaceXform(lutMathColorSpace, outColorSpace); + input = input->makeWithColorFilter(colorXformLutToDst); } return input; } diff --git a/libs/renderengine/skia/filters/LutShader.h b/libs/renderengine/skia/filters/LutShader.h index c157904b63..ce3e0592a1 100644 --- a/libs/renderengine/skia/filters/LutShader.h +++ b/libs/renderengine/skia/filters/LutShader.h @@ -28,8 +28,8 @@ namespace skia { class LutShader { public: - sk_sp<SkShader> lutShader(sk_sp<SkShader>& input, - std::shared_ptr<gui::DisplayLuts> displayLuts); + sk_sp<SkShader> lutShader(sk_sp<SkShader>& input, std::shared_ptr<gui::DisplayLuts> displayLuts, + sk_sp<SkColorSpace> outColorSpace); private: sk_sp<SkShader> generateLutShader(sk_sp<SkShader> input, const std::vector<float>& buffers, diff --git a/services/inputflinger/reader/mapper/gestures/GestureConverter.cpp b/services/inputflinger/reader/mapper/gestures/GestureConverter.cpp index 19594233fa..54270eb340 100644 --- a/services/inputflinger/reader/mapper/gestures/GestureConverter.cpp +++ b/services/inputflinger/reader/mapper/gestures/GestureConverter.cpp @@ -99,6 +99,8 @@ std::string GestureConverter::dump() const { out << "Current classification: " << ftl::enum_string(mCurrentClassification) << "\n"; out << "Is hovering: " << mIsHovering << "\n"; out << "Enable Tap Timestamp: " << mWhenToEnableTapToClick << "\n"; + out << "Three finger tap shortcut enabled: " + << (mThreeFingerTapShortcutEnabled ? "enabled" : "disabled") << "\n"; return out.str(); } diff --git a/services/powermanager/tests/PowerHalWrapperAidlTest.cpp b/services/powermanager/tests/PowerHalWrapperAidlTest.cpp index f5c4fc547c..ae84d7b133 100644 --- a/services/powermanager/tests/PowerHalWrapperAidlTest.cpp +++ b/services/powermanager/tests/PowerHalWrapperAidlTest.cpp @@ -30,6 +30,8 @@ using aidl::android::hardware::power::Boost; using aidl::android::hardware::power::ChannelConfig; +using aidl::android::hardware::power::CpuHeadroomParams; +using aidl::android::hardware::power::GpuHeadroomParams; using aidl::android::hardware::power::IPower; using aidl::android::hardware::power::IPowerHintSession; using aidl::android::hardware::power::Mode; @@ -71,6 +73,14 @@ public: MOCK_METHOD(ndk::ScopedAStatus, getInterfaceHash, (std::string * hash), (override)); MOCK_METHOD(ndk::SpAIBinder, asBinder, (), (override)); MOCK_METHOD(bool, isRemote, (), (override)); + MOCK_METHOD(ndk::ScopedAStatus, getCpuHeadroom, + (const CpuHeadroomParams& params, std::vector<float>* headroom), (override)); + MOCK_METHOD(ndk::ScopedAStatus, getGpuHeadroom, + (const GpuHeadroomParams& params, float* headroom), (override)); + MOCK_METHOD(ndk::ScopedAStatus, getCpuHeadroomMinIntervalMillis, (int64_t* interval), + (override)); + MOCK_METHOD(ndk::ScopedAStatus, getGpuHeadroomMinIntervalMillis, (int64_t* interval), + (override)); }; // ------------------------------------------------------------------------------------------------- diff --git a/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp b/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp index c13e444a99..86d7388f5b 100644 --- a/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp +++ b/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp @@ -29,6 +29,7 @@ #include <cinttypes> #include <numeric> #include <unordered_set> +#include <vector> #include "../Jank/JankTracker.h" @@ -1002,6 +1003,11 @@ void FrameTimeline::setSfPresent(nsecs_t sfPresentTime, finalizeCurrentDisplayFrame(); } +const std::vector<std::shared_ptr<frametimeline::SurfaceFrame>>& FrameTimeline::getPresentFrames() + const { + return mPresentFrames; +} + void FrameTimeline::onCommitNotComposited() { SFTRACE_CALL(); std::scoped_lock lock(mMutex); @@ -1521,6 +1527,7 @@ void FrameTimeline::flushPendingPresentFences() { mPendingPresentFences.erase(mPendingPresentFences.begin()); } + mPresentFrames.clear(); for (size_t i = 0; i < mPendingPresentFences.size(); i++) { const auto& pendingPresentFence = mPendingPresentFences[i]; nsecs_t signalTime = Fence::SIGNAL_TIME_INVALID; @@ -1533,6 +1540,13 @@ void FrameTimeline::flushPendingPresentFences() { auto& displayFrame = pendingPresentFence.second; displayFrame->onPresent(signalTime, mPreviousActualPresentTime); + + // Surface frames have been jank classified and can be provided to caller + // to detect if buffer stuffing is occurring. + for (const auto& frame : displayFrame->getSurfaceFrames()) { + mPresentFrames.push_back(frame); + } + mPreviousPredictionPresentTime = displayFrame->trace(mSurfaceFlingerPid, monoBootOffset, mPreviousPredictionPresentTime, mFilterFramesBeforeTraceStarts); diff --git a/services/surfaceflinger/FrameTimeline/FrameTimeline.h b/services/surfaceflinger/FrameTimeline/FrameTimeline.h index 6cda309440..a47bd573de 100644 --- a/services/surfaceflinger/FrameTimeline/FrameTimeline.h +++ b/services/surfaceflinger/FrameTimeline/FrameTimeline.h @@ -328,6 +328,11 @@ public: virtual void setSfPresent(nsecs_t sfPresentTime, const std::shared_ptr<FenceTime>& presentFence, const std::shared_ptr<FenceTime>& gpuFence) = 0; + // Provides surface frames that have already been jank classified in the most recent + // flush of pending present fences. This allows buffer stuffing detection from SF. + virtual const std::vector<std::shared_ptr<frametimeline::SurfaceFrame>>& getPresentFrames() + const = 0; + // Tells FrameTimeline that a frame was committed but not composited. This is used to flush // all the associated surface frames. virtual void onCommitNotComposited() = 0; @@ -505,6 +510,8 @@ public: void setSfWakeUp(int64_t token, nsecs_t wakeupTime, Fps refreshRate, Fps renderRate) override; void setSfPresent(nsecs_t sfPresentTime, const std::shared_ptr<FenceTime>& presentFence, const std::shared_ptr<FenceTime>& gpuFence = FenceTime::NO_FENCE) override; + const std::vector<std::shared_ptr<frametimeline::SurfaceFrame>>& getPresentFrames() + const override; void onCommitNotComposited() override; void parseArgs(const Vector<String16>& args, std::string& result) override; void setMaxDisplayFrames(uint32_t size) override; @@ -552,6 +559,9 @@ private: // display frame, this is a good starting size for the vector so that we can avoid the // internal vector resizing that happens with push_back. static constexpr uint32_t kNumSurfaceFramesInitial = 10; + // Presented surface frames that have been jank classified and can + // indicate of potential buffer stuffing. + std::vector<std::shared_ptr<frametimeline::SurfaceFrame>> mPresentFrames; }; } // namespace impl diff --git a/services/surfaceflinger/Scheduler/EventThread.cpp b/services/surfaceflinger/Scheduler/EventThread.cpp index 7729671401..fff4284199 100644 --- a/services/surfaceflinger/Scheduler/EventThread.cpp +++ b/services/surfaceflinger/Scheduler/EventThread.cpp @@ -44,10 +44,8 @@ #include <common/FlagManager.h> #include <scheduler/VsyncConfig.h> -#include "DisplayHardware/DisplayMode.h" #include "FrameTimeline.h" #include "VSyncDispatch.h" -#include "VSyncTracker.h" #include "EventThread.h" @@ -482,6 +480,14 @@ void EventThread::onHdcpLevelsChanged(PhysicalDisplayId displayId, int32_t conne mCondition.notify_all(); } +// Merge lists of buffer stuffed Uids +void EventThread::addBufferStuffedUids(BufferStuffingMap bufferStuffedUids) { + std::lock_guard<std::mutex> lock(mMutex); + for (auto& [uid, count] : bufferStuffedUids) { + mBufferStuffedUids.emplace_or_replace(uid, count); + } +} + void EventThread::threadMain(std::unique_lock<std::mutex>& lock) { DisplayEventConsumers consumers; @@ -721,6 +727,10 @@ void EventThread::generateFrameTimeline(VsyncEventData& outVsyncEventData, nsecs void EventThread::dispatchEvent(const DisplayEventReceiver::Event& event, const DisplayEventConsumers& consumers) { + // List of Uids that have been sent vsync data with queued buffer count. + // Used to keep track of which Uids can be removed from the map of + // buffer stuffed clients. + ftl::SmallVector<uid_t, 10> uidsPostedQueuedBuffers; for (const auto& consumer : consumers) { DisplayEventReceiver::Event copy = event; if (event.header.type == DisplayEventReceiver::DISPLAY_EVENT_VSYNC) { @@ -730,6 +740,13 @@ void EventThread::dispatchEvent(const DisplayEventReceiver::Event& event, event.vsync.vsyncData.preferredExpectedPresentationTime(), event.vsync.vsyncData.preferredDeadlineTimestamp()); } + auto it = mBufferStuffedUids.find(consumer->mOwnerUid); + if (it != mBufferStuffedUids.end()) { + copy.vsync.vsyncData.numberQueuedBuffers = it->second; + uidsPostedQueuedBuffers.emplace_back(consumer->mOwnerUid); + } else { + copy.vsync.vsyncData.numberQueuedBuffers = 0; + } switch (consumer->postEvent(copy)) { case NO_ERROR: break; @@ -745,6 +762,12 @@ void EventThread::dispatchEvent(const DisplayEventReceiver::Event& event, removeDisplayEventConnectionLocked(consumer); } } + // The clients that have already received the queued buffer count + // can be removed from the buffer stuffed Uid list to avoid + // being sent duplicate messages. + for (auto uid : uidsPostedQueuedBuffers) { + mBufferStuffedUids.erase(uid); + } if (event.header.type == DisplayEventReceiver::DISPLAY_EVENT_VSYNC && FlagManager::getInstance().vrr_config()) { mLastCommittedVsyncTime = diff --git a/services/surfaceflinger/Scheduler/EventThread.h b/services/surfaceflinger/Scheduler/EventThread.h index 2daf126d77..95632c7e66 100644 --- a/services/surfaceflinger/Scheduler/EventThread.h +++ b/services/surfaceflinger/Scheduler/EventThread.h @@ -32,7 +32,6 @@ #include <thread> #include <vector> -#include "DisplayHardware/DisplayMode.h" #include "TracedOrdinal.h" #include "VSyncDispatch.h" #include "VsyncSchedule.h" @@ -55,6 +54,7 @@ using gui::VsyncEventData; // --------------------------------------------------------------------------- using FrameRateOverride = DisplayEventReceiver::Event::FrameRateOverride; +using BufferStuffingMap = ftl::SmallMap<uid_t, uint32_t, 10>; enum class VSyncRequest { None = -2, @@ -136,6 +136,10 @@ public: virtual void onHdcpLevelsChanged(PhysicalDisplayId displayId, int32_t connectedLevel, int32_t maxLevel) = 0; + + // An elevated number of queued buffers in the server is detected. This propagates a + // flag to Choreographer indicating that buffer stuffing recovery should begin. + virtual void addBufferStuffedUids(BufferStuffingMap bufferStuffedUids); }; struct IEventThreadCallback { @@ -188,6 +192,8 @@ public: void onHdcpLevelsChanged(PhysicalDisplayId displayId, int32_t connectedLevel, int32_t maxLevel) override; + void addBufferStuffedUids(BufferStuffingMap bufferStuffedUids) override; + private: friend EventThreadTest; @@ -228,6 +234,10 @@ private: scheduler::VSyncCallbackRegistration mVsyncRegistration GUARDED_BY(mMutex); frametimeline::TokenManager* const mTokenManager; + // All consumers that need to recover from buffer stuffing and the number + // of their queued buffers. + BufferStuffingMap mBufferStuffedUids GUARDED_BY(mMutex); + IEventThreadCallback& mCallback; std::thread mThread; diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp index 274e121a79..8c587a9376 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.cpp +++ b/services/surfaceflinger/Scheduler/Scheduler.cpp @@ -951,6 +951,11 @@ bool Scheduler::updateFrameRateOverridesLocked(GlobalSignals consideredSignals, return mFrameRateOverrideMappings.updateFrameRateOverridesByContent(frameRateOverrides); } +void Scheduler::addBufferStuffedUids(BufferStuffingMap bufferStuffedUids) { + if (!mRenderEventThread) return; + mRenderEventThread->addBufferStuffedUids(std::move(bufferStuffedUids)); +} + void Scheduler::promotePacesetterDisplay(PhysicalDisplayId pacesetterId, PromotionParams params) { std::shared_ptr<VsyncSchedule> pacesetterVsyncSchedule; { diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h index e77af609e2..8340880918 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.h +++ b/services/surfaceflinger/Scheduler/Scheduler.h @@ -42,7 +42,6 @@ #include <ui/DisplayId.h> #include <ui/DisplayMap.h> -#include "Display/DisplayModeRequest.h" #include "EventThread.h" #include "FrameRateOverrideMappings.h" #include "ISchedulerCallback.h" @@ -334,6 +333,10 @@ public: mPacesetterFrameDurationFractionToSkip = frameDurationFraction; } + // Propagates a flag to the EventThread indicating that buffer stuffing + // recovery should begin. + void addBufferStuffedUids(BufferStuffingMap bufferStuffedUids); + private: friend class TestableScheduler; diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 0d03a6cef3..9854174bbd 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -64,11 +64,13 @@ #include <ftl/concat.h> #include <ftl/fake_guard.h> #include <ftl/future.h> +#include <ftl/small_map.h> #include <ftl/unit.h> #include <gui/AidlUtil.h> #include <gui/BufferQueue.h> #include <gui/DebugEGLImageTracker.h> #include <gui/IProducerListener.h> +#include <gui/JankInfo.h> #include <gui/LayerMetadata.h> #include <gui/LayerState.h> #include <gui/Surface.h> @@ -3072,12 +3074,40 @@ void SurfaceFlinger::onCompositionPresented(PhysicalDisplayId pacesetterId, const TimePoint presentTime = TimePoint::now(); + // The Uids of layer owners that are in buffer stuffing mode, and their elevated + // buffer counts. Messages to start recovery are sent exclusively to these Uids. + BufferStuffingMap bufferStuffedUids; + // Set presentation information before calling Layer::releasePendingBuffer, such that jank // information from previous' frame classification is already available when sending jank info // to clients, so they get jank classification as early as possible. mFrameTimeline->setSfPresent(presentTime.ns(), pacesetterPresentFenceTime, pacesetterGpuCompositionDoneFenceTime); + // Find and register any layers that are in buffer stuffing mode + const auto& presentFrames = mFrameTimeline->getPresentFrames(); + + for (const auto& frame : presentFrames) { + const auto& layer = mLayerLifecycleManager.getLayerFromId(frame->getLayerId()); + if (!layer) continue; + uint32_t numberQueuedBuffers = layer->pendingBuffers ? layer->pendingBuffers->load() : 0; + int32_t jankType = frame->getJankType().value_or(JankType::None); + if (jankType & JankType::BufferStuffing && + layer->flags & layer_state_t::eRecoverableFromBufferStuffing) { + auto [it, wasEmplaced] = + bufferStuffedUids.try_emplace(layer->ownerUid.val(), numberQueuedBuffers); + // Update with maximum number of queued buffers, allows clients drawing + // multiple windows to account for the most severely stuffed window + if (!wasEmplaced && it->second < numberQueuedBuffers) { + it->second = numberQueuedBuffers; + } + } + } + + if (!bufferStuffedUids.empty()) { + mScheduler->addBufferStuffedUids(std::move(bufferStuffedUids)); + } + // We use the CompositionEngine::getLastFrameRefreshTimestamp() which might // be sampled a little later than when we started doing work for this frame, // but that should be okay since CompositorTiming has snapping logic. diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index d3479b7e52..7e9d5b881f 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -1370,6 +1370,8 @@ private: // Flag used to set override desired display mode from backdoor bool mDebugDisplayModeSetByBackdoor = false; + // Tracks the number of maximum queued buffers by layer owner Uid. + using BufferStuffingMap = ftl::SmallMap<uid_t, uint32_t, 10>; BufferCountTracker mBufferCountTracker; std::unordered_map<DisplayId, sp<HdrLayerInfoReporter>> mHdrLayerInfoListeners diff --git a/services/surfaceflinger/tests/unittests/EventThreadTest.cpp b/services/surfaceflinger/tests/unittests/EventThreadTest.cpp index 625d2e68d8..268a6c416d 100644 --- a/services/surfaceflinger/tests/unittests/EventThreadTest.cpp +++ b/services/surfaceflinger/tests/unittests/EventThreadTest.cpp @@ -23,6 +23,7 @@ #include <gmock/gmock.h> #include <gtest/gtest.h> +#include <gui/DisplayEventReceiver.h> #include <log/log.h> #include <scheduler/VsyncConfig.h> #include <utils/Errors.h> @@ -111,6 +112,8 @@ protected: void expectOnExpectedPresentTimePosted(nsecs_t expectedPresentTime); void expectUidFrameRateMappingEventReceivedByConnection(PhysicalDisplayId expectedDisplayId, std::vector<FrameRateOverride>); + void expectQueuedBufferCountReceivedByConnection( + ConnectionEventRecorder& connectionEventRecorder, uint32_t expectedBufferCount); void onVSyncEvent(nsecs_t timestamp, nsecs_t expectedPresentationTime, nsecs_t deadlineTimestamp) { @@ -144,6 +147,7 @@ protected: sp<MockEventThreadConnection> mConnection; sp<MockEventThreadConnection> mThrottledConnection; std::unique_ptr<frametimeline::impl::TokenManager> mTokenManager; + std::vector<ConnectionEventRecorder*> mBufferStuffedConnectionRecorders; std::chrono::nanoseconds mVsyncPeriod; @@ -376,6 +380,14 @@ void EventThreadTest::expectUidFrameRateMappingEventReceivedByConnection( EXPECT_EQ(expectedDisplayId, event.header.displayId); } +void EventThreadTest::expectQueuedBufferCountReceivedByConnection( + ConnectionEventRecorder& connectionEventRecorder, uint32_t expectedBufferCount) { + auto args = connectionEventRecorder.waitForCall(); + ASSERT_TRUE(args.has_value()); + const auto& event = std::get<0>(args.value()); + EXPECT_EQ(expectedBufferCount, event.vsync.vsyncData.numberQueuedBuffers); +} + namespace { using namespace testing; @@ -868,6 +880,63 @@ TEST_F(EventThreadTest, postHcpLevelsChanged) { EXPECT_EQ(HDCP_V2, event.hdcpLevelsChange.maxLevel); } +TEST_F(EventThreadTest, connectionReceivesBufferStuffing) { + setupEventThread(); + + // Create a connection that will experience buffer stuffing. + ConnectionEventRecorder stuffedConnectionEventRecorder{0}; + sp<MockEventThreadConnection> stuffedConnection = + createConnection(stuffedConnectionEventRecorder, + gui::ISurfaceComposer::EventRegistration::modeChanged | + gui::ISurfaceComposer::EventRegistration::frameRateOverride, + 111); + + // Add a connection and buffer count to the list of stuffed Uids that will receive + // data in the next vsync event. + BufferStuffingMap bufferStuffedUids; + bufferStuffedUids.try_emplace(stuffedConnection->mOwnerUid, 3); + mThread->addBufferStuffedUids(bufferStuffedUids); + mBufferStuffedConnectionRecorders.emplace_back(&stuffedConnectionEventRecorder); + + // Signal that we want the next vsync event to be posted to two connections. + mThread->requestNextVsync(mConnection); + mThread->requestNextVsync(stuffedConnection); + onVSyncEvent(123, 456, 789); + + // Vsync event data contains number of queued buffers. + expectQueuedBufferCountReceivedByConnection(mConnectionEventCallRecorder, 0); + expectQueuedBufferCountReceivedByConnection(stuffedConnectionEventRecorder, 3); +} + +TEST_F(EventThreadTest, connectionsWithSameUidReceiveBufferStuffing) { + setupEventThread(); + + // Create a connection with the same Uid as another connection. + ConnectionEventRecorder secondConnectionEventRecorder{0}; + sp<MockEventThreadConnection> secondConnection = + createConnection(secondConnectionEventRecorder, + gui::ISurfaceComposer::EventRegistration::modeChanged | + gui::ISurfaceComposer::EventRegistration::frameRateOverride, + mConnectionUid); + + // Add connection Uid and buffer count to the list of stuffed Uids that will receive + // data in the next vsync event. + BufferStuffingMap bufferStuffedUids; + bufferStuffedUids.try_emplace(mConnectionUid, 3); + mThread->addBufferStuffedUids(bufferStuffedUids); + mBufferStuffedConnectionRecorders.emplace_back(&mConnectionEventCallRecorder); + mBufferStuffedConnectionRecorders.emplace_back(&secondConnectionEventRecorder); + + // Signal that we want the next vsync event to be posted to two connections. + mThread->requestNextVsync(mConnection); + mThread->requestNextVsync(secondConnection); + onVSyncEvent(123, 456, 789); + + // Vsync event data contains number of queued buffers. + expectQueuedBufferCountReceivedByConnection(mConnectionEventCallRecorder, 3); + expectQueuedBufferCountReceivedByConnection(secondConnectionEventRecorder, 3); +} + } // namespace } // namespace android diff --git a/services/surfaceflinger/tests/unittests/mock/MockEventThread.h b/services/surfaceflinger/tests/unittests/mock/MockEventThread.h index 82500fef10..c976697c08 100644 --- a/services/surfaceflinger/tests/unittests/mock/MockEventThread.h +++ b/services/surfaceflinger/tests/unittests/mock/MockEventThread.h @@ -53,6 +53,7 @@ public: MOCK_METHOD(void, onHdcpLevelsChanged, (PhysicalDisplayId displayId, int32_t connectedLevel, int32_t maxLevel), (override)); + MOCK_METHOD(void, addBufferStuffedUids, (BufferStuffingMap), (override)); }; } // namespace android::mock |