From aeed0da5878b59b42a9e9e40e48f36809bb315cb Mon Sep 17 00:00:00 2001 From: Siarhei Vishniakou Date: Tue, 9 Jan 2024 08:57:13 -0800 Subject: Check for redundant windows inside WindowInfos Recently, our team has seen traces where dispatcher is taking a long time to run - sometimes, more than 22 milliseconds. That's way longer than the expected ~ 0.3 milliseconds for the entire inputflinger. After examining the traces, I noticed that there's a lot of time spent in both findTouchedWindowTargets and dispatchInputEvent. It appears that we are trying to dispatch one input event multiple times to the same channel, dreamOverlay. One way this could be possible if we receive multiple, redundant windows. In this CL, we add a crash to identify and get out of this bad state. Bug: 311142655 Test: TEST=inputflinger_tests; m $TEST && $ANDROID_HOST_OUT/nativetest64/$TEST/$TEST Change-Id: I2170928d1bb1e591b9c93b42dd86827e86a0c7fb --- libs/gui/WindowInfo.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'libs/gui/WindowInfo.cpp') diff --git a/libs/gui/WindowInfo.cpp b/libs/gui/WindowInfo.cpp index ba1d196e9c..95b2641f04 100644 --- a/libs/gui/WindowInfo.cpp +++ b/libs/gui/WindowInfo.cpp @@ -258,8 +258,7 @@ void WindowInfoHandle::updateFrom(sp handle) { mInfo = handle->mInfo; } -std::ostream& operator<<(std::ostream& out, const WindowInfoHandle& window) { - const WindowInfo& info = *window.getInfo(); +std::ostream& operator<<(std::ostream& out, const WindowInfo& info) { std::string transform; info.transform.dump(transform, "transform", " "); out << "name=" << info.name << ", id=" << info.id << ", displayId=" << info.displayId @@ -277,4 +276,10 @@ std::ostream& operator<<(std::ostream& out, const WindowInfoHandle& window) { return out; } +std::ostream& operator<<(std::ostream& out, const WindowInfoHandle& window) { + const WindowInfo& info = *window.getInfo(); + out << info; + return out; +} + } // namespace android::gui -- cgit v1.2.3-59-g8ed1b From 59a6be3c5d547a02850bf486cfe1036f4008df83 Mon Sep 17 00:00:00 2001 From: Vishnu Nair Date: Mon, 29 Jan 2024 10:26:21 -0800 Subject: Introduce eCanOccludePresentation layer flag Sets a property on a layer hierarchy indicating that its visible region should be considered when computing TrustedPresentation Thresholds. This property is set on a layer and inherited by all its children. The property is then passed via windowinfos so the TrustedPresentation controller can determine which windows to use when computing the thresholds. Test: presubmit Bug: b/275574214 Change-Id: Ide384c64cb7b5a9b2b3ce293b20e2be64da8ad69 --- libs/gui/LayerState.cpp | 8 +++++++- libs/gui/WindowInfo.cpp | 12 ++++++++---- libs/gui/include/gui/LayerState.h | 3 +++ libs/gui/include/gui/WindowInfo.h | 4 ++++ services/surfaceflinger/FrontEnd/LayerSnapshot.cpp | 1 + .../FrontEnd/LayerSnapshotBuilder.cpp | 2 ++ .../surfaceflinger/FrontEnd/RequestedLayerState.cpp | 3 +++ .../tests/unittests/LayerSnapshotTest.cpp | 21 +++++++++++++++++++++ 8 files changed, 49 insertions(+), 5 deletions(-) (limited to 'libs/gui/WindowInfo.cpp') diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp index 38fab9cdaa..7564c26316 100644 --- a/libs/gui/LayerState.cpp +++ b/libs/gui/LayerState.cpp @@ -199,7 +199,7 @@ status_t layer_state_t::write(Parcel& output) const SAFE_PARCEL(output.writeParcelable, trustedPresentationListener); SAFE_PARCEL(output.writeFloat, currentHdrSdrRatio); SAFE_PARCEL(output.writeFloat, desiredHdrSdrRatio); - SAFE_PARCEL(output.writeInt32, static_cast(cachingHint)) + SAFE_PARCEL(output.writeInt32, static_cast(cachingHint)); return NO_ERROR; } @@ -484,6 +484,12 @@ void layer_state_t::sanitize(int32_t permissions) { flags &= ~eLayerIsDisplayDecoration; ALOGE("Stripped attempt to set LayerIsDisplayDecoration in sanitize"); } + if ((mask & eCanOccludePresentation) && + !(permissions & Permission::ACCESS_SURFACE_FLINGER)) { + flags &= ~eCanOccludePresentation; + mask &= ~eCanOccludePresentation; + ALOGE("Stripped attempt to set eCanOccludePresentation in sanitize"); + } } if (what & layer_state_t::eInputInfoChanged) { diff --git a/libs/gui/WindowInfo.cpp b/libs/gui/WindowInfo.cpp index 95b2641f04..9429d2cc15 100644 --- a/libs/gui/WindowInfo.cpp +++ b/libs/gui/WindowInfo.cpp @@ -109,7 +109,8 @@ bool WindowInfo::operator==(const WindowInfo& info) const { info.inputConfig == inputConfig && info.displayId == displayId && info.replaceTouchableRegionWithCrop == replaceTouchableRegionWithCrop && info.applicationInfo == applicationInfo && info.layoutParamsType == layoutParamsType && - info.layoutParamsFlags == layoutParamsFlags; + info.layoutParamsFlags == layoutParamsFlags && + info.canOccludePresentation == canOccludePresentation; } status_t WindowInfo::writeToParcel(android::Parcel* parcel) const { @@ -158,8 +159,9 @@ status_t WindowInfo::writeToParcel(android::Parcel* parcel) const { parcel->write(touchableRegion) ?: parcel->writeBool(replaceTouchableRegionWithCrop) ?: parcel->writeStrongBinder(touchableRegionCropHandle.promote()) ?: - parcel->writeStrongBinder(windowToken); - parcel->writeStrongBinder(focusTransferTarget); + parcel->writeStrongBinder(windowToken) ?: + parcel->writeStrongBinder(focusTransferTarget) ?: + parcel->writeBool(canOccludePresentation); // clang-format on return status; } @@ -210,7 +212,8 @@ status_t WindowInfo::readFromParcel(const android::Parcel* parcel) { parcel->readBool(&replaceTouchableRegionWithCrop) ?: parcel->readNullableStrongBinder(&touchableRegionCropHandleSp) ?: parcel->readNullableStrongBinder(&windowToken) ?: - parcel->readNullableStrongBinder(&focusTransferTarget); + parcel->readNullableStrongBinder(&focusTransferTarget) ?: + parcel->readBool(&canOccludePresentation); // clang-format on @@ -273,6 +276,7 @@ std::ostream& operator<<(std::ostream& out, const WindowInfo& info) { << "ms, token=" << info.token.get() << ", touchOcclusionMode=" << ftl::enum_string(info.touchOcclusionMode) << "\n" << transform; + if (info.canOccludePresentation) out << " canOccludePresentation"; return out; } diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h index 6b8e8248b8..920310ea9e 100644 --- a/libs/gui/include/gui/LayerState.h +++ b/libs/gui/include/gui/LayerState.h @@ -161,6 +161,9 @@ struct layer_state_t { // See SurfaceView scaling behavior for more details. eIgnoreDestinationFrame = 0x400, eLayerIsRefreshRateIndicator = 0x800, // REFRESH_RATE_INDICATOR + // Sets a property on this layer indicating that its visible region should be considered + // when computing TrustedPresentation Thresholds. + eCanOccludePresentation = 0x1000, }; enum { diff --git a/libs/gui/include/gui/WindowInfo.h b/libs/gui/include/gui/WindowInfo.h index e72fd59147..32d60be612 100644 --- a/libs/gui/include/gui/WindowInfo.h +++ b/libs/gui/include/gui/WindowInfo.h @@ -246,6 +246,10 @@ struct WindowInfo : public Parcelable { // any other window. sp focusTransferTarget; + // Sets a property on this window indicating that its visible region should be considered when + // computing TrustedPresentation Thresholds. + bool canOccludePresentation = false; + void setInputConfig(ftl::Flags config, bool value); void addTouchableRegion(const Rect& region); diff --git a/services/surfaceflinger/FrontEnd/LayerSnapshot.cpp b/services/surfaceflinger/FrontEnd/LayerSnapshot.cpp index 38974a2f58..3ef9e69620 100644 --- a/services/surfaceflinger/FrontEnd/LayerSnapshot.cpp +++ b/services/surfaceflinger/FrontEnd/LayerSnapshot.cpp @@ -315,6 +315,7 @@ std::ostream& operator<<(std::ostream& out, const LayerSnapshot& obj) { if (obj.hasInputInfo()) { out << "\n input{" << "(" << obj.inputInfo.inputConfig.string() << ")"; + if (obj.inputInfo.canOccludePresentation) out << " canOccludePresentation"; if (obj.touchCropId != UNASSIGNED_LAYER_ID) out << " touchCropId=" << obj.touchCropId; if (obj.inputInfo.replaceTouchableRegionWithCrop) out << " replaceTouchableRegionWithCrop"; auto touchableRegion = obj.inputInfo.touchableRegion.getBounds(); diff --git a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp index 8df5d8c679..0966fe0496 100644 --- a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp +++ b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp @@ -1044,6 +1044,8 @@ void LayerSnapshotBuilder::updateInput(LayerSnapshot& snapshot, snapshot.inputInfo.touchOcclusionMode = requested.hasInputInfo() ? requested.windowInfoHandle->getInfo()->touchOcclusionMode : parentSnapshot.inputInfo.touchOcclusionMode; + snapshot.inputInfo.canOccludePresentation = parentSnapshot.inputInfo.canOccludePresentation || + (requested.flags & layer_state_t::eCanOccludePresentation); if (requested.dropInputMode == gui::DropInputMode::ALL || parentSnapshot.dropInputMode == gui::DropInputMode::ALL) { snapshot.dropInputMode = gui::DropInputMode::ALL; diff --git a/services/surfaceflinger/FrontEnd/RequestedLayerState.cpp b/services/surfaceflinger/FrontEnd/RequestedLayerState.cpp index 209df79f8e..2cf4c1b7dd 100644 --- a/services/surfaceflinger/FrontEnd/RequestedLayerState.cpp +++ b/services/surfaceflinger/FrontEnd/RequestedLayerState.cpp @@ -170,6 +170,9 @@ void RequestedLayerState::merge(const ResolvedComposerState& resolvedComposerSta if ((oldFlags ^ flags) & layer_state_t::eIgnoreDestinationFrame) { changes |= RequestedLayerState::Changes::Geometry; } + if ((oldFlags ^ flags) & layer_state_t::eCanOccludePresentation) { + changes |= RequestedLayerState::Changes::Input; + } } if (clientState.what & layer_state_t::eBufferChanged) { diff --git a/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp b/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp index 13b47ffa4c..3baa48d002 100644 --- a/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp +++ b/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp @@ -1237,4 +1237,25 @@ TEST_F(LayerSnapshotTest, NonVisibleLayerWithInput) { EXPECT_TRUE(foundInputLayer); } +TEST_F(LayerSnapshotTest, canOccludePresentation) { + setFlags(12, layer_state_t::eCanOccludePresentation, layer_state_t::eCanOccludePresentation); + LayerSnapshotBuilder::Args args{.root = mHierarchyBuilder.getHierarchy(), + .layerLifecycleManager = mLifecycleManager, + .includeMetadata = false, + .displays = mFrontEndDisplayInfos, + .displayChanges = false, + .globalShadowSettings = globalShadowSettings, + .supportsBlur = true, + .supportedLayerGenericMetadata = {}, + .genericLayerMetadataKeyMap = {}}; + UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER); + + EXPECT_EQ(getSnapshot(1)->inputInfo.canOccludePresentation, false); + + // ensure we can set the property on the window info for layer and all its children + EXPECT_EQ(getSnapshot(12)->inputInfo.canOccludePresentation, true); + EXPECT_EQ(getSnapshot(121)->inputInfo.canOccludePresentation, true); + EXPECT_EQ(getSnapshot(1221)->inputInfo.canOccludePresentation, true); +} + } // namespace android::surfaceflinger::frontend -- cgit v1.2.3-59-g8ed1b From aa2077038093f0283809adcb5c89d35674669d91 Mon Sep 17 00:00:00 2001 From: Vishnu Nair Date: Fri, 23 Feb 2024 22:22:18 +0000 Subject: Format WindowInfo dump Bug: 26029576 Change-Id: I234e811f367fca2aa6f678cea2a94885575df67e Test: presubmit --- libs/gui/WindowInfo.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'libs/gui/WindowInfo.cpp') diff --git a/libs/gui/WindowInfo.cpp b/libs/gui/WindowInfo.cpp index 9429d2cc15..86bf0ee745 100644 --- a/libs/gui/WindowInfo.cpp +++ b/libs/gui/WindowInfo.cpp @@ -262,8 +262,6 @@ void WindowInfoHandle::updateFrom(sp handle) { } std::ostream& operator<<(std::ostream& out, const WindowInfo& info) { - std::string transform; - info.transform.dump(transform, "transform", " "); out << "name=" << info.name << ", id=" << info.id << ", displayId=" << info.displayId << ", inputConfig=" << info.inputConfig.string() << ", alpha=" << info.alpha << ", frame=[" << info.frame.left << "," << info.frame.top << "][" << info.frame.right << "," @@ -274,9 +272,11 @@ std::ostream& operator<<(std::ostream& out, const WindowInfo& info) { << ", ownerUid=" << info.ownerUid.toString() << ", dispatchingTimeout=" << std::chrono::duration_cast(info.dispatchingTimeout).count() << "ms, token=" << info.token.get() - << ", touchOcclusionMode=" << ftl::enum_string(info.touchOcclusionMode) << "\n" - << transform; - if (info.canOccludePresentation) out << " canOccludePresentation"; + << ", touchOcclusionMode=" << ftl::enum_string(info.touchOcclusionMode); + if (info.canOccludePresentation) out << ", canOccludePresentation"; + std::string transform; + info.transform.dump(transform, "transform", " "); + out << "\n" << transform; return out; } -- cgit v1.2.3-59-g8ed1b