Merge "Only reset short term model after timeout" into udc-dev am: 852658503c

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/22463382

Change-Id: I6c607de87604f945f6bed93ab0fd56caaf58207b
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

services/core/java/com/android/server/display/AutomaticBrightnessController.java

@@ -75,10 +75,11 @@ public class AutomaticBrightnessController {
 
     private static final int MSG_UPDATE_AMBIENT_LUX = 1;
     private static final int MSG_BRIGHTNESS_ADJUSTMENT_SAMPLE = 2;
-    private static final int MSG_INVALIDATE_SHORT_TERM_MODEL = 3;
+    private static final int MSG_INVALIDATE_CURRENT_SHORT_TERM_MODEL = 3;
     private static final int MSG_UPDATE_FOREGROUND_APP = 4;
     private static final int MSG_UPDATE_FOREGROUND_APP_SYNC = 5;
     private static final int MSG_RUN_UPDATE = 6;
+    private static final int MSG_INVALIDATE_PAUSED_SHORT_TERM_MODEL = 7;
 
     // Callbacks for requesting updates to the display's power state
     private final Callbacks mCallbacks;
@@ -216,12 +217,11 @@ public class AutomaticBrightnessController {
     private float mBrightnessAdjustmentSampleOldLux;
     private float mBrightnessAdjustmentSampleOldBrightness;
 
-    // When the short term model is invalidated, we don't necessarily reset it (i.e. clear the
+    // The short term models, current and previous. Eg, we might use the "paused" one to save out
-    // user's adjustment) immediately, but wait for a drastic enough change in the ambient light.
+    // the interactive short term model when switching to idle screen brightness mode, and
-    // The anchor determines what were the light levels when the user has set their preference, and
+    // vice-versa.
-    // we use a relative threshold to determine when to revert to the OEM curve.
+    private final ShortTermModel mShortTermModel;
-    private boolean mShortTermModelValid;
+    private final ShortTermModel mPausedShortTermModel;
-    private float mShortTermModelAnchor;
 
     // Controls High Brightness Mode.
     private HighBrightnessModeController mHbmController;
@@ -309,8 +309,8 @@ public class AutomaticBrightnessController {
         mAmbientBrightnessThresholdsIdle = ambientBrightnessThresholdsIdle;
         mScreenBrightnessThresholds = screenBrightnessThresholds;
         mScreenBrightnessThresholdsIdle = screenBrightnessThresholdsIdle;
-        mShortTermModelValid = true;
+        mShortTermModel = new ShortTermModel();
-        mShortTermModelAnchor = -1;
+        mPausedShortTermModel = new ShortTermModel();
         mHandler = new AutomaticBrightnessHandler(looper);
         mAmbientLightRingBuffer =
             new AmbientLightRingBuffer(mNormalLightSensorRate, mAmbientLightHorizonLong, mClock);
@@ -492,10 +492,10 @@ public class AutomaticBrightnessController {
             Slog.d(TAG, "Display policy transitioning from " + oldPolicy + " to " + policy);
         }
         if (!isInteractivePolicy(policy) && isInteractivePolicy(oldPolicy) && !isInIdleMode()) {
-            mHandler.sendEmptyMessageDelayed(MSG_INVALIDATE_SHORT_TERM_MODEL,
+            mHandler.sendEmptyMessageDelayed(MSG_INVALIDATE_CURRENT_SHORT_TERM_MODEL,
                     mCurrentBrightnessMapper.getShortTermModelTimeout());
         } else if (isInteractivePolicy(policy) && !isInteractivePolicy(oldPolicy)) {
-            mHandler.removeMessages(MSG_INVALIDATE_SHORT_TERM_MODEL);
+            mHandler.removeMessages(MSG_INVALIDATE_CURRENT_SHORT_TERM_MODEL);
         }
         return true;
     }
@@ -516,25 +516,13 @@ public class AutomaticBrightnessController {
 
     private boolean setScreenBrightnessByUser(float lux, float brightness) {
         mCurrentBrightnessMapper.addUserDataPoint(lux, brightness);
-        mShortTermModelValid = true;
+        mShortTermModel.setUserBrightness(lux, brightness);
-        mShortTermModelAnchor = lux;
-        if (mLoggingEnabled) {
-            Slog.d(TAG, "ShortTermModel: anchor=" + mShortTermModelAnchor);
-        }
         return true;
     }
 
     public void resetShortTermModel() {
         mCurrentBrightnessMapper.clearUserDataPoints();
-        mShortTermModelValid = true;
+        mShortTermModel.reset();
-        mShortTermModelAnchor = -1;
-    }
-
-    private void invalidateShortTermModel() {
-        if (mLoggingEnabled) {
-            Slog.d(TAG, "ShortTermModel: invalidate user data");
-        }
-        mShortTermModelValid = false;
     }
 
     public boolean setBrightnessConfiguration(BrightnessConfiguration configuration,
@@ -595,8 +583,12 @@ public class AutomaticBrightnessController {
             pw.println("  mShortTermModelTimeout(idle)="
                     + mIdleModeBrightnessMapper.getShortTermModelTimeout());
         }
-        pw.println("  mShortTermModelAnchor=" + mShortTermModelAnchor);
+        pw.println("  mShortTermModel=");
-        pw.println("  mShortTermModelValid=" + mShortTermModelValid);
+        mShortTermModel.dump(pw);
+        pw.println("  mPausedShortTermModel=");
+        mPausedShortTermModel.dump(pw);
+
+        pw.println();
         pw.println("  mBrightnessAdjustmentSamplePending=" + mBrightnessAdjustmentSamplePending);
         pw.println("  mBrightnessAdjustmentSampleOldLux=" + mBrightnessAdjustmentSampleOldLux);
         pw.println("  mBrightnessAdjustmentSampleOldBrightness="
@@ -740,15 +732,9 @@ public class AutomaticBrightnessController {
         }
         mHbmController.onAmbientLuxChange(mAmbientLux);
 
+
         // If the short term model was invalidated and the change is drastic enough, reset it.
-        if (!mShortTermModelValid && mShortTermModelAnchor != -1) {
+        mShortTermModel.maybeReset(mAmbientLux);
-            if (mCurrentBrightnessMapper.shouldResetShortTermModel(
-                    mAmbientLux, mShortTermModelAnchor)) {
-                resetShortTermModel();
-            } else {
-                mShortTermModelValid = true;
-            }
-        }
     }
 
     private float calculateAmbientLux(long now, long horizon) {
@@ -1118,8 +1104,29 @@ public class AutomaticBrightnessController {
             return;
         }
         Slog.i(TAG, "Switching to Idle Screen Brightness Mode");
+        // Stash short term model
+        ShortTermModel tempShortTermModel = new ShortTermModel();
+        tempShortTermModel.set(mCurrentBrightnessMapper.getUserLux(),
+                mCurrentBrightnessMapper.getUserBrightness(), /* valid= */ true);
+
+        // Send delayed timeout
+        mHandler.sendEmptyMessageAtTime(MSG_INVALIDATE_PAUSED_SHORT_TERM_MODEL,
+                mClock.uptimeMillis()
+                        + mCurrentBrightnessMapper.getShortTermModelTimeout());
+
+        Slog.i(TAG, "mPreviousShortTermModel" + mPausedShortTermModel);
+        // new brightness mapper
         mCurrentBrightnessMapper = mIdleModeBrightnessMapper;
-        resetShortTermModel();
+
+        // if previous stm has been invalidated, and lux has drastically changed, just use
+        // the new, reset stm.
+        // if previous stm is still valid then revalidate it
+        if (mPausedShortTermModel != null && !mPausedShortTermModel.maybeReset(mAmbientLux)) {
+            setScreenBrightnessByUser(mPausedShortTermModel.mAnchor,
+                    mPausedShortTermModel.mBrightness);
+        }
+        mPausedShortTermModel.copyFrom(tempShortTermModel);
+
         update();
     }
 
@@ -1128,8 +1135,28 @@ public class AutomaticBrightnessController {
             return;
         }
         Slog.i(TAG, "Switching to Interactive Screen Brightness Mode");
+        ShortTermModel tempShortTermModel = new ShortTermModel();
+        tempShortTermModel.set(mCurrentBrightnessMapper.getUserLux(),
+                mCurrentBrightnessMapper.getUserBrightness(), /* valid= */ true);
+        mHandler.removeMessages(MSG_INVALIDATE_PAUSED_SHORT_TERM_MODEL);
+        // Send delayed timeout
+        mHandler.sendEmptyMessageAtTime(MSG_INVALIDATE_PAUSED_SHORT_TERM_MODEL,
+                mClock.uptimeMillis()
+                        + mCurrentBrightnessMapper.getShortTermModelTimeout());
+        Slog.i(TAG, "mPreviousShortTermModel" + mPausedShortTermModel.toString());
+
+        // restore interactive mapper.
         mCurrentBrightnessMapper = mInteractiveModeBrightnessMapper;
-        resetShortTermModel();
+
+        // if previous stm has been invalidated, and lux has drastically changed, just use
+        // the new, reset stm.
+        // if previous stm is still valid then revalidate it
+        if (!mPausedShortTermModel.maybeReset(mAmbientLux)) {
+            setScreenBrightnessByUser(mPausedShortTermModel.mAnchor,
+                    mPausedShortTermModel.mBrightness);
+        }
+        mPausedShortTermModel.copyFrom(tempShortTermModel);
+
         update();
     }
 
@@ -1164,6 +1191,77 @@ public class AutomaticBrightnessController {
         }
     }
 
+    private class ShortTermModel {
+        // When the short term model is invalidated, we don't necessarily reset it (i.e. clear the
+        // user's adjustment) immediately, but wait for a drastic enough change in the ambient
+        // light.
+        // The anchor determines what were the light levels when the user has set their preference,
+        // and we use a relative threshold to determine when to revert to the OEM curve.
+        private float mAnchor = -1f;
+        private float mBrightness;
+        private boolean mIsValid = true;
+
+        private void reset() {
+            mAnchor = -1f;
+            mBrightness = -1f;
+            mIsValid = true;
+        }
+
+        private void invalidate() {
+            mIsValid = false;
+            if (mLoggingEnabled) {
+                Slog.d(TAG, "ShortTermModel: invalidate user data");
+            }
+        }
+
+        private void setUserBrightness(float lux, float brightness) {
+            mAnchor = lux;
+            mBrightness = brightness;
+            mIsValid = true;
+            if (mLoggingEnabled) {
+                Slog.d(TAG, "ShortTermModel: anchor=" + mAnchor);
+            }
+        }
+
+        private boolean maybeReset(float currentLux) {
+            // If the short term model was invalidated and the change is drastic enough, reset it.
+            // Otherwise, we revalidate it.
+            if (!mIsValid && mAnchor != -1) {
+                if (mCurrentBrightnessMapper != null
+                        && mCurrentBrightnessMapper.shouldResetShortTermModel(
+                        currentLux, mAnchor)) {
+                    resetShortTermModel();
+                } else {
+                    mIsValid = true;
+                }
+                return mIsValid;
+            }
+            return false;
+        }
+
+        private void set(float anchor, float brightness, boolean valid) {
+            mAnchor = anchor;
+            mBrightness = brightness;
+            mIsValid = valid;
+        }
+        private void copyFrom(ShortTermModel from) {
+            mAnchor = from.mAnchor;
+            mBrightness = from.mBrightness;
+            mIsValid = from.mIsValid;
+        }
+
+        public String toString() {
+            return " mAnchor: " + mAnchor
+                    + "\n mBrightness: " + mBrightness
+                    + "\n mIsValid: " + mIsValid;
+        }
+
+        void dump(PrintWriter pw) {
+            pw.println(this);
+        }
+
+    }
+
     private final class AutomaticBrightnessHandler extends Handler {
         public AutomaticBrightnessHandler(Looper looper) {
             super(looper, null, true /*async*/);
@@ -1184,8 +1282,8 @@ public class AutomaticBrightnessController {
                     collectBrightnessAdjustmentSample();
                     break;
 
-                case MSG_INVALIDATE_SHORT_TERM_MODEL:
+                case MSG_INVALIDATE_CURRENT_SHORT_TERM_MODEL:
-                    invalidateShortTermModel();
+                    mShortTermModel.invalidate();
                     break;
 
                 case MSG_UPDATE_FOREGROUND_APP:
@@ -1195,6 +1293,10 @@ public class AutomaticBrightnessController {
                 case MSG_UPDATE_FOREGROUND_APP_SYNC:
                     updateForegroundAppSync();
                     break;
+
+                case MSG_INVALIDATE_PAUSED_SHORT_TERM_MODEL:
+                    mPausedShortTermModel.invalidate();
+                    break;
             }
         }
     }

services/core/java/com/android/server/display/BrightnessMappingStrategy.java

@@ -363,13 +363,17 @@ public abstract class BrightnessMappingStrategy {
     public abstract void recalculateSplines(boolean applyAdjustment, float[] adjustment);
 
     /**
-     * Returns the timeout for the short term model
+     * Returns the timeout, in milliseconds for the short term model
      *
      * Timeout after which we remove the effects any user interactions might've had on the
      * brightness mapping. This timeout doesn't start until we transition to a non-interactive
      * display policy so that we don't reset while users are using their devices, but also so that
      * we don't erroneously keep the short-term model if the device is dozing but the
      * display is fully on.
+     *
+     * This timeout is also used when the device switches from interactive screen brightness mode
+     * to idle screen brightness mode, to preserve the user's preference when they resume usage of
+     * the device, within the specified timeframe.
      */
     public abstract long getShortTermModelTimeout();
 

services/tests/servicestests/src/com/android/server/display/AutomaticBrightnessControllerTest.java

@@ -126,7 +126,7 @@ public class AutomaticBrightnessControllerTest {
                         return mClock::now;
                     }
 
-                }, // pass in test looper instead, pass in offsetable clock
+                }, // pass in test looper instead, pass in offsettable clock
                 () -> { }, mTestLooper.getLooper(), mSensorManager, lightSensor,
                 mBrightnessMappingStrategy, LIGHT_SENSOR_WARMUP_TIME, BRIGHTNESS_MIN_FLOAT,
                 BRIGHTNESS_MAX_FLOAT, DOZE_SCALE_FACTOR, LIGHT_SENSOR_RATE,
@@ -299,6 +299,179 @@ public class AutomaticBrightnessControllerTest {
         verifyNoMoreInteractions(mBrightnessMappingStrategy);
     }
 
+    @Test
+    public void testShortTermModelTimesOut() throws Exception {
+        ArgumentCaptor<SensorEventListener> listenerCaptor =
+                ArgumentCaptor.forClass(SensorEventListener.class);
+        verify(mSensorManager).registerListener(listenerCaptor.capture(), eq(mLightSensor),
+                eq(INITIAL_LIGHT_SENSOR_RATE * 1000), any(Handler.class));
+        SensorEventListener listener = listenerCaptor.getValue();
+
+        // Sensor reads 123 lux,
+        listener.onSensorChanged(TestUtils.createSensorEvent(mLightSensor, 123));
+        // User sets brightness to 100
+        mController.configure(AUTO_BRIGHTNESS_ENABLED, /* configuration= */ null,
+                /* brightness= */ 0.5f, /* userChangedBrightness= */ true, /* adjustment= */ 0,
+                /* userChanged= */ false, DisplayPowerRequest.POLICY_BRIGHT,
+                /* shouldResetShortTermModel= */ true);
+
+        when(mBrightnessMappingStrategy.getShortTermModelTimeout()).thenReturn(2000L);
+
+        mController.switchToIdleMode();
+        when(mIdleBrightnessMappingStrategy.isForIdleMode()).thenReturn(true);
+        when(mBrightnessMappingStrategy.shouldResetShortTermModel(
+                123f, 0.5f)).thenReturn(true);
+
+        // Sensor reads 1000 lux,
+        listener.onSensorChanged(TestUtils.createSensorEvent(mLightSensor, 1000));
+        mTestLooper.moveTimeForward(
+                mBrightnessMappingStrategy.getShortTermModelTimeout() + 1000);
+        mTestLooper.dispatchAll();
+
+        mController.switchToInteractiveScreenBrightnessMode();
+        mTestLooper.moveTimeForward(4000);
+        mTestLooper.dispatchAll();
+
+        // Verify only happens on the first configure. (i.e. not again when switching back)
+        // Intentionally using any() to ensure it's not called whatsoever.
+        verify(mBrightnessMappingStrategy, times(1))
+                .addUserDataPoint(123.0f, 0.5f);
+        verify(mBrightnessMappingStrategy, times(1))
+                .addUserDataPoint(anyFloat(), anyFloat());
+    }
+
+    @Test
+    public void testShortTermModelDoesntTimeOut() throws Exception {
+        ArgumentCaptor<SensorEventListener> listenerCaptor =
+                ArgumentCaptor.forClass(SensorEventListener.class);
+        verify(mSensorManager).registerListener(listenerCaptor.capture(), eq(mLightSensor),
+                eq(INITIAL_LIGHT_SENSOR_RATE * 1000), any(Handler.class));
+        SensorEventListener listener = listenerCaptor.getValue();
+
+        // Sensor reads 123 lux,
+        listener.onSensorChanged(TestUtils.createSensorEvent(mLightSensor, 123));
+        // User sets brightness to 100
+        mController.configure(AUTO_BRIGHTNESS_ENABLED, null /* configuration= */,
+                0.51f /* brightness= */, true /* userChangedBrightness= */, 0 /* adjustment= */,
+                false /* userChanged= */, DisplayPowerRequest.POLICY_BRIGHT,
+                /* shouldResetShortTermModel= */ true);
+
+        when(mBrightnessMappingStrategy.shouldResetShortTermModel(
+                anyFloat(), anyFloat())).thenReturn(true);
+        when(mBrightnessMappingStrategy.getShortTermModelTimeout()).thenReturn(2000L);
+        when(mBrightnessMappingStrategy.getUserBrightness()).thenReturn(0.51f);
+        when(mBrightnessMappingStrategy.getUserLux()).thenReturn(123.0f);
+
+        mController.switchToIdleMode();
+        when(mIdleBrightnessMappingStrategy.isForIdleMode()).thenReturn(true);
+
+        // Time does not move forward, since clock is doesn't increment naturally.
+        mTestLooper.dispatchAll();
+
+        // Sensor reads 100000 lux,
+        listener.onSensorChanged(TestUtils.createSensorEvent(mLightSensor, 678910));
+        mController.switchToInteractiveScreenBrightnessMode();
+
+        // Verify short term model is not reset.
+        verify(mBrightnessMappingStrategy, never()).clearUserDataPoints();
+
+        // Verify that we add the data point once when the user sets it, and again when we return
+        // interactive mode.
+        verify(mBrightnessMappingStrategy, times(2))
+                .addUserDataPoint(123.0f, 0.51f);
+    }
+
+    @Test
+    public void testShortTermModelIsRestoredWhenSwitchingWithinTimeout() throws Exception {
+        ArgumentCaptor<SensorEventListener> listenerCaptor =
+                ArgumentCaptor.forClass(SensorEventListener.class);
+        verify(mSensorManager).registerListener(listenerCaptor.capture(), eq(mLightSensor),
+                eq(INITIAL_LIGHT_SENSOR_RATE * 1000), any(Handler.class));
+        SensorEventListener listener = listenerCaptor.getValue();
+
+        // Sensor reads 123 lux,
+        listener.onSensorChanged(TestUtils.createSensorEvent(mLightSensor, 123));
+        // User sets brightness to 100
+        mController.configure(AUTO_BRIGHTNESS_ENABLED, /* configuration= */ null,
+                /* brightness= */ 0.5f, /* userChangedBrightness= */ true, /* adjustment= */ 0,
+                /* userChanged= */ false, DisplayPowerRequest.POLICY_BRIGHT,
+                /* shouldResetShortTermModel= */ true);
+
+        when(mBrightnessMappingStrategy.getShortTermModelTimeout()).thenReturn(2000L);
+        when(mBrightnessMappingStrategy.getUserBrightness()).thenReturn(0.5f);
+        when(mBrightnessMappingStrategy.getUserLux()).thenReturn(123f);
+
+        mController.switchToIdleMode();
+        when(mIdleBrightnessMappingStrategy.isForIdleMode()).thenReturn(true);
+        when(mIdleBrightnessMappingStrategy.getUserBrightness()).thenReturn(-1f);
+        when(mIdleBrightnessMappingStrategy.getUserLux()).thenReturn(-1f);
+        when(mBrightnessMappingStrategy.shouldResetShortTermModel(
+                123f, 0.5f)).thenReturn(true);
+
+        // Sensor reads 1000 lux,
+        listener.onSensorChanged(TestUtils.createSensorEvent(mLightSensor, 1000));
+        mTestLooper.moveTimeForward(
+                mBrightnessMappingStrategy.getShortTermModelTimeout() + 1000);
+        mTestLooper.dispatchAll();
+
+        mController.switchToInteractiveScreenBrightnessMode();
+        mTestLooper.moveTimeForward(4000);
+        mTestLooper.dispatchAll();
+
+        // Verify only happens on the first configure. (i.e. not again when switching back)
+        // Intentionally using any() to ensure it's not called whatsoever.
+        verify(mBrightnessMappingStrategy, times(1))
+                .addUserDataPoint(123.0f, 0.5f);
+        verify(mBrightnessMappingStrategy, times(1))
+                .addUserDataPoint(anyFloat(), anyFloat());
+    }
+
+    @Test
+    public void testShortTermModelNotRestoredAfterTimeout() throws Exception {
+        ArgumentCaptor<SensorEventListener> listenerCaptor =
+                ArgumentCaptor.forClass(SensorEventListener.class);
+        verify(mSensorManager).registerListener(listenerCaptor.capture(), eq(mLightSensor),
+                eq(INITIAL_LIGHT_SENSOR_RATE * 1000), any(Handler.class));
+        SensorEventListener listener = listenerCaptor.getValue();
+
+        // Sensor reads 123 lux,
+        listener.onSensorChanged(TestUtils.createSensorEvent(mLightSensor, 123));
+        // User sets brightness to 100
+        mController.configure(AUTO_BRIGHTNESS_ENABLED, /* configuration= */ null,
+                /* brightness= */ 0.5f, /* userChangedBrightness= */ true, /* adjustment= */ 0,
+                /* userChanged= */ false, DisplayPowerRequest.POLICY_BRIGHT,
+                /* shouldResetShortTermModel= */ true);
+
+        when(mBrightnessMappingStrategy.getShortTermModelTimeout()).thenReturn(2000L);
+
+        when(mBrightnessMappingStrategy.getUserBrightness()).thenReturn(0.5f);
+        when(mBrightnessMappingStrategy.getUserLux()).thenReturn(123f);
+
+        mController.switchToIdleMode();
+        when(mIdleBrightnessMappingStrategy.isForIdleMode()).thenReturn(true);
+        when(mIdleBrightnessMappingStrategy.getUserBrightness()).thenReturn(-1f);
+        when(mIdleBrightnessMappingStrategy.getUserLux()).thenReturn(-1f);
+
+        when(mBrightnessMappingStrategy.shouldResetShortTermModel(
+                123f, 0.5f)).thenReturn(true);
+
+        // Sensor reads 1000 lux,
+        listener.onSensorChanged(TestUtils.createSensorEvent(mLightSensor, 1000));
+        // Do not fast-forward time.
+        mTestLooper.dispatchAll();
+
+        mController.switchToInteractiveScreenBrightnessMode();
+        // Do not fast-forward time
+        mTestLooper.dispatchAll();
+
+        // Verify this happens on the first configure and again when switching back
+        // Intentionally using any() to ensure it's not called any other times whatsoever.
+        verify(mBrightnessMappingStrategy, times(2))
+                .addUserDataPoint(123.0f, 0.5f);
+        verify(mBrightnessMappingStrategy, times(2))
+                .addUserDataPoint(anyFloat(), anyFloat());
+    }
+
     @Test
     public void testSwitchToIdleMappingStrategy() throws Exception {
         ArgumentCaptor<SensorEventListener> listenerCaptor =
@@ -326,6 +499,11 @@ public class AutomaticBrightnessControllerTest {
         // Called once for init, and once when switching,
         // setAmbientLux() is called twice and once in updateAutoBrightness()
         verify(mBrightnessMappingStrategy, times(5)).isForIdleMode();
+        // Called when switching.
+        verify(mBrightnessMappingStrategy, times(1)).getShortTermModelTimeout();
+        verify(mBrightnessMappingStrategy, times(1)).getUserBrightness();
+        verify(mBrightnessMappingStrategy, times(1)).getUserLux();
+
         // Ensure, after switching, original BMS is not used anymore
         verifyNoMoreInteractions(mBrightnessMappingStrategy);