From 4d0345c4e9a2cd3be4dc1b5514a628dd92862904 Mon Sep 17 00:00:00 2001 From: Huihong Luo Date: Tue, 25 Apr 2023 20:40:52 +0000 Subject: Add pending command buffer reset when error occurs, so it won't affect the upcoming commands. Bug: 273525126 Test: manual (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:3fd3acc99b20cd29837053716086eca8406cd3b0) Merged-In: I08f938764dda78e6a85e625ae6767816582f5312 Change-Id: I08f938764dda78e6a85e625ae6767816582f5312 --- services/surfaceflinger/DisplayHardware/AidlComposerHal.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/services/surfaceflinger/DisplayHardware/AidlComposerHal.cpp b/services/surfaceflinger/DisplayHardware/AidlComposerHal.cpp index 79dcd159d3..fb5f0cb30f 100644 --- a/services/surfaceflinger/DisplayHardware/AidlComposerHal.cpp +++ b/services/surfaceflinger/DisplayHardware/AidlComposerHal.cpp @@ -765,6 +765,7 @@ Error AidlComposer::execute() { auto status = mAidlComposerClient->executeCommands(commands, &results); if (!status.isOk()) { ALOGE("executeCommands failed %s", status.getDescription().c_str()); + mWriter.reset(); return static_cast(status.getServiceSpecificError()); } @@ -776,6 +777,7 @@ Error AidlComposer::execute() { const auto index = static_cast(cmdErr.commandIndex); if (index < 0 || index >= commands.size()) { ALOGE("invalid command index %zu", index); + mWriter.reset(); return Error::BAD_PARAMETER; } -- cgit v1.2.3 From 675bd8b7963d1d5ba1fe364ab0f5c9361255d779 Mon Sep 17 00:00:00 2001 From: Patrick Williams Date: Fri, 28 Apr 2023 09:51:07 -0500 Subject: DO NOT MERGE Reverts WindowInfosListenerInvoker throttling Revert "DO NOT MERGE: Force input window updates when layer visibility changes" Revert "SF: throttle WindowInfosListener calls" The reverted CLs may be causing input issues where InputDispatcher doesn't seem to align with the windows currently displayed. This reverts commit 243e2e9af82700c76f743ee71e0f27fe1a3f4383. This reverts commit f3213d2ca821472b12ce2452e4bd33b4cd152e8f. Bug: 279649321 Bug: 270894765 Test: presubmits (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:fd9c44fb10610d8c9d7f698868a4f6a5d0547fd9) Merged-In: I8eb46caced6f395460fabc1d6a529a88c1c4566b Change-Id: I8eb46caced6f395460fabc1d6a529a88c1c4566b --- services/surfaceflinger/Layer.cpp | 11 ++- services/surfaceflinger/Layer.h | 15 ---- services/surfaceflinger/SurfaceFlinger.cpp | 24 +----- services/surfaceflinger/SurfaceFlinger.h | 5 -- .../surfaceflinger/WindowInfosListenerInvoker.cpp | 93 +++++----------------- .../surfaceflinger/WindowInfosListenerInvoker.h | 12 ++- 6 files changed, 38 insertions(+), 122 deletions(-) diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 4593b40b7d..aff94d132e 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -2391,7 +2391,16 @@ WindowInfo Layer::fillInputInfo(const InputDisplayArgs& displayArgs) { info.inputConfig |= WindowInfo::InputConfig::NOT_TOUCHABLE; } - info.setInputConfig(WindowInfo::InputConfig::NOT_VISIBLE, !isVisibleForInput()); + // For compatibility reasons we let layers which can receive input + // receive input before they have actually submitted a buffer. Because + // of this we use canReceiveInput instead of isVisible to check the + // policy-visibility, ignoring the buffer state. However for layers with + // hasInputInfo()==false we can use the real visibility state. + // We are just using these layers for occlusion detection in + // InputDispatcher, and obviously if they aren't visible they can't occlude + // anything. + const bool visible = hasInputInfo() ? canReceiveInput() : isVisible(); + info.setInputConfig(WindowInfo::InputConfig::NOT_VISIBLE, !visible); info.alpha = getAlpha(); fillTouchOcclusionMode(info); diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index 1724c150ad..200baf0ba1 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -470,21 +470,6 @@ public: */ virtual bool canReceiveInput() const; - /* - * Whether or not the layer should be considered visible for input calculations. - */ - virtual bool isVisibleForInput() const { - // For compatibility reasons we let layers which can receive input - // receive input before they have actually submitted a buffer. Because - // of this we use canReceiveInput instead of isVisible to check the - // policy-visibility, ignoring the buffer state. However for layers with - // hasInputInfo()==false we can use the real visibility state. - // We are just using these layers for occlusion detection in - // InputDispatcher, and obviously if they aren't visible they can't occlude - // anything. - return hasInputInfo() ? canReceiveInput() : isVisible(); - } - /* * isProtected - true if the layer may contain protected contents in the * GRALLOC_USAGE_PROTECTED sense. diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 1dfb1d3e9b..494aa2c2cf 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -3249,34 +3249,16 @@ void SurfaceFlinger::updateInputFlinger() { if (!updateWindowInfo && mInputWindowCommands.empty()) { return; } - - std::unordered_set visibleLayers; - mDrawingState.traverse([&visibleLayers](Layer* layer) { - if (layer->isVisibleForInput()) { - visibleLayers.insert(layer); - } - }); - bool visibleLayersChanged = false; - if (visibleLayers != mVisibleLayers) { - visibleLayersChanged = true; - mVisibleLayers = std::move(visibleLayers); - } - BackgroundExecutor::getInstance().sendCallbacks({[updateWindowInfo, windowInfos = std::move(windowInfos), displayInfos = std::move(displayInfos), inputWindowCommands = std::move(mInputWindowCommands), - inputFlinger = mInputFlinger, this, - visibleLayersChanged]() { + inputFlinger = mInputFlinger, this]() { ATRACE_NAME("BackgroundExecutor::updateInputFlinger"); if (updateWindowInfo) { - mWindowInfosListenerInvoker - ->windowInfosChanged(std::move(windowInfos), std::move(displayInfos), - /* shouldSync= */ inputWindowCommands.syncInputWindows, - /* forceImmediateCall= */ - visibleLayersChanged || - !inputWindowCommands.focusRequests.empty()); + mWindowInfosListenerInvoker->windowInfosChanged(windowInfos, displayInfos, + inputWindowCommands.syncInputWindows); } else if (inputWindowCommands.syncInputWindows) { // If the caller requested to sync input windows, but there are no // changes to input windows, notify immediately. diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index a78144dea2..3a45229ed3 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -1449,11 +1449,6 @@ private: nsecs_t mAnimationTransactionTimeout = s2ns(5); friend class SurfaceComposerAIDL; - - // Layers visible during the last commit. This set should only be used for testing set equality - // and membership. The pointers should not be dereferenced as it's possible the set contains - // pointers to freed layers. - std::unordered_set mVisibleLayers; }; class SurfaceComposerAIDL : public gui::BnSurfaceComposer { diff --git a/services/surfaceflinger/WindowInfosListenerInvoker.cpp b/services/surfaceflinger/WindowInfosListenerInvoker.cpp index 023402f747..30b9d8f1cb 100644 --- a/services/surfaceflinger/WindowInfosListenerInvoker.cpp +++ b/services/surfaceflinger/WindowInfosListenerInvoker.cpp @@ -28,26 +28,19 @@ using gui::WindowInfo; struct WindowInfosListenerInvoker::WindowInfosReportedListener : gui::BnWindowInfosReportedListener { - explicit WindowInfosReportedListener(WindowInfosListenerInvoker& invoker, size_t callbackCount, - bool shouldSync) - : mInvoker(invoker), mCallbacksPending(callbackCount), mShouldSync(shouldSync) {} + explicit WindowInfosReportedListener(WindowInfosListenerInvoker& invoker) : mInvoker(invoker) {} binder::Status onWindowInfosReported() override { - mCallbacksPending--; - if (mCallbacksPending == 0) { - mInvoker.windowInfosReported(mShouldSync); - } + mInvoker.windowInfosReported(); return binder::Status::ok(); } -private: WindowInfosListenerInvoker& mInvoker; - std::atomic mCallbacksPending; - bool mShouldSync; }; WindowInfosListenerInvoker::WindowInfosListenerInvoker(SurfaceFlinger& flinger) - : mFlinger(flinger) {} + : mFlinger(flinger), + mWindowInfosReportedListener(sp::make(*this)) {} void WindowInfosListenerInvoker::addWindowInfosListener(sp listener) { sp asBinder = IInterface::asBinder(listener); @@ -71,76 +64,30 @@ void WindowInfosListenerInvoker::binderDied(const wp& who) { mWindowInfosListeners.erase(who); } -void WindowInfosListenerInvoker::windowInfosChanged(std::vector windowInfos, - std::vector displayInfos, - bool shouldSync, bool forceImmediateCall) { - auto callListeners = [this, windowInfos = std::move(windowInfos), - displayInfos = std::move(displayInfos)](bool shouldSync) mutable { - ftl::SmallVector, kStaticCapacity> windowInfosListeners; - { - std::scoped_lock lock(mListenersMutex); - for (const auto& [_, listener] : mWindowInfosListeners) { - windowInfosListeners.push_back(listener); - } - } - - auto reportedListener = - sp::make(*this, windowInfosListeners.size(), - shouldSync); - - for (const auto& listener : windowInfosListeners) { - auto status = - listener->onWindowInfosChanged(windowInfos, displayInfos, reportedListener); - if (!status.isOk()) { - reportedListener->onWindowInfosReported(); - } - } - }; - +void WindowInfosListenerInvoker::windowInfosChanged(const std::vector& windowInfos, + const std::vector& displayInfos, + bool shouldSync) { + ftl::SmallVector, kStaticCapacity> windowInfosListeners; { - std::scoped_lock lock(mMessagesMutex); - // If there are unacked messages and this isn't a forced call, then return immediately. - // If a forced window infos change doesn't happen first, the update will be sent after - // the WindowInfosReportedListeners are called. If a forced window infos change happens or - // if there are subsequent delayed messages before this update is sent, then this message - // will be dropped and the listeners will only be called with the latest info. This is done - // to reduce the amount of binder memory used. - if (mActiveMessageCount > 0 && !forceImmediateCall) { - mWindowInfosChangedDelayed = std::move(callListeners); - mShouldSyncDelayed |= shouldSync; - return; + std::scoped_lock lock(mListenersMutex); + for (const auto& [_, listener] : mWindowInfosListeners) { + windowInfosListeners.push_back(listener); } + } - mWindowInfosChangedDelayed = nullptr; - shouldSync |= mShouldSyncDelayed; - mShouldSyncDelayed = false; - mActiveMessageCount++; + mCallbacksPending = windowInfosListeners.size(); + + for (const auto& listener : windowInfosListeners) { + listener->onWindowInfosChanged(windowInfos, displayInfos, + shouldSync ? mWindowInfosReportedListener : nullptr); } - callListeners(shouldSync); } -void WindowInfosListenerInvoker::windowInfosReported(bool shouldSync) { - if (shouldSync) { +void WindowInfosListenerInvoker::windowInfosReported() { + mCallbacksPending--; + if (mCallbacksPending == 0) { mFlinger.windowInfosReported(); } - - std::function callListeners; - bool shouldSyncDelayed; - { - std::scoped_lock lock{mMessagesMutex}; - mActiveMessageCount--; - if (!mWindowInfosChangedDelayed || mActiveMessageCount > 0) { - return; - } - - mActiveMessageCount++; - callListeners = std::move(mWindowInfosChangedDelayed); - mWindowInfosChangedDelayed = nullptr; - shouldSyncDelayed = mShouldSyncDelayed; - mShouldSyncDelayed = false; - } - - callListeners(shouldSyncDelayed); } } // namespace android diff --git a/services/surfaceflinger/WindowInfosListenerInvoker.h b/services/surfaceflinger/WindowInfosListenerInvoker.h index 701f11efcd..d8d8d0f570 100644 --- a/services/surfaceflinger/WindowInfosListenerInvoker.h +++ b/services/surfaceflinger/WindowInfosListenerInvoker.h @@ -34,15 +34,15 @@ public: void addWindowInfosListener(sp); void removeWindowInfosListener(const sp& windowInfosListener); - void windowInfosChanged(std::vector, std::vector, - bool shouldSync, bool forceImmediateCall); + void windowInfosChanged(const std::vector&, + const std::vector&, bool shouldSync); protected: void binderDied(const wp& who) override; private: struct WindowInfosReportedListener; - void windowInfosReported(bool shouldSync); + void windowInfosReported(); SurfaceFlinger& mFlinger; std::mutex mListenersMutex; @@ -51,10 +51,8 @@ private: ftl::SmallMap, const sp, kStaticCapacity> mWindowInfosListeners GUARDED_BY(mListenersMutex); - std::mutex mMessagesMutex; - uint32_t mActiveMessageCount GUARDED_BY(mMessagesMutex) = 0; - std::function mWindowInfosChangedDelayed GUARDED_BY(mMessagesMutex); - bool mShouldSyncDelayed; + sp mWindowInfosReportedListener; + std::atomic mCallbacksPending{0}; }; } // namespace android -- cgit v1.2.3