Fixing brightness slider jank in split shade

This jank was caused by two sliders - one in QS and one in QQS - being updated at the same time.
When sliding one, the other one register changes in brightness and is trying to animate itself into the new value. Because they share one mirror, seems like mirror is getting conflicting signals about current values - one mirror animates, the other doesn't.
Another issue this fixes is changing brightness significantly and quickly switching to collapsed or expanded QS - the animation would not end till new slider shows up and slider would be moving, even though the other slider was not moving.
Also renaming BrightnessSlider to BrightnessSliderController (because it seems to be naming convention for controllers) - that makes this change so wide-reaching.

Fixes: 196523321
Test: open split shade -> slide finger (not tap) on top of slider to change brightness -> lift finger and see no jank
Change-Id: Ie11e3670f1464e5f1c85e221ce09b6ebf6dd4d0c

packages/SystemUI/src/com/android/systemui/qs/QSPanel.java

@@ -41,7 +41,7 @@ import com.android.internal.widget.RemeasuringLinearLayout;
 import com.android.systemui.R;
 import com.android.systemui.plugins.qs.DetailAdapter;
 import com.android.systemui.plugins.qs.QSTile;
-import com.android.systemui.settings.brightness.BrightnessSlider;
+import com.android.systemui.settings.brightness.BrightnessSliderController;
 import com.android.systemui.statusbar.policy.BrightnessMirrorController;
 import com.android.systemui.tuner.TunerService;
 import com.android.systemui.tuner.TunerService.Tunable;
