From cafd48c62c74e40d29c6a90e224aa3eb2fe7ab95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Go=CC=88llner?= Date: Mon, 20 Feb 2023 15:44:37 +0100 Subject: [PATCH] Fix unlocked split shade jumping back into place after over expanding The problem was that there were separate code paths for over-scroll/ over-expansion for split shade and non split shade. In this case the non split shade path was altering translation at the end of the drag, even when on split shade. To avoid adding even more "if split shade" code, removed the split shade over-scrolling code path/classes. There is only one overscroll code path now, with just 1 "if split" shade condition to decide which components to over expand. Test: Manually on device. See videos. Fixes: 270005948 Change-Id: Ia47aa30957260a9c2d1efd7e5f18536f76a2dcc9 --- .../NotificationPanelViewController.java | 16 +- .../shade/QuickSettingsController.java | 4 + .../transition/ShadeTransitionController.kt | 20 --- .../transition/SplitShadeOverScroller.kt | 167 ------------------ .../ShadeTransitionControllerTest.kt | 130 -------------- .../transition/SplitShadeOverScrollerTest.kt | 112 ------------ 6 files changed, 14 insertions(+), 435 deletions(-) delete mode 100644 packages/SystemUI/src/com/android/systemui/shade/transition/SplitShadeOverScroller.kt delete mode 100644 packages/SystemUI/tests/src/com/android/systemui/shade/transition/SplitShadeOverScrollerTest.kt diff --git a/packages/SystemUI/src/com/android/systemui/shade/NotificationPanelViewController.java b/packages/SystemUI/src/com/android/systemui/shade/NotificationPanelViewController.java index 2175a3358396f..1e0c49ebd3697 100644 --- a/packages/SystemUI/src/com/android/systemui/shade/NotificationPanelViewController.java +++ b/packages/SystemUI/src/com/android/systemui/shade/NotificationPanelViewController.java @@ -1914,7 +1914,6 @@ public final class NotificationPanelViewController implements Dumpable { // we want to perform an overshoot animation when flinging open final boolean addOverscroll = expand - && !mSplitShadeEnabled // Split shade has its own overscroll logic && mStatusBarStateController.getState() != KEYGUARD && mOverExpansion == 0.0f && vel >= 0; @@ -2590,9 +2589,14 @@ public final class NotificationPanelViewController implements Dumpable { return; } mOverExpansion = overExpansion; - // Translating the quick settings by half the overexpansion to center it in the background - // frame - mQsController.updateQsFrameTranslation(); + if (mSplitShadeEnabled) { + mQsController.setOverScrollAmount((int) overExpansion); + mScrimController.setNotificationsOverScrollAmount((int) overExpansion); + } else { + // Translating the quick settings by half the overexpansion to center it in the + // background frame + mQsController.updateQsFrameTranslation(); + } mNotificationStackScrollLayoutController.setOverExpansion(overExpansion); } @@ -3594,7 +3598,7 @@ public final class NotificationPanelViewController implements Dumpable { private void fling(float vel, boolean expand, float collapseSpeedUpFactor, boolean expandBecauseOfFalsing) { - float target = expand ? getMaxPanelHeight() : 0; + float target = expand ? getMaxPanelTransitionDistance() : 0; if (!expand) { setClosing(true); } @@ -3679,7 +3683,7 @@ public final class NotificationPanelViewController implements Dumpable { float maxPanelHeight = getMaxPanelTransitionDistance(); if (mHeightAnimator == null) { // Split shade has its own overscroll logic - if (mTracking && !mSplitShadeEnabled) { + if (mTracking) { float overExpansionPixels = Math.max(0, h - maxPanelHeight); setOverExpansionInternal(overExpansionPixels, true /* isFromGesture */); } diff --git a/packages/SystemUI/src/com/android/systemui/shade/QuickSettingsController.java b/packages/SystemUI/src/com/android/systemui/shade/QuickSettingsController.java index d041212d24c7a..fbceafe7520f3 100644 --- a/packages/SystemUI/src/com/android/systemui/shade/QuickSettingsController.java +++ b/packages/SystemUI/src/com/android/systemui/shade/QuickSettingsController.java @@ -803,6 +803,10 @@ public class QuickSettingsController { } } + void setOverScrollAmount(int overExpansion) { + mQs.setOverScrollAmount(overExpansion); + } + private void setOverScrolling(boolean overscrolling) { mStackScrollerOverscrolling = overscrolling; if (mQs != null) { diff --git a/packages/SystemUI/src/com/android/systemui/shade/transition/ShadeTransitionController.kt b/packages/SystemUI/src/com/android/systemui/shade/transition/ShadeTransitionController.kt index 07820ecd513ef..129d09ec3c140 100644 --- a/packages/SystemUI/src/com/android/systemui/shade/transition/ShadeTransitionController.kt +++ b/packages/SystemUI/src/com/android/systemui/shade/transition/ShadeTransitionController.kt @@ -43,8 +43,6 @@ constructor( shadeExpansionStateManager: ShadeExpansionStateManager, dumpManager: DumpManager, private val context: Context, - private val splitShadeOverScrollerFactory: SplitShadeOverScroller.Factory, - private val noOpOverScroller: NoOpOverScroller, private val scrimShadeTransitionController: ScrimShadeTransitionController, private val statusBarStateController: SysuiStatusBarStateController, ) { @@ -57,17 +55,6 @@ constructor( private var currentPanelState: Int? = null private var lastShadeExpansionChangeEvent: ShadeExpansionChangeEvent? = null - private val splitShadeOverScroller by lazy { - splitShadeOverScrollerFactory.create({ qs }, { notificationStackScrollLayoutController }) - } - private val shadeOverScroller: ShadeOverScroller - get() = - if (inSplitShade && isScreenUnlocked() && propertiesInitialized()) { - splitShadeOverScroller - } else { - noOpOverScroller - } - init { updateResources() configurationController.addCallback( @@ -89,21 +76,14 @@ constructor( private fun onPanelStateChanged(@PanelState state: Int) { currentPanelState = state - shadeOverScroller.onPanelStateChanged(state) scrimShadeTransitionController.onPanelStateChanged(state) } private fun onPanelExpansionChanged(event: ShadeExpansionChangeEvent) { lastShadeExpansionChangeEvent = event - shadeOverScroller.onDragDownAmountChanged(event.dragDownPxAmount) scrimShadeTransitionController.onPanelExpansionChanged(event) } - private fun propertiesInitialized() = - this::qs.isInitialized && - this::notificationPanelViewController.isInitialized && - this::notificationStackScrollLayoutController.isInitialized - private fun dump(pw: PrintWriter) { pw.println( """ diff --git a/packages/SystemUI/src/com/android/systemui/shade/transition/SplitShadeOverScroller.kt b/packages/SystemUI/src/com/android/systemui/shade/transition/SplitShadeOverScroller.kt deleted file mode 100644 index f95125f5cb6ca..0000000000000 --- a/packages/SystemUI/src/com/android/systemui/shade/transition/SplitShadeOverScroller.kt +++ /dev/null @@ -1,167 +0,0 @@ -/* - * Copyright (C) 2022 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.android.systemui.shade.transition - -import android.animation.Animator -import android.animation.ValueAnimator -import android.content.Context -import android.content.res.Configuration -import android.util.MathUtils -import com.android.internal.annotations.VisibleForTesting -import com.android.systemui.R -import com.android.systemui.animation.Interpolators -import com.android.systemui.dump.DumpManager -import com.android.systemui.plugins.qs.QS -import com.android.systemui.shade.PanelState -import com.android.systemui.shade.STATE_CLOSED -import com.android.systemui.shade.STATE_OPENING -import com.android.systemui.statusbar.notification.stack.NotificationStackScrollLayoutController -import com.android.systemui.statusbar.phone.ScrimController -import com.android.systemui.statusbar.policy.ConfigurationController -import dagger.assisted.Assisted -import dagger.assisted.AssistedFactory -import dagger.assisted.AssistedInject -import java.io.PrintWriter - -class SplitShadeOverScroller -@AssistedInject -constructor( - configurationController: ConfigurationController, - dumpManager: DumpManager, - private val context: Context, - private val scrimController: ScrimController, - @Assisted private val qSProvider: () -> QS, - @Assisted private val nsslControllerProvider: () -> NotificationStackScrollLayoutController -) : ShadeOverScroller { - - private var releaseOverScrollDuration = 0L - private var maxOverScrollAmount = 0 - private var previousOverscrollAmount = 0 - private var dragDownAmount: Float = 0f - @PanelState private var panelState: Int = STATE_CLOSED - - private var releaseOverScrollAnimator: Animator? = null - - private val qS: QS - get() = qSProvider() - - private val nsslController: NotificationStackScrollLayoutController - get() = nsslControllerProvider() - - init { - updateResources() - configurationController.addCallback( - object : ConfigurationController.ConfigurationListener { - override fun onConfigChanged(newConfig: Configuration?) { - updateResources() - } - }) - dumpManager.registerCriticalDumpable("SplitShadeOverScroller") { printWriter, _ -> - dump(printWriter) - } - } - - private fun updateResources() { - val resources = context.resources - maxOverScrollAmount = resources.getDimensionPixelSize(R.dimen.shade_max_over_scroll_amount) - releaseOverScrollDuration = - resources.getInteger(R.integer.lockscreen_shade_over_scroll_release_duration).toLong() - } - - override fun onPanelStateChanged(@PanelState newPanelState: Int) { - if (shouldReleaseOverscroll(previousState = panelState, newState = newPanelState)) { - releaseOverScroll() - } - panelState = newPanelState - } - - override fun onDragDownAmountChanged(newDragDownAmount: Float) { - if (dragDownAmount == newDragDownAmount) { - return - } - dragDownAmount = newDragDownAmount - if (shouldOverscroll()) { - overScroll(newDragDownAmount) - } - } - - private fun shouldOverscroll() = panelState == STATE_OPENING - - private fun shouldReleaseOverscroll(@PanelState previousState: Int, @PanelState newState: Int) = - previousState == STATE_OPENING && newState != STATE_OPENING - - private fun overScroll(dragDownAmount: Float) { - val overscrollAmount: Int = calculateOverscrollAmount(dragDownAmount) - applyOverscroll(overscrollAmount) - previousOverscrollAmount = overscrollAmount - } - - private fun calculateOverscrollAmount(dragDownAmount: Float): Int { - val fullHeight: Int = nsslController.height - val fullHeightProgress: Float = MathUtils.saturate(dragDownAmount / fullHeight) - return (fullHeightProgress * maxOverScrollAmount).toInt() - } - - private fun applyOverscroll(overscrollAmount: Int) { - qS.setOverScrollAmount(overscrollAmount) - scrimController.setNotificationsOverScrollAmount(overscrollAmount) - nsslController.setOverScrollAmount(overscrollAmount) - } - - private fun releaseOverScroll() { - val animator = ValueAnimator.ofInt(previousOverscrollAmount, 0) - animator.addUpdateListener { - val overScrollAmount = it.animatedValue as Int - qS.setOverScrollAmount(overScrollAmount) - scrimController.setNotificationsOverScrollAmount(overScrollAmount) - nsslController.setOverScrollAmount(overScrollAmount) - } - animator.interpolator = Interpolators.STANDARD - animator.duration = releaseOverScrollDuration - animator.start() - releaseOverScrollAnimator = animator - previousOverscrollAmount = 0 - } - - @VisibleForTesting - internal fun finishAnimations() { - releaseOverScrollAnimator?.end() - releaseOverScrollAnimator = null - } - - private fun dump(pw: PrintWriter) { - pw.println( - """ - SplitShadeOverScroller: - Resources: - releaseOverScrollDuration: $releaseOverScrollDuration - maxOverScrollAmount: $maxOverScrollAmount - State: - previousOverscrollAmount: $previousOverscrollAmount - dragDownAmount: $dragDownAmount - panelState: $panelState - """.trimIndent()) - } - - @AssistedFactory - fun interface Factory { - fun create( - qSProvider: () -> QS, - nsslControllerProvider: () -> NotificationStackScrollLayoutController - ): SplitShadeOverScroller - } -} diff --git a/packages/SystemUI/tests/src/com/android/systemui/shade/transition/ShadeTransitionControllerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/shade/transition/ShadeTransitionControllerTest.kt index 7cac854c08534..d5a1f804e6a0b 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/shade/transition/ShadeTransitionControllerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/shade/transition/ShadeTransitionControllerTest.kt @@ -2,37 +2,24 @@ package com.android.systemui.shade.transition import android.testing.AndroidTestingRunner import androidx.test.filters.SmallTest -import com.android.systemui.R import com.android.systemui.SysuiTestCase import com.android.systemui.dump.DumpManager -import com.android.systemui.plugins.qs.QS -import com.android.systemui.shade.NotificationPanelViewController import com.android.systemui.shade.STATE_OPENING import com.android.systemui.shade.ShadeExpansionChangeEvent import com.android.systemui.shade.ShadeExpansionStateManager -import com.android.systemui.statusbar.StatusBarState import com.android.systemui.statusbar.SysuiStatusBarStateController -import com.android.systemui.statusbar.notification.stack.NotificationStackScrollLayoutController import com.android.systemui.statusbar.policy.FakeConfigurationController import org.junit.Before import org.junit.Test import org.junit.runner.RunWith import org.mockito.Mock -import org.mockito.Mockito.reset import org.mockito.Mockito.verify -import org.mockito.Mockito.verifyZeroInteractions -import org.mockito.Mockito.`when` as whenever import org.mockito.MockitoAnnotations @RunWith(AndroidTestingRunner::class) @SmallTest class ShadeTransitionControllerTest : SysuiTestCase() { - @Mock private lateinit var npvc: NotificationPanelViewController - @Mock private lateinit var nsslController: NotificationStackScrollLayoutController - @Mock private lateinit var qs: QS - @Mock private lateinit var noOpOverScroller: NoOpOverScroller - @Mock private lateinit var splitShadeOverScroller: SplitShadeOverScroller @Mock private lateinit var scrimShadeTransitionController: ScrimShadeTransitionController @Mock private lateinit var dumpManager: DumpManager @Mock private lateinit var statusBarStateController: SysuiStatusBarStateController @@ -52,119 +39,19 @@ class ShadeTransitionControllerTest : SysuiTestCase() { shadeExpansionStateManager, dumpManager, context, - splitShadeOverScrollerFactory = { _, _ -> splitShadeOverScroller }, - noOpOverScroller, scrimShadeTransitionController, statusBarStateController, ) - - // Resetting as they are notified upon initialization. - reset(noOpOverScroller, splitShadeOverScroller) - } - - @Test - fun onPanelExpansionChanged_inSplitShade_forwardsToSplitShadeOverScroller() { - initLateProperties() - enableSplitShade() - - startPanelExpansion() - - verify(splitShadeOverScroller).onPanelStateChanged(STATE_OPENING) - verify(splitShadeOverScroller).onDragDownAmountChanged(DEFAULT_DRAG_DOWN_AMOUNT) - verifyZeroInteractions(noOpOverScroller) - } - - @Test - fun onPanelStateChanged_inSplitShade_propertiesNotInitialized_forwardsToNoOpOverScroller() { - enableSplitShade() - - startPanelExpansion() - - verify(noOpOverScroller).onPanelStateChanged(STATE_OPENING) - verify(noOpOverScroller).onDragDownAmountChanged(DEFAULT_DRAG_DOWN_AMOUNT) - verifyZeroInteractions(splitShadeOverScroller) - } - - @Test - fun onPanelStateChanged_inSplitShade_onKeyguard_forwardsToNoOpOverScroller() { - initLateProperties() - enableSplitShade() - setOnKeyguard() - - startPanelExpansion() - - verify(noOpOverScroller).onPanelStateChanged(STATE_OPENING) - verify(noOpOverScroller).onDragDownAmountChanged(DEFAULT_DRAG_DOWN_AMOUNT) - verifyZeroInteractions(splitShadeOverScroller) - } - - @Test - fun onPanelStateChanged_inSplitShade_onLockedShade_forwardsToNoOpOverScroller() { - initLateProperties() - enableSplitShade() - setOnLockedShade() - - startPanelExpansion() - - verify(noOpOverScroller).onPanelStateChanged(STATE_OPENING) - verify(noOpOverScroller).onDragDownAmountChanged(DEFAULT_DRAG_DOWN_AMOUNT) - verifyZeroInteractions(splitShadeOverScroller) - } - - @Test - fun onPanelExpansionChanged_inSplitShade_onUnlockedShade_forwardsToSplitShadeOverScroller() { - initLateProperties() - enableSplitShade() - setOnUnlockedShade() - - startPanelExpansion() - - verify(splitShadeOverScroller).onPanelStateChanged(STATE_OPENING) - verify(splitShadeOverScroller).onDragDownAmountChanged(DEFAULT_DRAG_DOWN_AMOUNT) - verifyZeroInteractions(noOpOverScroller) - } - - @Test - fun onPanelStateChanged_notInSplitShade_forwardsToNoOpOverScroller() { - initLateProperties() - disableSplitShade() - - startPanelExpansion() - - verify(noOpOverScroller).onPanelStateChanged(STATE_OPENING) - verify(noOpOverScroller).onDragDownAmountChanged(DEFAULT_DRAG_DOWN_AMOUNT) - verifyZeroInteractions(splitShadeOverScroller) } @Test fun onPanelStateChanged_forwardsToScrimTransitionController() { - initLateProperties() - startPanelExpansion() verify(scrimShadeTransitionController).onPanelStateChanged(STATE_OPENING) verify(scrimShadeTransitionController).onPanelExpansionChanged(DEFAULT_EXPANSION_EVENT) } - private fun initLateProperties() { - controller.qs = qs - controller.notificationStackScrollLayoutController = nsslController - controller.notificationPanelViewController = npvc - } - - private fun disableSplitShade() { - setSplitShadeEnabled(false) - } - - private fun enableSplitShade() { - setSplitShadeEnabled(true) - } - - private fun setSplitShadeEnabled(enabled: Boolean) { - overrideResource(R.bool.config_use_split_notification_shade, enabled) - configurationController.notifyConfigurationChanged() - } - private fun startPanelExpansion() { shadeExpansionStateManager.onPanelExpansionChanged( DEFAULT_EXPANSION_EVENT.fraction, @@ -174,23 +61,6 @@ class ShadeTransitionControllerTest : SysuiTestCase() { ) } - private fun setOnKeyguard() { - setShadeState(StatusBarState.KEYGUARD) - } - - private fun setOnLockedShade() { - setShadeState(StatusBarState.SHADE_LOCKED) - } - - private fun setOnUnlockedShade() { - setShadeState(StatusBarState.SHADE) - } - - private fun setShadeState(state: Int) { - whenever(statusBarStateController.state).thenReturn(state) - whenever(statusBarStateController.currentOrUpcomingState).thenReturn(state) - } - companion object { private const val DEFAULT_DRAG_DOWN_AMOUNT = 123f private val DEFAULT_EXPANSION_EVENT = diff --git a/packages/SystemUI/tests/src/com/android/systemui/shade/transition/SplitShadeOverScrollerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/shade/transition/SplitShadeOverScrollerTest.kt deleted file mode 100644 index 0e48b4835dfe2..0000000000000 --- a/packages/SystemUI/tests/src/com/android/systemui/shade/transition/SplitShadeOverScrollerTest.kt +++ /dev/null @@ -1,112 +0,0 @@ -package com.android.systemui.shade.transition - -import android.testing.AndroidTestingRunner -import android.testing.TestableLooper -import androidx.test.filters.SmallTest -import com.android.systemui.R -import com.android.systemui.SysuiTestCase -import com.android.systemui.dump.DumpManager -import com.android.systemui.plugins.qs.QS -import com.android.systemui.shade.STATE_CLOSED -import com.android.systemui.shade.STATE_OPEN -import com.android.systemui.shade.STATE_OPENING -import com.android.systemui.statusbar.notification.stack.NotificationStackScrollLayoutController -import com.android.systemui.statusbar.phone.ScrimController -import com.android.systemui.statusbar.policy.FakeConfigurationController -import org.junit.Before -import org.junit.Test -import org.junit.runner.RunWith -import org.mockito.Mock -import org.mockito.Mockito.atLeastOnce -import org.mockito.Mockito.verify -import org.mockito.Mockito.verifyZeroInteractions -import org.mockito.Mockito.`when` as whenever -import org.mockito.MockitoAnnotations - -@RunWith(AndroidTestingRunner::class) -@TestableLooper.RunWithLooper(setAsMainLooper = true) -@SmallTest -class SplitShadeOverScrollerTest : SysuiTestCase() { - - @Mock private lateinit var dumpManager: DumpManager - @Mock private lateinit var scrimController: ScrimController - @Mock private lateinit var qs: QS - @Mock private lateinit var nsslController: NotificationStackScrollLayoutController - - private val configurationController = FakeConfigurationController() - private lateinit var overScroller: SplitShadeOverScroller - - @Before - fun setUp() { - MockitoAnnotations.initMocks(this) - - whenever(nsslController.height).thenReturn(1000) - overScroller = - SplitShadeOverScroller( - configurationController, - dumpManager, - context, - scrimController, - { qs }, - { nsslController }) - } - - @Test - fun onDragDownAmountChanged_panelOpening_overScrolls_basedOnHeightAndMaxAmount() { - val maxOverScrollAmount = 50 - val dragDownAmount = 100f - overrideResource(R.dimen.shade_max_over_scroll_amount, maxOverScrollAmount) - configurationController.notifyConfigurationChanged() - - overScroller.onPanelStateChanged(STATE_OPENING) - overScroller.onDragDownAmountChanged(dragDownAmount) - - val expectedOverScrollAmount = - (dragDownAmount / nsslController.height * maxOverScrollAmount).toInt() - verify(qs).setOverScrollAmount(expectedOverScrollAmount) - verify(nsslController).setOverScrollAmount(expectedOverScrollAmount) - verify(scrimController).setNotificationsOverScrollAmount(expectedOverScrollAmount) - } - - @Test - fun onDragDownAmountChanged_panelClosed_doesNotOverScroll() { - overScroller.onPanelStateChanged(STATE_CLOSED) - overScroller.onDragDownAmountChanged(100f) - - verifyZeroInteractions(qs, scrimController, nsslController) - } - - @Test - fun onDragDownAmountChanged_panelOpen_doesNotOverScroll() { - overScroller.onPanelStateChanged(STATE_OPEN) - overScroller.onDragDownAmountChanged(100f) - - verifyZeroInteractions(qs, scrimController, nsslController) - } - - @Test - fun onPanelStateChanged_opening_thenOpen_releasesOverScroll() { - overScroller.onPanelStateChanged(STATE_OPENING) - overScroller.onDragDownAmountChanged(100f) - - overScroller.onPanelStateChanged(STATE_OPEN) - overScroller.finishAnimations() - - verify(qs, atLeastOnce()).setOverScrollAmount(0) - verify(scrimController, atLeastOnce()).setNotificationsOverScrollAmount(0) - verify(nsslController, atLeastOnce()).setOverScrollAmount(0) - } - - @Test - fun onPanelStateChanged_opening_thenClosed_releasesOverScroll() { - overScroller.onPanelStateChanged(STATE_OPENING) - overScroller.onDragDownAmountChanged(100f) - - overScroller.onPanelStateChanged(STATE_CLOSED) - overScroller.finishAnimations() - - verify(qs, atLeastOnce()).setOverScrollAmount(0) - verify(scrimController, atLeastOnce()).setNotificationsOverScrollAmount(0) - verify(nsslController, atLeastOnce()).setOverScrollAmount(0) - } -}