From f630a829554b6288e36a6e99eb85c0d89c3e49df Mon Sep 17 00:00:00 2001 From: Selim Cinek Date: Mon, 22 Jun 2020 11:26:09 -0700 Subject: [PATCH 1/2] Fixed some cases where we were animating Qs even though we shouldn't When collapsing QS, the player should remain in the qs panel, since notifications are invisible, which would look odd. Similarly, when unlocking using fingerprint, we should keep the media experience where it is. Fixes: 157553726 Fixes: 156105562 Test: expand qs on lock, directly collapse Change-Id: I1abd1561e878ad57b8cdc435bfacd40897616ff1 --- .../systemui/media/MediaHierarchyManager.kt | 39 +++++++++++++++++++ .../NotificationPanelViewController.java | 9 +++++ 2 files changed, 48 insertions(+) diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaHierarchyManager.kt b/packages/SystemUI/src/com/android/systemui/media/MediaHierarchyManager.kt index 3266074eba7de..c41e6104833e8 100644 --- a/packages/SystemUI/src/com/android/systemui/media/MediaHierarchyManager.kt +++ b/packages/SystemUI/src/com/android/systemui/media/MediaHierarchyManager.kt @@ -139,6 +139,18 @@ class MediaHierarchyManager @Inject constructor( } } + /** + * Is the shade currently collapsing from the expanded qs? If we're on the lockscreen and in qs, + * we wouldn't want to transition in that case. + */ + var collapsingShadeFromQS: Boolean = false + set(value) { + if (field != value) { + field = value + updateDesiredLocation(forceNoAnimation = true) + } + } + /** * Are location changes currently blocked? */ @@ -160,6 +172,19 @@ class MediaHierarchyManager @Inject constructor( } } + /** + * Are we currently fullyAwake + */ + private var fullyAwake: Boolean = false + set(value) { + if (field != value) { + field = value + if (value) { + updateDesiredLocation(forceNoAnimation = true) + } + } + } + /** * Is the doze animation currently Running */ @@ -206,10 +231,12 @@ class MediaHierarchyManager @Inject constructor( override fun onStartedGoingToSleep() { goingToSleep = true + fullyAwake = false } override fun onFinishedWakingUp() { goingToSleep = false + fullyAwake = true } override fun onStartedWakingUp() { @@ -531,6 +558,18 @@ class MediaHierarchyManager @Inject constructor( !statusBarStateController.isDozing) { return LOCATION_QS } + if (location == LOCATION_LOCKSCREEN && desiredLocation == LOCATION_QS && + collapsingShadeFromQS) { + // When collapsing on the lockscreen, we want to remain in QS + return LOCATION_QS + } + if (location != LOCATION_LOCKSCREEN && desiredLocation == LOCATION_LOCKSCREEN + && !fullyAwake) { + // When unlocking from dozing / while waking up, the media shouldn't be transitioning + // in an animated way. Let's keep it in the lockscreen until we're fully awake and + // reattach it without an animation + return LOCATION_LOCKSCREEN + } return location } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationPanelViewController.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationPanelViewController.java index e720d820fd76f..9e000f4190e97 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationPanelViewController.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationPanelViewController.java @@ -443,6 +443,7 @@ public class NotificationPanelViewController extends PanelViewController { */ private boolean mDelayShowingKeyguardStatusBar; + private boolean mAnimatingQS; private int mOldLayoutDirection; private View.AccessibilityDelegate mAccessibilityDelegate = new View.AccessibilityDelegate() { @@ -1860,6 +1861,7 @@ public class NotificationPanelViewController extends PanelViewController { @Override public void onAnimationEnd(Animator animation) { + mAnimatingQS = false; notifyExpandingFinished(); mNotificationStackScroller.resetCheckSnoozeLeavebehind(); mQsExpansionAnimator = null; @@ -1868,6 +1870,9 @@ public class NotificationPanelViewController extends PanelViewController { } } }); + // Let's note that we're animating QS. Moving the animator here will cancel it immediately, + // so we need a separate flag. + mAnimatingQS = true; animator.start(); mQsExpansionAnimator = animator; mQsAnimatorExpand = expanding; @@ -2220,6 +2225,9 @@ public class NotificationPanelViewController extends PanelViewController { mNotificationStackScroller.onExpansionStarted(); mIsExpanding = true; mQsExpandedWhenExpandingStarted = mQsFullyExpanded; + mMediaHierarchyManager.setCollapsingShadeFromQS(mQsExpandedWhenExpandingStarted && + /* We also start expanding when flinging closed Qs. Let's exclude that */ + !mAnimatingQS); if (mQsExpanded) { onQsExpansionStarted(); } @@ -2236,6 +2244,7 @@ public class NotificationPanelViewController extends PanelViewController { mHeadsUpManager.onExpandingFinished(); mConversationNotificationManager.onNotificationPanelExpandStateChanged(isFullyCollapsed()); mIsExpanding = false; + mMediaHierarchyManager.setCollapsingShadeFromQS(false); if (isFullyCollapsed()) { DejankUtils.postAfterTraversal(new Runnable() { @Override From 523022499ec6bb497a83a190c8d3cc287cff11c6 Mon Sep 17 00:00:00 2001 From: Selim Cinek Date: Mon, 22 Jun 2020 19:31:45 -0700 Subject: [PATCH 2/2] Fix cliping when the media player is fully clipped off Previously we didn't clip when at height 0, but we still need to as very often the view ill have height 0. The clipRect should always be up to date. Also, removing inactive players when resumption is off to avoid this completely Fixes: 159414197 Test: turn off resumption, swipe away media, observe no weird clipping Change-Id: I30a2fa4fe19d142c8a6472f8368d64e6ac4074d8 --- .../systemui/media/MediaCarouselController.kt | 12 ++++++++++-- .../systemui/util/animation/TransitionLayout.kt | 11 +++-------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaCarouselController.kt b/packages/SystemUI/src/com/android/systemui/media/MediaCarouselController.kt index db45a5fff4991..e2215d57a0946 100644 --- a/packages/SystemUI/src/com/android/systemui/media/MediaCarouselController.kt +++ b/packages/SystemUI/src/com/android/systemui/media/MediaCarouselController.kt @@ -15,6 +15,7 @@ import com.android.systemui.plugins.FalsingManager import com.android.systemui.qs.PageIndicator import com.android.systemui.statusbar.notification.VisualStabilityManager import com.android.systemui.statusbar.policy.ConfigurationController +import com.android.systemui.util.Utils import com.android.systemui.util.animation.UniqueObjectHostView import com.android.systemui.util.animation.requiresRemeasuring import com.android.systemui.util.concurrency.DelayableExecutor @@ -150,8 +151,15 @@ class MediaCarouselController @Inject constructor( mediaManager.addListener(object : MediaDataManager.Listener { override fun onMediaDataLoaded(key: String, oldKey: String?, data: MediaData) { oldKey?.let { mediaData.remove(it) } - mediaData.put(key, data) - addOrUpdatePlayer(key, oldKey, data) + if (!data.active && !Utils.useMediaResumption(context)) { + // This view is inactive, let's remove this! This happens e.g when dismissing / + // timing out a view. We still have the data around because resumption could + // be on, but we should save the resources and release this. + onMediaDataRemoved(key) + } else { + mediaData.put(key, data) + addOrUpdatePlayer(key, oldKey, data) + } } override fun onMediaDataRemoved(key: String) { diff --git a/packages/SystemUI/src/com/android/systemui/util/animation/TransitionLayout.kt b/packages/SystemUI/src/com/android/systemui/util/animation/TransitionLayout.kt index e5b126d7ff7f2..3c0a23aa2eca5 100644 --- a/packages/SystemUI/src/com/android/systemui/util/animation/TransitionLayout.kt +++ b/packages/SystemUI/src/com/android/systemui/util/animation/TransitionLayout.kt @@ -150,15 +150,10 @@ class TransitionLayout @JvmOverloads constructor( } override fun dispatchDraw(canvas: Canvas?) { - val clip = !boundsRect.isEmpty - if (clip) { - canvas?.save() - canvas?.clipRect(boundsRect) - } + canvas?.save() + canvas?.clipRect(boundsRect) super.dispatchDraw(canvas) - if (clip) { - canvas?.restore() - } + canvas?.restore() } private fun updateBounds() {