@@ -69,7 +69,7 @@ public class QSPanel extends LinearLayout implements Tunable {
     @Nullable
     protected View mBrightnessView;
     @Nullable
-    protected BrightnessSlider mToggleSliderController;
+    protected BrightnessSliderController mToggleSliderController;
 
     private final H mHandler = new H();
     /** Whether or not the QS media player feature is enabled. */

packages/SystemUI/src/com/android/systemui/qs/QSPanelController.java

@@ -41,7 +41,7 @@ import com.android.systemui.qs.dagger.QSScope;
 import com.android.systemui.qs.logging.QSLogger;
 import com.android.systemui.settings.brightness.BrightnessController;
 import com.android.systemui.settings.brightness.BrightnessMirrorHandler;
-import com.android.systemui.settings.brightness.BrightnessSlider;
+import com.android.systemui.settings.brightness.BrightnessSliderController;
 import com.android.systemui.statusbar.CommandQueue;
 import com.android.systemui.statusbar.policy.BrightnessMirrorController;
 import com.android.systemui.tuner.TunerService;
@@ -63,7 +63,7 @@ public class QSPanelController extends QSPanelControllerBase<QSPanel> {
     private final FalsingManager mFalsingManager;
     private final CommandQueue mCommandQueue;
     private final BrightnessController mBrightnessController;
-    private final BrightnessSlider mBrightnessSlider;
+    private final BrightnessSliderController mBrightnessSliderController;
     private final BrightnessMirrorHandler mBrightnessMirrorHandler;
 
     private boolean mGridContentVisible = true;
@@ -99,8 +99,8 @@ public class QSPanelController extends QSPanelControllerBase<QSPanel> {
             QSTileRevealController.Factory qsTileRevealControllerFactory,
             DumpManager dumpManager, MetricsLogger metricsLogger, UiEventLogger uiEventLogger,
             QSLogger qsLogger, BrightnessController.Factory brightnessControllerFactory,
-            BrightnessSlider.Factory brightnessSliderFactory, FalsingManager falsingManager,
-            CommandQueue commandQueue) {
+            BrightnessSliderController.Factory brightnessSliderFactory,
+            FalsingManager falsingManager, CommandQueue commandQueue) {
         super(view, qstileHost, qsCustomizerController, usingMediaPlayer, mediaHost,
                 metricsLogger, uiEventLogger, qsLogger, dumpManager);
         mQsSecurityFooter = qsSecurityFooter;
@@ -111,10 +111,10 @@ public class QSPanelController extends QSPanelControllerBase<QSPanel> {
         mCommandQueue = commandQueue;
         mQsSecurityFooter.setHostEnvironment(qstileHost);
 
-        mBrightnessSlider = brightnessSliderFactory.create(getContext(), mView);
-        mView.setBrightnessView(mBrightnessSlider.getRootView());
+        mBrightnessSliderController = brightnessSliderFactory.create(getContext(), mView);
+        mView.setBrightnessView(mBrightnessSliderController.getRootView());
 
-        mBrightnessController = brightnessControllerFactory.create(mBrightnessSlider);
+        mBrightnessController = brightnessControllerFactory.create(mBrightnessSliderController);
         mBrightnessMirrorHandler = new BrightnessMirrorHandler(mBrightnessController);
     }
 
@@ -125,7 +125,7 @@ public class QSPanelController extends QSPanelControllerBase<QSPanel> {
         mMediaHost.setShowsOnlyActiveMedia(false);
         mMediaHost.init(MediaHierarchyManager.LOCATION_QS);
         mQsCustomizerController.init();
-        mBrightnessSlider.init();
+        mBrightnessSliderController.init();
     }
 
     @Override

packages/SystemUI/src/com/android/systemui/qs/QuickQSBrightnessController.kt

@@ -18,7 +18,7 @@ package com.android.systemui.qs
 
 import androidx.annotation.VisibleForTesting
 import com.android.systemui.settings.brightness.BrightnessController
-import com.android.systemui.settings.brightness.BrightnessSlider
+import com.android.systemui.settings.brightness.BrightnessSliderController
 import com.android.systemui.settings.brightness.MirroredBrightnessController
 import com.android.systemui.statusbar.policy.BrightnessMirrorController
 import javax.inject.Inject
@@ -33,10 +33,11 @@ class QuickQSBrightnessController @VisibleForTesting constructor(
 
     @Inject constructor(
         brightnessControllerFactory: BrightnessController.Factory,
-        brightnessSliderFactory: BrightnessSlider.Factory,
+        brightnessSliderControllerFactory: BrightnessSliderController.Factory,
         quickQSPanel: QuickQSPanel
     ) : this(brightnessControllerFactory = {
-            val slider = brightnessSliderFactory.create(quickQSPanel.context, quickQSPanel)
+            val slider = brightnessSliderControllerFactory.create(quickQSPanel.context,
+                    quickQSPanel)
             slider.init()
             quickQSPanel.setBrightnessView(slider.rootView)
             brightnessControllerFactory.create(slider)

packages/SystemUI/src/com/android/systemui/settings/brightness/BrightnessController.java

@@ -51,8 +51,6 @@ import com.android.systemui.dagger.qualifiers.Background;
 import com.android.systemui.settings.CurrentUserTracker;
 import com.android.systemui.statusbar.policy.BrightnessMirrorController;
 
-import java.util.ArrayList;
-
 import javax.inject.Inject;
 
 public class BrightnessController implements ToggleSlider.Listener, MirroredBrightnessController {
@@ -92,13 +90,9 @@ public class BrightnessController implements ToggleSlider.Listener, MirroredBrig
         @Override
         public void onDisplayChanged(int displayId) {
             mBackgroundHandler.post(mUpdateSliderRunnable);
-            notifyCallbacks();
         }
     };
 
-    private ArrayList<BrightnessStateChangeCallback> mChangeCallbacks =
-            new ArrayList<BrightnessStateChangeCallback>();
-
     private volatile boolean mAutomatic;  // Brightness adjusted automatically using ambient light.
     private volatile boolean mIsVrModeEnabled;
     private boolean mListening;
@@ -114,11 +108,6 @@ public class BrightnessController implements ToggleSlider.Listener, MirroredBrig
         mControl.setMirrorControllerAndMirror(controller);
     }
 
-    public interface BrightnessStateChangeCallback {
-        /** Indicates that some of the brightness settings have changed */
-        void onBrightnessLevelChanged();
-    }
-
     /** ContentObserver to watch brightness */
     private class BrightnessObserver extends ContentObserver {
 
@@ -139,7 +128,6 @@ public class BrightnessController implements ToggleSlider.Listener, MirroredBrig
                 mBackgroundHandler.post(mUpdateModeRunnable);
                 mBackgroundHandler.post(mUpdateSliderRunnable);
             }
-            notifyCallbacks();
         }
 
         public void startObserving() {
@@ -317,14 +305,6 @@ public class BrightnessController implements ToggleSlider.Listener, MirroredBrig
                 Context.VR_SERVICE));
     }
 
-    public void addStateChangedCallback(BrightnessStateChangeCallback cb) {
-        mChangeCallbacks.add(cb);
-    }
-
-    public boolean removeStateChangedCallback(BrightnessStateChangeCallback cb) {
-        return mChangeCallbacks.remove(cb);
-    }
-
     public void registerCallbacks() {
         mBackgroundHandler.post(mStartListeningRunnable);
     }
@@ -375,10 +355,6 @@ public class BrightnessController implements ToggleSlider.Listener, MirroredBrig
                     }
                 });
         }
