From a3dbe509c1d4a46efc71ed8275b65202c9606056 Mon Sep 17 00:00:00 2001 From: Jeff DeCew Date: Fri, 26 May 2023 13:37:05 -0400 Subject: [PATCH 1/2] Clear mTouchedView when a snap-back animation ends Bug: 259529672 Test: dumpsysui SwipeHelper Change-Id: Iff38e3721a4abfa613974e6df32cb178c3a73fb9 --- packages/SystemUI/src/com/android/systemui/SwipeHelper.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/SystemUI/src/com/android/systemui/SwipeHelper.java b/packages/SystemUI/src/com/android/systemui/SwipeHelper.java index c30a2146436c6..ff9dfc697b678 100644 --- a/packages/SystemUI/src/com/android/systemui/SwipeHelper.java +++ b/packages/SystemUI/src/com/android/systemui/SwipeHelper.java @@ -548,6 +548,10 @@ public class SwipeHelper implements Gefingerpoken, Dumpable { if (!cancelled) { updateSwipeProgressFromOffset(animView, canBeDismissed); resetSwipeOfView(animView); + // Clear the snapped view after success, assuming it's not being swiped now + if (animView == mTouchedView && !mIsSwiping) { + mTouchedView = null; + } } onChildSnappedBack(animView, targetLeft); }); From 27c8877af3f60994803f622777880893f196aa3b Mon Sep 17 00:00:00 2001 From: Jeff DeCew Date: Fri, 26 May 2023 13:37:05 -0400 Subject: [PATCH 2/2] Aggressively reset SwipeHelper state * Resetting just swipe (used internally as resetSwipeState) when snap was also in progress will now clear the mSnappingChild field, which also ensures snapChildIfNeeded() doesn't return early. * Resetting all state (used externally as resetTouchState) will always clear mSnappingChild and will cancel snap animations and reset state. * NSSL will now also resetTouchState whenever the notifications are hidden or unhidden. These changes all assume that the mTouchedView still pointed to the view which was misaligned in order to fix the bug. If the mTouchedView does NOT point to the misaligned view, then the fix for this bug is going to require one of two things: 1) we could track all active swipe/snap/dismiss views, rather than just the latest one, so that we can appropriately tear down all state. This is a major project. 2) we could, when notifications become visible, iterate all rows recursively and reset all their swipe translations naively. I'm worried about possible jank this could cause. Fixes: 259529672 Test: dumpsysui SwipeHelper Test: atest NotificationStackScrollLayoutTest Change-Id: I53d8e217e7f133388943e07be088d63623c4b735 --- .../src/com/android/systemui/SwipeHelper.java | 36 ++++++++++++++++--- .../stack/NotificationStackScrollLayout.java | 3 +- .../NotificationStackScrollLayoutTest.java | 12 +++---- 3 files changed, 39 insertions(+), 12 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/SwipeHelper.java b/packages/SystemUI/src/com/android/systemui/SwipeHelper.java index ff9dfc697b678..8af92ce4bcb11 100644 --- a/packages/SystemUI/src/com/android/systemui/SwipeHelper.java +++ b/packages/SystemUI/src/com/android/systemui/SwipeHelper.java @@ -817,13 +817,39 @@ public class SwipeHelper implements Gefingerpoken, Dumpable { } } - public void resetSwipeState() { - View swipedView = getSwipedView(); + private void resetSwipeState() { + resetSwipeStates(/* resetAll= */ false); + } + + public void resetTouchState() { + resetSwipeStates(/* resetAll= */ true); + } + + /** This method resets the swipe state, and if `resetAll` is true, also resets the snap state */ + private void resetSwipeStates(boolean resetAll) { + final View touchedView = mTouchedView; + final boolean wasSnapping = mSnappingChild; + final boolean wasSwiping = mIsSwiping; mTouchedView = null; mIsSwiping = false; - if (swipedView != null) { - snapChildIfNeeded(swipedView, false, 0); - onChildSnappedBack(swipedView, 0); + // If we were swiping, then we resetting swipe requires resetting everything. + resetAll |= wasSwiping; + if (resetAll) { + mSnappingChild = false; + } + if (touchedView == null) return; // No view to reset visually + // When snap needs to be reset, first thing is to cancel any translation animation + final boolean snapNeedsReset = resetAll && wasSnapping; + if (snapNeedsReset) { + cancelTranslateAnimation(touchedView); + } + // actually reset the view to default state + if (resetAll) { + snapChildIfNeeded(touchedView, false, 0); + } + // report if a swipe or snap was reset. + if (wasSwiping || snapNeedsReset) { + onChildSnappedBack(touchedView, 0); } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java index b81cb2be1c4d6..3db670e364222 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java @@ -4395,6 +4395,7 @@ public class NotificationStackScrollLayout extends ViewGroup implements Dumpable boolean nowHiddenAtAll = mAmbientState.isHiddenAtAll(); if (nowFullyHidden != wasFullyHidden) { updateVisibility(); + mSwipeHelper.resetTouchState(); } if (!wasHiddenAtAll && nowHiddenAtAll) { resetExposedMenuView(true /* animate */, true /* animate */); @@ -5850,7 +5851,7 @@ public class NotificationStackScrollLayout extends ViewGroup implements Dumpable } private void cancelActiveSwipe() { - mSwipeHelper.resetSwipeState(); + mSwipeHelper.resetTouchState(); updateContinuousShadowDrawing(); } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutTest.java index a4ee349f5b710..9f24a4798e458 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutTest.java @@ -794,7 +794,7 @@ public class NotificationStackScrollLayoutTest extends SysuiTestCase { } @Test - public void onShadeClosesWithAnimationWillResetSwipeState() { + public void onShadeClosesWithAnimationWillResetTouchState() { // GIVEN shade is expanded mStackScroller.setIsExpanded(true); clearInvocations(mNotificationSwipeHelper); @@ -804,12 +804,12 @@ public class NotificationStackScrollLayoutTest extends SysuiTestCase { mStackScroller.setIsExpanded(false); mStackScroller.onExpansionStopped(); - // VERIFY swipe is reset - verify(mNotificationSwipeHelper).resetSwipeState(); + // VERIFY touch is reset + verify(mNotificationSwipeHelper).resetTouchState(); } @Test - public void onShadeClosesWithoutAnimationWillResetSwipeState() { + public void onShadeClosesWithoutAnimationWillResetTouchState() { // GIVEN shade is expanded mStackScroller.setIsExpanded(true); clearInvocations(mNotificationSwipeHelper); @@ -817,8 +817,8 @@ public class NotificationStackScrollLayoutTest extends SysuiTestCase { // WHEN closing the shade without the animation mStackScroller.setIsExpanded(false); - // VERIFY swipe is reset - verify(mNotificationSwipeHelper).resetSwipeState(); + // VERIFY touch is reset + verify(mNotificationSwipeHelper).resetTouchState(); } @Test