diff --git a/services/core/java/com/android/server/display/AutomaticBrightnessController.java b/services/core/java/com/android/server/display/AutomaticBrightnessController.java index af5609a68952e..299f86550efd3 100644 --- a/services/core/java/com/android/server/display/AutomaticBrightnessController.java +++ b/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 - // 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 boolean mShortTermModelValid; - private float mShortTermModelAnchor; + // The short term models, current and previous. Eg, we might use the "paused" one to save out + // the interactive short term model when switching to idle screen brightness mode, and + // vice-versa. + private final ShortTermModel mShortTermModel; + private final ShortTermModel mPausedShortTermModel; // Controls High Brightness Mode. private HighBrightnessModeController mHbmController; @@ -309,8 +309,8 @@ public class AutomaticBrightnessController { mAmbientBrightnessThresholdsIdle = ambientBrightnessThresholdsIdle; mScreenBrightnessThresholds = screenBrightnessThresholds; mScreenBrightnessThresholdsIdle = screenBrightnessThresholdsIdle; - mShortTermModelValid = true; - mShortTermModelAnchor = -1; + mShortTermModel = new ShortTermModel(); + 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; - mShortTermModelAnchor = lux; - if (mLoggingEnabled) { - Slog.d(TAG, "ShortTermModel: anchor=" + mShortTermModelAnchor); - } + mShortTermModel.setUserBrightness(lux, brightness); return true; } public void resetShortTermModel() { mCurrentBrightnessMapper.clearUserDataPoints(); - mShortTermModelValid = true; - mShortTermModelAnchor = -1; - } - - private void invalidateShortTermModel() { - if (mLoggingEnabled) { - Slog.d(TAG, "ShortTermModel: invalidate user data"); - } - mShortTermModelValid = false; + mShortTermModel.reset(); } public boolean setBrightnessConfiguration(BrightnessConfiguration configuration, @@ -595,8 +583,12 @@ public class AutomaticBrightnessController { pw.println(" mShortTermModelTimeout(idle)=" + mIdleModeBrightnessMapper.getShortTermModelTimeout()); } - pw.println(" mShortTermModelAnchor=" + mShortTermModelAnchor); - pw.println(" mShortTermModelValid=" + mShortTermModelValid); + pw.println(" mShortTermModel="); + 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) { - if (mCurrentBrightnessMapper.shouldResetShortTermModel( - mAmbientLux, mShortTermModelAnchor)) { - resetShortTermModel(); - } else { - mShortTermModelValid = true; - } - } + mShortTermModel.maybeReset(mAmbientLux); } 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: - invalidateShortTermModel(); + case MSG_INVALIDATE_CURRENT_SHORT_TERM_MODEL: + 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; } } } diff --git a/services/core/java/com/android/server/display/BrightnessMappingStrategy.java b/services/core/java/com/android/server/display/BrightnessMappingStrategy.java index d0471837d79b3..3456e3e262dec 100644 --- a/services/core/java/com/android/server/display/BrightnessMappingStrategy.java +++ b/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(); diff --git a/services/tests/servicestests/src/com/android/server/display/AutomaticBrightnessControllerTest.java b/services/tests/servicestests/src/com/android/server/display/AutomaticBrightnessControllerTest.java index fc251526476e1..3bef413a3e723 100644 --- a/services/tests/servicestests/src/com/android/server/display/AutomaticBrightnessControllerTest.java +++ b/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 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 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 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 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 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);