-
-        for (BrightnessStateChangeCallback cb : mChangeCallbacks) {
-            cb.onBrightnessLevelChanged();
-        }
     }
 
     public void checkRestrictionAndSetEnabled() {
@@ -435,8 +411,12 @@ public class BrightnessController implements ToggleSlider.Listener, MirroredBrig
     }
 
     private void animateSliderTo(int target) {
-        if (!mControlValueInitialized) {
+        if (!mControlValueInitialized || !mControl.isVisible()) {
             // Don't animate the first value since its default state isn't meaningful to users.
+            // We also don't want to animate slider if it's not visible - especially important when
+            // two sliders are active at the same time in split shade (one in QS and one in QQS),
+            // as this negatively affects transition between them and they share mirror slider -
+            // animating it from two different sources causes janky motion
             mControl.setValue(target);
             mControlValueInitialized = true;
         }
@@ -455,13 +435,6 @@ public class BrightnessController implements ToggleSlider.Listener, MirroredBrig
         mSliderAnimator.start();
     }
 
-    private void notifyCallbacks() {
-        final int size = mChangeCallbacks.size();
-        for (int i = 0; i < size; i++) {
-            mChangeCallbacks.get(i).onBrightnessLevelChanged();
-        }
-    }
-
     /** Factory for creating a {@link BrightnessController}. */
     public static class Factory {
         private final Context mContext;

packages/SystemUI/src/com/android/systemui/settings/brightness/BrightnessDialog.java

@@ -41,14 +41,14 @@ import javax.inject.Inject;
 public class BrightnessDialog extends Activity {
 
     private BrightnessController mBrightnessController;
-    private final BrightnessSlider.Factory mToggleSliderFactory;
+    private final BrightnessSliderController.Factory mToggleSliderFactory;
     private final BroadcastDispatcher mBroadcastDispatcher;
     private final Handler mBackgroundHandler;
 
     @Inject
     public BrightnessDialog(
             BroadcastDispatcher broadcastDispatcher,
-            BrightnessSlider.Factory factory,
+            BrightnessSliderController.Factory factory,
             @Background Handler bgHandler) {
         mBroadcastDispatcher = broadcastDispatcher;
         mToggleSliderFactory = factory;
@@ -77,7 +77,7 @@ public class BrightnessDialog extends Activity {
         // The brightness mirror container is INVISIBLE by default.
         frame.setVisibility(View.VISIBLE);
 
-        BrightnessSlider controller = mToggleSliderFactory.create(this, frame);
+        BrightnessSliderController controller = mToggleSliderFactory.create(this, frame);
         controller.init();
         frame.addView(controller.getRootView(), MATCH_PARENT, WRAP_CONTENT);
 

packages/SystemUI/src/com/android/systemui/settings/brightness/BrightnessSliderController.java

@@ -44,7 +44,8 @@ import javax.inject.Inject;
  *
  * @see BrightnessMirrorController
  */
-public class BrightnessSlider extends ViewController<BrightnessSliderView> implements ToggleSlider {
+public class BrightnessSliderController extends ViewController<BrightnessSliderView> implements
+        ToggleSlider {
 
     private Listener mListener;
     private ToggleSlider mMirror;
@@ -69,7 +70,7 @@ public class BrightnessSlider extends ViewController<BrightnessSliderView> imple
         }
     };
 
-    BrightnessSlider(
+    BrightnessSliderController(
             BrightnessSliderView brightnessSliderView,
             FalsingManager falsingManager) {
         super(brightnessSliderView);
@@ -184,6 +185,15 @@ public class BrightnessSlider extends ViewController<BrightnessSliderView> imple
         mView.setVisibility(View.VISIBLE);
     }
 
+    @Override
+    public boolean isVisible() {
+        // this should be called rarely - once or twice per slider's value change, but not for
+        // every value change when user slides finger - only the final one.
+        // If view is not visible this call is quick (around 50 µs) as it sees parent is not visible
+        // otherwise it's slightly longer (70 µs) because there are more checks to be done
+        return mView.isVisibleToUser();
+    }
+
     private final SeekBar.OnSeekBarChangeListener mSeekListener =
             new SeekBar.OnSeekBarChangeListener() {
         @Override
@@ -222,7 +232,7 @@ public class BrightnessSlider extends ViewController<BrightnessSliderView> imple
     };
 
     /**
-     * Creates a {@link BrightnessSlider} with its associated view.
+     * Creates a {@link BrightnessSliderController} with its associated view.
      */
     public static class Factory {
 
@@ -240,11 +250,11 @@ public class BrightnessSlider extends ViewController<BrightnessSliderView> imple
          * @param viewRoot the {@link ViewGroup} that will contain the hierarchy. The inflated
          *                 hierarchy will not be attached
          */
-        public BrightnessSlider create(Context context, @Nullable ViewGroup viewRoot) {
+        public BrightnessSliderController create(Context context, @Nullable ViewGroup viewRoot) {
             int layout = getLayout();
             BrightnessSliderView root = (BrightnessSliderView) LayoutInflater.from(context)
                     .inflate(layout, viewRoot, false);
-            return new BrightnessSlider(root, mFalsingManager);
+            return new BrightnessSliderController(root, mFalsingManager);
         }
 
         /** Get the layout to inflate based on what slider to use */

packages/SystemUI/src/com/android/systemui/settings/brightness/ToggleSlider.java

@@ -38,4 +38,5 @@ public interface ToggleSlider {
 
     void showView();
     void hideView();
+    boolean isVisible();
 }

packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBar.java

@@ -174,7 +174,7 @@ import com.android.systemui.qs.QSFragment;
 import com.android.systemui.qs.QSPanelController;
 import com.android.systemui.recents.ScreenPinningRequest;
 import com.android.systemui.scrim.ScrimView;
-import com.android.systemui.settings.brightness.BrightnessSlider;
+import com.android.systemui.settings.brightness.BrightnessSliderController;
 import com.android.systemui.shared.plugins.PluginManager;
 import com.android.systemui.statusbar.AutoHideUiElement;
 import com.android.systemui.statusbar.BackDropView;
@@ -539,7 +539,7 @@ public class StatusBar extends SystemUI implements
     private final NotificationViewHierarchyManager mViewHierarchyManager;
     private final KeyguardViewMediator mKeyguardViewMediator;
     protected final NotificationInterruptStateProvider mNotificationInterruptStateProvider;
-    private final BrightnessSlider.Factory mBrightnessSliderFactory;
+    private final BrightnessSliderController.Factory mBrightnessSliderFactory;
     private final FeatureFlags mFeatureFlags;
     private final UnfoldTransitionConfig mUnfoldTransitionConfig;
     private final Lazy<UnfoldLightRevealOverlayAnimation> mUnfoldLightRevealOverlayAnimation;
@@ -778,7 +778,7 @@ public class StatusBar extends SystemUI implements
             Lazy<NotificationShadeDepthController> notificationShadeDepthControllerLazy,
             StatusBarTouchableRegionManager statusBarTouchableRegionManager,
             NotificationIconAreaController notificationIconAreaController,
-            BrightnessSlider.Factory brightnessSliderFactory,
+            BrightnessSliderController.Factory brightnessSliderFactory,
             UnfoldTransitionConfig unfoldTransitionConfig,
             Lazy<UnfoldLightRevealOverlayAnimation> unfoldLightRevealOverlayAnimation,
             Lazy<UnfoldTransitionWallpaperController> unfoldTransitionWallpaperController,

packages/SystemUI/src/com/android/systemui/statusbar/phone/dagger/StatusBarPhoneModule.java

@@ -48,7 +48,7 @@ import com.android.systemui.navigationbar.NavigationBarController;
 import com.android.systemui.plugins.FalsingManager;
 import com.android.systemui.plugins.PluginDependencyProvider;
 import com.android.systemui.recents.ScreenPinningRequest;
-import com.android.systemui.settings.brightness.BrightnessSlider;
+import com.android.systemui.settings.brightness.BrightnessSliderController;
 import com.android.systemui.shared.plugins.PluginManager;
 import com.android.systemui.statusbar.CommandQueue;
 import com.android.systemui.statusbar.KeyguardIndicationController;
@@ -217,7 +217,7 @@ public interface StatusBarPhoneModule {
             Lazy<NotificationShadeDepthController> notificationShadeDepthController,
             StatusBarTouchableRegionManager statusBarTouchableRegionManager,
             NotificationIconAreaController notificationIconAreaController,
-            BrightnessSlider.Factory brightnessSliderFactory,
+            BrightnessSliderController.Factory brightnessSliderFactory,
             UnfoldTransitionConfig unfoldTransitionConfig,
             Lazy<UnfoldLightRevealOverlayAnimation> unfoldLightRevealOverlayAnimation,
             Lazy<UnfoldTransitionWallpaperController> unfoldTransitionWallpaperController,

packages/SystemUI/src/com/android/systemui/statusbar/policy/BrightnessMirrorController.java

@@ -26,7 +26,7 @@ import android.view.ViewGroup;
 import android.widget.FrameLayout;
 
 import com.android.systemui.R;
-import com.android.systemui.settings.brightness.BrightnessSlider;
+import com.android.systemui.settings.brightness.BrightnessSliderController;
 import com.android.systemui.settings.brightness.ToggleSlider;
 import com.android.systemui.statusbar.NotificationShadeDepthController;
 import com.android.systemui.statusbar.phone.NotificationPanelViewController;
@@ -46,8 +46,8 @@ public class BrightnessMirrorController
     private final NotificationPanelViewController mNotificationPanel;
     private final NotificationShadeDepthController mDepthController;
     private final ArraySet<BrightnessMirrorListener> mBrightnessMirrorListeners = new ArraySet<>();
-    private final BrightnessSlider.Factory mToggleSliderFactory;
-    private BrightnessSlider mToggleSliderController;
+    private final BrightnessSliderController.Factory mToggleSliderFactory;
+    private BrightnessSliderController mToggleSliderController;
     private final int[] mInt2Cache = new int[2];
     private FrameLayout mBrightnessMirror;
     private int mBrightnessMirrorBackgroundPadding;
@@ -56,7 +56,7 @@ public class BrightnessMirrorController
     public BrightnessMirrorController(NotificationShadeWindowView statusBarWindow,
             NotificationPanelViewController notificationPanelViewController,
             NotificationShadeDepthController notificationShadeDepthController,
-            BrightnessSlider.Factory factory,
+            BrightnessSliderController.Factory factory,
             @NonNull Consumer<Boolean> visibilityCallback) {
         mStatusBarWindow = statusBarWindow;
         mToggleSliderFactory = factory;
@@ -135,9 +135,10 @@ public class BrightnessMirrorController
         reinflate();
     }
 
-    private BrightnessSlider setMirrorLayout() {
+    private BrightnessSliderController setMirrorLayout() {
         Context context = mBrightnessMirror.getContext();
-        BrightnessSlider controller = mToggleSliderFactory.create(context, mBrightnessMirror);
+        BrightnessSliderController controller = mToggleSliderFactory.create(context,
+                mBrightnessMirror);
         controller.init();
 
         mBrightnessMirror.addView(controller.getRootView(), ViewGroup.LayoutParams.MATCH_PARENT,

packages/SystemUI/tests/src/com/android/systemui/qs/QSPanelControllerTest.java

@@ -42,7 +42,7 @@ import com.android.systemui.qs.customize.QSCustomizerController;
 import com.android.systemui.qs.logging.QSLogger;
 import com.android.systemui.qs.tileimpl.QSTileImpl;
 import com.android.systemui.settings.brightness.BrightnessController;
-import com.android.systemui.settings.brightness.BrightnessSlider;
+import com.android.systemui.settings.brightness.BrightnessSliderController;
 import com.android.systemui.settings.brightness.ToggleSlider;
 import com.android.systemui.statusbar.CommandQueue;
 import com.android.systemui.tuner.TunerService;
@@ -88,9 +88,9 @@ public class QSPanelControllerTest extends SysuiTestCase {
     @Mock
     private BrightnessController mBrightnessController;
     @Mock
-    private BrightnessSlider.Factory mToggleSliderViewControllerFactory;
+    private BrightnessSliderController.Factory mToggleSliderViewControllerFactory;
     @Mock
-    private BrightnessSlider mBrightnessSlider;
+    private BrightnessSliderController mBrightnessSliderController;
     @Mock
     QSTileImpl mQSTile;
     @Mock
@@ -120,7 +120,7 @@ public class QSPanelControllerTest extends SysuiTestCase {
         when(mQSTileHost.getTiles()).thenReturn(Collections.singleton(mQSTile));
         when(mQSTileHost.createTileView(any(), eq(mQSTile), anyBoolean())).thenReturn(mQSTileView);
         when(mToggleSliderViewControllerFactory.create(any(), any()))
-                .thenReturn(mBrightnessSlider);
+                .thenReturn(mBrightnessSliderController);
         when(mBrightnessControllerFactory.create(any(ToggleSlider.class)))
                 .thenReturn(mBrightnessController);
         when(mQSTileRevealControllerFactory.create(any(), any()))

packages/SystemUI/tests/src/com/android/systemui/settings/brightness/BrightnessSliderControllerTest.kt

@@ -45,7 +45,7 @@ import org.mockito.Mockito.`when` as whenever
 
 @SmallTest
 @RunWith(AndroidTestingRunner::class)
-class BrightnessSliderTest : SysuiTestCase() {
+class BrightnessSliderControllerTest : SysuiTestCase() {
 
     @Mock
     private lateinit var brightnessSliderView: BrightnessSliderView
@@ -66,7 +66,7 @@ class BrightnessSliderTest : SysuiTestCase() {
     private lateinit var seekBar: SeekBar
     private var mFalsingManager: FalsingManagerFake = FalsingManagerFake()
 
-    private lateinit var mController: BrightnessSlider
+    private lateinit var mController: BrightnessSliderController
 
     @Before
     fun setUp() {
@@ -75,7 +75,7 @@ class BrightnessSliderTest : SysuiTestCase() {
         whenever(mirrorController.toggleSlider).thenReturn(mirror)
         whenever(motionEvent.copy()).thenReturn(motionEvent)
 
-        mController = BrightnessSlider(brightnessSliderView, mFalsingManager)
+        mController = BrightnessSliderController(brightnessSliderView, mFalsingManager)
         mController.init()
         mController.setOnChangedListener(listener)
     }

packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarTest.java

@@ -96,7 +96,7 @@ import com.android.systemui.plugins.ActivityStarter.OnDismissAction;
 import com.android.systemui.plugins.PluginDependencyProvider;
 import com.android.systemui.plugins.statusbar.StatusBarStateController;
 import com.android.systemui.recents.ScreenPinningRequest;
-import com.android.systemui.settings.brightness.BrightnessSlider;
+import com.android.systemui.settings.brightness.BrightnessSliderController;
 import com.android.systemui.shared.plugins.PluginManager;
 import com.android.systemui.statusbar.CommandQueue;
 import com.android.systemui.statusbar.KeyguardIndicationController;
@@ -256,7 +256,7 @@ public class StatusBarTest extends SysuiTestCase {
     @Mock private PhoneStatusBarPolicy mPhoneStatusBarPolicy;
     @Mock private DemoModeController mDemoModeController;
     @Mock private Lazy<NotificationShadeDepthController> mNotificationShadeDepthControllerLazy;
-    @Mock private BrightnessSlider.Factory mBrightnessSliderFactory;
+    @Mock private BrightnessSliderController.Factory mBrightnessSliderFactory;
     @Mock private UnfoldTransitionConfig mUnfoldTransitionConfig;
     @Mock private Lazy<UnfoldLightRevealOverlayAnimation> mUnfoldLightRevealOverlayAnimationLazy;
     @Mock private Lazy<StatusBarMoveFromCenterAnimationController> mMoveFromCenterAnimationLazy;