From f214018142ba6b29589338dfcfd331213252a76d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wilczy=C5=84ski?= Date: Mon, 26 Jun 2023 18:18:59 +0000 Subject: [PATCH] Clamp brightness before saving it If we save unclamped brightness, later we'll call ABC.configure with the unclamped value and the short-term model will be adjusted with the unclamped value which is wrong. Bug: 288368190 Test: DisplayPowerControllerTest Test: DisplayPowerController2Test Change-Id: I668b1fde3eef8a1721f3b14f16fec0c1eae3b04d --- .../display/DisplayPowerController.java | 21 ++++++++-- .../display/DisplayPowerController2.java | 20 ++++++++-- .../display/DisplayPowerController2Test.java | 38 +++++++++++++++++-- .../display/DisplayPowerControllerTest.java | 36 ++++++++++++++++-- 4 files changed, 100 insertions(+), 15 deletions(-) diff --git a/services/core/java/com/android/server/display/DisplayPowerController.java b/services/core/java/com/android/server/display/DisplayPowerController.java index 7d8bde9feabff..ffddf5acec758 100644 --- a/services/core/java/com/android/server/display/DisplayPowerController.java +++ b/services/core/java/com/android/server/display/DisplayPowerController.java @@ -2155,8 +2155,9 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call final DisplayDeviceConfig.HighBrightnessModeData hbmData = ddConfig != null ? ddConfig.getHighBrightnessModeData() : null; final DisplayDeviceInfo info = device.getDisplayDeviceInfoLocked(); - return new HighBrightnessModeController(mHandler, info.width, info.height, displayToken, - displayUniqueId, PowerManager.BRIGHTNESS_MIN, PowerManager.BRIGHTNESS_MAX, hbmData, + return mInjector.getHighBrightnessModeController(mHandler, info.width, info.height, + displayToken, displayUniqueId, PowerManager.BRIGHTNESS_MIN, + PowerManager.BRIGHTNESS_MAX, hbmData, new HighBrightnessModeController.HdrBrightnessDeviceConfig() { @Override public float getHdrBrightnessFromSdr( @@ -2667,10 +2668,11 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call public void setBrightness(float brightnessValue, int userSerial) { // Update the setting, which will eventually call back into DPC to have us actually update // the display with the new value. + float clampedBrightnessValue = clampScreenBrightness(brightnessValue); mBrightnessSetting.setUserSerial(userSerial); - mBrightnessSetting.setBrightness(brightnessValue); + mBrightnessSetting.setBrightness(clampedBrightnessValue); if (mDisplayId == Display.DEFAULT_DISPLAY && mPersistBrightnessNitsForDefaultDisplay) { - float nits = convertToNits(brightnessValue); + float nits = convertToNits(clampedBrightnessValue); if (nits >= 0) { mBrightnessSetting.setBrightnessNitsForDefaultDisplay(nits); } @@ -3526,6 +3528,17 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call brightnessMapper ); } + + HighBrightnessModeController getHighBrightnessModeController(Handler handler, int width, + int height, IBinder displayToken, String displayUniqueId, float brightnessMin, + float brightnessMax, DisplayDeviceConfig.HighBrightnessModeData hbmData, + HighBrightnessModeController.HdrBrightnessDeviceConfig hdrBrightnessCfg, + Runnable hbmChangeCallback, HighBrightnessModeMetadata hbmMetadata, + Context context) { + return new HighBrightnessModeController(handler, width, height, displayToken, + displayUniqueId, brightnessMin, brightnessMax, hbmData, hdrBrightnessCfg, + hbmChangeCallback, hbmMetadata, context); + } } static class CachedBrightnessInfo { diff --git a/services/core/java/com/android/server/display/DisplayPowerController2.java b/services/core/java/com/android/server/display/DisplayPowerController2.java index 040ceccbc7c9f..04c825441a837 100644 --- a/services/core/java/com/android/server/display/DisplayPowerController2.java +++ b/services/core/java/com/android/server/display/DisplayPowerController2.java @@ -1802,9 +1802,9 @@ final class DisplayPowerController2 implements AutomaticBrightnessController.Cal final DisplayDeviceConfig.HighBrightnessModeData hbmData = ddConfig != null ? ddConfig.getHighBrightnessModeData() : null; final DisplayDeviceInfo info = device.getDisplayDeviceInfoLocked(); - return new HighBrightnessModeController(mHandler, info.width, info.height, displayToken, - displayUniqueId, PowerManager.BRIGHTNESS_MIN, PowerManager.BRIGHTNESS_MAX, hbmData, - (sdrBrightness, maxDesiredHdrSdrRatio) -> + return mInjector.getHighBrightnessModeController(mHandler, info.width, info.height, + displayToken, displayUniqueId, PowerManager.BRIGHTNESS_MIN, + PowerManager.BRIGHTNESS_MAX, hbmData, (sdrBrightness, maxDesiredHdrSdrRatio) -> mDisplayDeviceConfig.getHdrBrightnessFromSdr(sdrBrightness, maxDesiredHdrSdrRatio), modeChangeCallback, hbmMetadata, mContext); } @@ -2173,7 +2173,8 @@ final class DisplayPowerController2 implements AutomaticBrightnessController.Cal @Override public void setBrightness(float brightnessValue, int userSerial) { - mDisplayBrightnessController.setBrightness(brightnessValue, userSerial); + mDisplayBrightnessController.setBrightness(clampScreenBrightness(brightnessValue), + userSerial); } @Override @@ -2890,6 +2891,17 @@ final class DisplayPowerController2 implements AutomaticBrightnessController.Cal brightnessMapper ); } + + HighBrightnessModeController getHighBrightnessModeController(Handler handler, int width, + int height, IBinder displayToken, String displayUniqueId, float brightnessMin, + float brightnessMax, DisplayDeviceConfig.HighBrightnessModeData hbmData, + HighBrightnessModeController.HdrBrightnessDeviceConfig hdrBrightnessCfg, + Runnable hbmChangeCallback, HighBrightnessModeMetadata hbmMetadata, + Context context) { + return new HighBrightnessModeController(handler, width, height, displayToken, + displayUniqueId, brightnessMin, brightnessMax, hbmData, hdrBrightnessCfg, + hbmChangeCallback, hbmMetadata, context); + } } static class CachedBrightnessInfo { diff --git a/services/tests/mockingservicestests/src/com/android/server/display/DisplayPowerController2Test.java b/services/tests/mockingservicestests/src/com/android/server/display/DisplayPowerController2Test.java index ed68fb9a4d544..45f36e1ac0c93 100644 --- a/services/tests/mockingservicestests/src/com/android/server/display/DisplayPowerController2Test.java +++ b/services/tests/mockingservicestests/src/com/android/server/display/DisplayPowerController2Test.java @@ -46,6 +46,7 @@ import android.hardware.SensorManager; import android.hardware.display.DisplayManagerInternal.DisplayPowerCallbacks; import android.hardware.display.DisplayManagerInternal.DisplayPowerRequest; import android.os.Handler; +import android.os.IBinder; import android.os.Looper; import android.os.PowerManager; import android.os.SystemProperties; @@ -838,6 +839,16 @@ public final class DisplayPowerController2Test { .getThermalBrightnessThrottlingDataMapByThrottlingId(); } + @Test + public void testSetBrightness_BrightnessShouldBeClamped() { + float clampedBrightness = 0.6f; + when(mHolder.hbmController.getCurrentBrightnessMax()).thenReturn(clampedBrightness); + + mHolder.dpc.setBrightness(PowerManager.BRIGHTNESS_MAX); + + verify(mHolder.brightnessSetting).setBrightness(clampedBrightness); + } + /** * Creates a mock and registers it to {@link LocalServices}. */ @@ -920,10 +931,13 @@ public final class DisplayPowerController2Test { final HysteresisLevels hysteresisLevels = mock(HysteresisLevels.class); final ScreenOffBrightnessSensorController screenOffBrightnessSensorController = mock(ScreenOffBrightnessSensorController.class); + final HighBrightnessModeController hbmController = mock(HighBrightnessModeController.class); + + when(hbmController.getCurrentBrightnessMax()).thenReturn(PowerManager.BRIGHTNESS_MAX); TestInjector injector = spy(new TestInjector(displayPowerState, animator, automaticBrightnessController, wakelockController, brightnessMappingStrategy, - hysteresisLevels, screenOffBrightnessSensorController)); + hysteresisLevels, screenOffBrightnessSensorController, hbmController)); final LogicalDisplay display = mock(LogicalDisplay.class); final DisplayDevice device = mock(DisplayDevice.class); @@ -941,8 +955,8 @@ public final class DisplayPowerController2Test { return new DisplayPowerControllerHolder(dpc, display, displayPowerState, brightnessSetting, animator, automaticBrightnessController, wakelockController, - screenOffBrightnessSensorController, hbmMetadata, brightnessMappingStrategy, - injector); + screenOffBrightnessSensorController, hbmController, hbmMetadata, + brightnessMappingStrategy, injector); } /** @@ -958,6 +972,7 @@ public final class DisplayPowerController2Test { public final AutomaticBrightnessController automaticBrightnessController; public final WakelockController wakelockController; public final ScreenOffBrightnessSensorController screenOffBrightnessSensorController; + public final HighBrightnessModeController hbmController; public final HighBrightnessModeMetadata hbmMetadata; public final BrightnessMappingStrategy brightnessMappingStrategy; public final DisplayPowerController2.Injector injector; @@ -968,6 +983,7 @@ public final class DisplayPowerController2Test { AutomaticBrightnessController automaticBrightnessController, WakelockController wakelockController, ScreenOffBrightnessSensorController screenOffBrightnessSensorController, + HighBrightnessModeController hbmController, HighBrightnessModeMetadata hbmMetadata, BrightnessMappingStrategy brightnessMappingStrategy, DisplayPowerController2.Injector injector) { @@ -979,6 +995,7 @@ public final class DisplayPowerController2Test { this.automaticBrightnessController = automaticBrightnessController; this.wakelockController = wakelockController; this.screenOffBrightnessSensorController = screenOffBrightnessSensorController; + this.hbmController = hbmController; this.hbmMetadata = hbmMetadata; this.brightnessMappingStrategy = brightnessMappingStrategy; this.injector = injector; @@ -993,13 +1010,15 @@ public final class DisplayPowerController2Test { private final BrightnessMappingStrategy mBrightnessMappingStrategy; private final HysteresisLevels mHysteresisLevels; private final ScreenOffBrightnessSensorController mScreenOffBrightnessSensorController; + private final HighBrightnessModeController mHighBrightnessModeController; TestInjector(DisplayPowerState dps, DualRampAnimator animator, AutomaticBrightnessController automaticBrightnessController, WakelockController wakelockController, BrightnessMappingStrategy brightnessMappingStrategy, HysteresisLevels hysteresisLevels, - ScreenOffBrightnessSensorController screenOffBrightnessSensorController) { + ScreenOffBrightnessSensorController screenOffBrightnessSensorController, + HighBrightnessModeController highBrightnessModeController) { mDisplayPowerState = dps; mAnimator = animator; mAutomaticBrightnessController = automaticBrightnessController; @@ -1007,6 +1026,7 @@ public final class DisplayPowerController2Test { mBrightnessMappingStrategy = brightnessMappingStrategy; mHysteresisLevels = hysteresisLevels; mScreenOffBrightnessSensorController = screenOffBrightnessSensorController; + mHighBrightnessModeController = highBrightnessModeController; } @Override @@ -1100,5 +1120,15 @@ public final class DisplayPowerController2Test { BrightnessMappingStrategy brightnessMapper) { return mScreenOffBrightnessSensorController; } + + @Override + HighBrightnessModeController getHighBrightnessModeController(Handler handler, int width, + int height, IBinder displayToken, String displayUniqueId, float brightnessMin, + float brightnessMax, DisplayDeviceConfig.HighBrightnessModeData hbmData, + HighBrightnessModeController.HdrBrightnessDeviceConfig hdrBrightnessCfg, + Runnable hbmChangeCallback, HighBrightnessModeMetadata hbmMetadata, + Context context) { + return mHighBrightnessModeController; + } } } diff --git a/services/tests/mockingservicestests/src/com/android/server/display/DisplayPowerControllerTest.java b/services/tests/mockingservicestests/src/com/android/server/display/DisplayPowerControllerTest.java index 95bc26ac99edf..fd62a2a34e8ac 100644 --- a/services/tests/mockingservicestests/src/com/android/server/display/DisplayPowerControllerTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/display/DisplayPowerControllerTest.java @@ -46,6 +46,7 @@ import android.hardware.SensorManager; import android.hardware.display.DisplayManagerInternal.DisplayPowerCallbacks; import android.hardware.display.DisplayManagerInternal.DisplayPowerRequest; import android.os.Handler; +import android.os.IBinder; import android.os.Looper; import android.os.PowerManager; import android.os.SystemProperties; @@ -844,6 +845,16 @@ public final class DisplayPowerControllerTest { .getThermalBrightnessThrottlingDataMapByThrottlingId(); } + @Test + public void testSetBrightness_BrightnessShouldBeClamped() { + float clampedBrightness = 0.6f; + when(mHolder.hbmController.getCurrentBrightnessMax()).thenReturn(clampedBrightness); + + mHolder.dpc.setBrightness(PowerManager.BRIGHTNESS_MAX); + + verify(mHolder.brightnessSetting).setBrightness(clampedBrightness); + } + /** * Creates a mock and registers it to {@link LocalServices}. */ @@ -925,10 +936,13 @@ public final class DisplayPowerControllerTest { final HysteresisLevels hysteresisLevels = mock(HysteresisLevels.class); final ScreenOffBrightnessSensorController screenOffBrightnessSensorController = mock(ScreenOffBrightnessSensorController.class); + final HighBrightnessModeController hbmController = mock(HighBrightnessModeController.class); + + when(hbmController.getCurrentBrightnessMax()).thenReturn(PowerManager.BRIGHTNESS_MAX); DisplayPowerController.Injector injector = spy(new TestInjector(displayPowerState, animator, automaticBrightnessController, brightnessMappingStrategy, hysteresisLevels, - screenOffBrightnessSensorController)); + screenOffBrightnessSensorController, hbmController)); final LogicalDisplay display = mock(LogicalDisplay.class); final DisplayDevice device = mock(DisplayDevice.class); @@ -946,7 +960,7 @@ public final class DisplayPowerControllerTest { return new DisplayPowerControllerHolder(dpc, display, displayPowerState, brightnessSetting, animator, automaticBrightnessController, screenOffBrightnessSensorController, - hbmMetadata, brightnessMappingStrategy, injector); + hbmController, hbmMetadata, brightnessMappingStrategy, injector); } /** @@ -961,6 +975,7 @@ public final class DisplayPowerControllerTest { public final DualRampAnimator animator; public final AutomaticBrightnessController automaticBrightnessController; public final ScreenOffBrightnessSensorController screenOffBrightnessSensorController; + public final HighBrightnessModeController hbmController; public final HighBrightnessModeMetadata hbmMetadata; public final BrightnessMappingStrategy brightnessMappingStrategy; public final DisplayPowerController.Injector injector; @@ -970,6 +985,7 @@ public final class DisplayPowerControllerTest { DualRampAnimator animator, AutomaticBrightnessController automaticBrightnessController, ScreenOffBrightnessSensorController screenOffBrightnessSensorController, + HighBrightnessModeController hbmController, HighBrightnessModeMetadata hbmMetadata, BrightnessMappingStrategy brightnessMappingStrategy, DisplayPowerController.Injector injector) { @@ -980,6 +996,7 @@ public final class DisplayPowerControllerTest { this.animator = animator; this.automaticBrightnessController = automaticBrightnessController; this.screenOffBrightnessSensorController = screenOffBrightnessSensorController; + this.hbmController = hbmController; this.hbmMetadata = hbmMetadata; this.brightnessMappingStrategy = brightnessMappingStrategy; this.injector = injector; @@ -993,18 +1010,21 @@ public final class DisplayPowerControllerTest { private final BrightnessMappingStrategy mBrightnessMappingStrategy; private final HysteresisLevels mHysteresisLevels; private final ScreenOffBrightnessSensorController mScreenOffBrightnessSensorController; + private final HighBrightnessModeController mHighBrightnessModeController; TestInjector(DisplayPowerState dps, DualRampAnimator animator, AutomaticBrightnessController automaticBrightnessController, BrightnessMappingStrategy brightnessMappingStrategy, HysteresisLevels hysteresisLevels, - ScreenOffBrightnessSensorController screenOffBrightnessSensorController) { + ScreenOffBrightnessSensorController screenOffBrightnessSensorController, + HighBrightnessModeController highBrightnessModeController) { mDisplayPowerState = dps; mAnimator = animator; mAutomaticBrightnessController = automaticBrightnessController; mBrightnessMappingStrategy = brightnessMappingStrategy; mHysteresisLevels = hysteresisLevels; mScreenOffBrightnessSensorController = screenOffBrightnessSensorController; + mHighBrightnessModeController = highBrightnessModeController; } @Override @@ -1076,5 +1096,15 @@ public final class DisplayPowerControllerTest { BrightnessMappingStrategy brightnessMapper) { return mScreenOffBrightnessSensorController; } + + @Override + HighBrightnessModeController getHighBrightnessModeController(Handler handler, int width, + int height, IBinder displayToken, String displayUniqueId, float brightnessMin, + float brightnessMax, DisplayDeviceConfig.HighBrightnessModeData hbmData, + HighBrightnessModeController.HdrBrightnessDeviceConfig hdrBrightnessCfg, + Runnable hbmChangeCallback, HighBrightnessModeMetadata hbmMetadata, + Context context) { + return mHighBrightnessModeController; + } } }