From 0db446adb4b2393507a2a6ec0ab9ff001d08e0b0 Mon Sep 17 00:00:00 2001 From: Santos Cordon Date: Fri, 16 Jul 2021 11:49:13 +0100 Subject: [PATCH] Fix HDR for app-override brightness. - HBMController no longer listens to changes on the brightness setting directly, and instead gets brightness updates from DPC. This was necessary since some of the brightness-sources (such as app-override) are not reflected in the actual setting. - Clamp override and temporary brightness sources so that they cannot push brightness beyond the bounds set by HbmController. - Always save away the actual brightness setting in BrightnessInfo cache, ignoring any temporary or override brightness values. This ensures that Display.getBrightnessInfo() reflects the user setting, and not some temporary brightness state. Bug: 193392737 Test: manually test HDR with app-override Test: atest HighBrightnessModeControllerTest Change-Id: Iffe777a22d058ddb779dc6fc22c2382f6bbac1ab --- .../display/DisplayPowerController.java | 30 +++++++++------- .../display/HighBrightnessModeController.java | 36 +++++++------------ .../HighBrightnessModeControllerTest.java | 7 ++-- 3 files changed, 33 insertions(+), 40 deletions(-) diff --git a/services/core/java/com/android/server/display/DisplayPowerController.java b/services/core/java/com/android/server/display/DisplayPowerController.java index b6d65197d8579..f82c7e9fe6f12 100644 --- a/services/core/java/com/android/server/display/DisplayPowerController.java +++ b/services/core/java/com/android/server/display/DisplayPowerController.java @@ -776,7 +776,7 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call setUpAutoBrightness(mContext.getResources(), mHandler); reloadReduceBrightColours(); mHbmController.resetHbmData(info.width, info.height, token, - mDisplayDeviceConfig.getHighBrightnessModeData(), mBrightnessSetting); + mDisplayDeviceConfig.getHighBrightnessModeData()); } private void sendUpdatePowerState() { @@ -966,7 +966,6 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call final boolean mustNotify; final int previousPolicy; boolean mustInitialize = false; - boolean shouldSaveBrightnessInfo = true; int brightnessAdjustmentFlags = 0; mBrightnessReasonTemp.set(null); synchronized (mLock) { @@ -1097,7 +1096,6 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call if (state == Display.STATE_OFF) { brightnessState = PowerManager.BRIGHTNESS_OFF_FLOAT; mBrightnessReasonTemp.setReason(BrightnessReason.REASON_SCREEN_OFF); - shouldSaveBrightnessInfo = false; } // Always use the VR brightness when in the VR state. @@ -1215,6 +1213,10 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call brightnessAdjustmentFlags = 0; } } else { + // Any non-auto-brightness values such as override or temporary should still be subject + // to clamping so that they don't go beyond the current max as specified by HBM + // Controller. + brightnessState = clampScreenBrightness(brightnessState); mAppliedAutoBrightness = false; brightnessAdjustmentFlags = 0; } @@ -1222,9 +1224,8 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call // Use default brightness when dozing unless overridden. if ((Float.isNaN(brightnessState)) && Display.isDozeState(state)) { - brightnessState = mScreenBrightnessDozeConfig; + brightnessState = clampScreenBrightness(mScreenBrightnessDozeConfig); mBrightnessReasonTemp.setReason(BrightnessReason.REASON_DOZE_DEFAULT); - shouldSaveBrightnessInfo = false; } // Apply manual brightness. @@ -1239,12 +1240,13 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call mBrightnessReasonTemp.setReason(BrightnessReason.REASON_MANUAL); } - // Save out the brightness info now that the brightness state for this iteration has been - // finalized and before we send out notifications about the brightness changing. - if (shouldSaveBrightnessInfo) { - saveBrightnessInfo(brightnessState); - - } + // The current brightness to use has been calculated at this point (minus the adjustments + // like low-power and dim), and HbmController should be notified so that it can accurately + // calculate HDR or HBM levels. We specifically do it here instead of having HbmController + // listen to the brightness setting because certain brightness sources (just as an app + // override) are not saved to the setting, but should be reflected in HBM + // calculations. + mHbmController.onBrightnessChanged(brightnessState); if (updateScreenBrightnessSetting) { // Tell the rest of the system about the new brightness in case we had to change it @@ -1255,6 +1257,10 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call putScreenBrightnessSetting(brightnessState, /* updateCurrent */ true); } + // We save the brightness info *after* the brightness setting has been changed so that + // the brightness info reflects the latest value. + saveBrightnessInfo(getScreenBrightnessSetting()); + // Apply dimming by at least some minimum amount when user activity // timeout is about to expire. if (mPowerRequest.policy == DisplayPowerRequest.POLICY_DIM) { @@ -1531,7 +1537,7 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call mHandler.post(mOnBrightnessChangeRunnable); // TODO(b/192258832): Switch the HBMChangeCallback to a listener pattern. mAutomaticBrightnessController.update(); - }, mContext, mBrightnessSetting); + }, mContext); } private void blockScreenOn() { diff --git a/services/core/java/com/android/server/display/HighBrightnessModeController.java b/services/core/java/com/android/server/display/HighBrightnessModeController.java index 645131c1eee8a..2791f6a409be3 100644 --- a/services/core/java/com/android/server/display/HighBrightnessModeController.java +++ b/services/core/java/com/android/server/display/HighBrightnessModeController.java @@ -37,7 +37,6 @@ import android.util.TimeUtils; import android.view.SurfaceControlHdrLayerInfoListener; import com.android.internal.annotations.VisibleForTesting; -import com.android.server.display.BrightnessSetting.BrightnessSettingListener; import com.android.server.display.DisplayDeviceConfig.HighBrightnessModeData; import com.android.server.display.DisplayManagerService.Clock; @@ -70,7 +69,6 @@ class HighBrightnessModeController { private final Context mContext; private final SettingsObserver mSettingsObserver; private final Injector mInjector; - private final BrightnessSettingListener mBrightnessSettingListener = this::onBrightnessChanged; private HdrListener mHdrListener; private HighBrightnessModeData mHbmData; @@ -86,7 +84,6 @@ class HighBrightnessModeController { private boolean mIsBlockedByLowPowerMode = false; private int mWidth; private int mHeight; - private BrightnessSetting mBrightnessSetting; private float mAmbientLux; /** @@ -103,30 +100,30 @@ class HighBrightnessModeController { HighBrightnessModeController(Handler handler, int width, int height, IBinder displayToken, float brightnessMin, float brightnessMax, HighBrightnessModeData hbmData, - Runnable hbmChangeCallback, Context context, BrightnessSetting brightnessSetting) { + Runnable hbmChangeCallback, Context context) { this(new Injector(), handler, width, height, displayToken, brightnessMin, brightnessMax, - hbmData, hbmChangeCallback, context, brightnessSetting); + hbmData, hbmChangeCallback, context); } @VisibleForTesting HighBrightnessModeController(Injector injector, Handler handler, int width, int height, IBinder displayToken, float brightnessMin, float brightnessMax, HighBrightnessModeData hbmData, Runnable hbmChangeCallback, - Context context, BrightnessSetting brightnessSetting) { + Context context) { mInjector = injector; mContext = context; mClock = injector.getClock(); mHandler = handler; + mBrightness = brightnessMin; mBrightnessMin = brightnessMin; mBrightnessMax = brightnessMax; - mBrightness = brightnessSetting.getBrightness(); mHbmChangeCallback = hbmChangeCallback; mSkinThermalStatusObserver = new SkinThermalStatusObserver(mInjector, mHandler); mSettingsObserver = new SettingsObserver(mHandler); mRecalcRunnable = this::recalculateTimeAllowance; mHdrListener = new HdrListener(); - resetHbmData(width, height, displayToken, hbmData, brightnessSetting); + resetHbmData(width, height, displayToken, hbmData); } void setAutoBrightnessEnabled(boolean isEnabled) { @@ -185,7 +182,6 @@ class HighBrightnessModeController { } } - @VisibleForTesting void onBrightnessChanged(float brightness) { if (!deviceSupportsHbm()) { return; @@ -224,12 +220,11 @@ class HighBrightnessModeController { mSettingsObserver.stopObserving(); } - void resetHbmData(int width, int height, IBinder displayToken, HighBrightnessModeData hbmData, - BrightnessSetting brightnessSetting) { + void resetHbmData(int width, int height, IBinder displayToken, HighBrightnessModeData hbmData) { mWidth = width; mHeight = height; mHbmData = hbmData; - resetBrightnessSetting(brightnessSetting); + unregisterHdrListener(); mSkinThermalStatusObserver.stopObserving(); mSettingsObserver.stopObserving(); @@ -261,9 +256,12 @@ class HighBrightnessModeController { pw.println(" mBrightness=" + mBrightness); pw.println(" mCurrentMin=" + getCurrentBrightnessMin()); pw.println(" mCurrentMax=" + getCurrentBrightnessMax()); - pw.println(" mHbmMode=" + BrightnessInfo.hbmToString(mHbmMode)); + pw.println(" mHbmMode=" + BrightnessInfo.hbmToString(mHbmMode) + + (mHbmMode == BrightnessInfo.HIGH_BRIGHTNESS_MODE_HDR + ? "(" + getHdrBrightnessValue() + ")" : "")); pw.println(" mHbmData=" + mHbmData); - pw.println(" mAmbientLux=" + mAmbientLux); + pw.println(" mAmbientLux=" + mAmbientLux + + (mIsAutoBrightnessEnabled ? "" : " (old/invalid)")); pw.println(" mIsInAllowedAmbientRange=" + mIsInAllowedAmbientRange); pw.println(" mIsAutoBrightnessEnabled=" + mIsAutoBrightnessEnabled); pw.println(" mIsHdrLayerPresent=" + mIsHdrLayerPresent); @@ -301,16 +299,6 @@ class HighBrightnessModeController { return event.startTimeMillis; } - private void resetBrightnessSetting(BrightnessSetting brightnessSetting) { - if (mBrightnessSetting != null) { - mBrightnessSetting.unregisterListener(mBrightnessSettingListener); - } - mBrightnessSetting = brightnessSetting; - if (mBrightnessSetting != null) { - mBrightnessSetting.registerListener(mBrightnessSettingListener); - } - } - private boolean isCurrentlyAllowed() { // Returns true if HBM is allowed (above the ambient lux threshold) and there's still // time within the current window for additional HBM usage. We return false if there is an diff --git a/services/tests/servicestests/src/com/android/server/display/HighBrightnessModeControllerTest.java b/services/tests/servicestests/src/com/android/server/display/HighBrightnessModeControllerTest.java index 1ad8850a19212..cc3591c895605 100644 --- a/services/tests/servicestests/src/com/android/server/display/HighBrightnessModeControllerTest.java +++ b/services/tests/servicestests/src/com/android/server/display/HighBrightnessModeControllerTest.java @@ -92,7 +92,6 @@ public class HighBrightnessModeControllerTest { @Mock IThermalService mThermalServiceMock; @Mock Injector mInjectorMock; - @Mock BrightnessSetting mBrightnessSetting; @Captor ArgumentCaptor mThermalEventListenerCaptor; @@ -123,7 +122,7 @@ public class HighBrightnessModeControllerTest { initHandler(null); final HighBrightnessModeController hbmc = new HighBrightnessModeController( mInjectorMock, mHandler, DISPLAY_WIDTH, DISPLAY_HEIGHT, mDisplayToken, DEFAULT_MIN, - DEFAULT_MAX, null, () -> {}, mContextSpy, mBrightnessSetting); + DEFAULT_MAX, null, () -> {}, mContextSpy); assertState(hbmc, DEFAULT_MIN, DEFAULT_MAX, HIGH_BRIGHTNESS_MODE_OFF); } @@ -132,7 +131,7 @@ public class HighBrightnessModeControllerTest { initHandler(null); final HighBrightnessModeController hbmc = new HighBrightnessModeController( mInjectorMock, mHandler, DISPLAY_WIDTH, DISPLAY_HEIGHT, mDisplayToken, DEFAULT_MIN, - DEFAULT_MAX, null, () -> {}, mContextSpy, mBrightnessSetting); + DEFAULT_MAX, null, () -> {}, mContextSpy); hbmc.setAutoBrightnessEnabled(true); hbmc.onAmbientLuxChange(MINIMUM_LUX - 1); // below allowed range assertState(hbmc, DEFAULT_MIN, DEFAULT_MAX, HIGH_BRIGHTNESS_MODE_OFF); @@ -464,7 +463,7 @@ public class HighBrightnessModeControllerTest { initHandler(clock); return new HighBrightnessModeController(mInjectorMock, mHandler, DISPLAY_WIDTH, DISPLAY_HEIGHT, mDisplayToken, DEFAULT_MIN, DEFAULT_MAX, DEFAULT_HBM_DATA, () -> {}, - mContextSpy, mBrightnessSetting); + mContextSpy); } private void initHandler(OffsettableClock clock) {