From 25fd67372ebc10a03ba587332687b596affdfcf2 Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Wed, 7 Jul 2021 15:04:59 +0100 Subject: [PATCH] Use exact brightnesses values for comparison. Using BrightnessSynchronizer introduced an epsilon value in the comparison. This meant it was possible for brightness changes to never reach min or max if they were made in sufficiently small steps. Given: 1) The need to reach both min and max brightnesses. 2) The increased resolution of brightness controls on devices. 3) The desire to make the framework as independent as possible of brightness resolution. we no longer allow for an epsilon delta. Fixes: 191935286 Test: manual Change-Id: Ie4872fa81aa6f23799cd6b4dfdbfd2a61ec6ce7a --- .../server/display/DisplayManagerService.java | 6 ++--- .../display/DisplayPowerController.java | 20 ++++++++-------- .../server/display/DisplayPowerState.java | 24 ++++++++----------- .../server/display/LocalDisplayAdapter.java | 12 ++++------ .../android/server/display/RampAnimator.java | 6 ++--- 5 files changed, 29 insertions(+), 39 deletions(-) diff --git a/services/core/java/com/android/server/display/DisplayManagerService.java b/services/core/java/com/android/server/display/DisplayManagerService.java index 79ea108a75d54..fce52bef45f7d 100644 --- a/services/core/java/com/android/server/display/DisplayManagerService.java +++ b/services/core/java/com/android/server/display/DisplayManagerService.java @@ -708,10 +708,8 @@ public final class DisplayManagerService extends SystemService { final BrightnessPair brightnessPair = index < 0 ? null : mDisplayBrightnesses.valueAt(index); if (index < 0 || (mDisplayStates.valueAt(index) == state - && BrightnessSynchronizer.floatEquals( - brightnessPair.brightness, brightnessState) - && BrightnessSynchronizer.floatEquals( - brightnessPair.sdrBrightness, sdrBrightnessState))) { + && brightnessPair.brightness == brightnessState + && brightnessPair.sdrBrightness == sdrBrightnessState)) { return; // Display no longer exists or no change. } diff --git a/services/core/java/com/android/server/display/DisplayPowerController.java b/services/core/java/com/android/server/display/DisplayPowerController.java index 8396f664146df..f7b6e8a9e0d93 100644 --- a/services/core/java/com/android/server/display/DisplayPowerController.java +++ b/services/core/java/com/android/server/display/DisplayPowerController.java @@ -1341,9 +1341,8 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call final float currentBrightness = mPowerState.getScreenBrightness(); final float currentSdrBrightness = mPowerState.getSdrScreenBrightness(); if (isValidBrightnessValue(animateValue) - && (!BrightnessSynchronizer.floatEquals(animateValue, currentBrightness) - || !BrightnessSynchronizer.floatEquals( - sdrAnimateValue, currentSdrBrightness))) { + && (animateValue != currentBrightness + || sdrAnimateValue != currentSdrBrightness)) { if (initialRampSkip || hasBrightnessBuckets || wasOrWillBeInVr || !isDisplayContentVisible || brightnessIsTemporary) { animateScreenBrightness(animateValue, sdrAnimateValue, @@ -1672,11 +1671,10 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call mHbmController.getCurrentBrightnessMin(), mHbmController.getCurrentBrightnessMax()); } - // Checks whether the brightness is within the valid brightness range, not including the off or - // invalid states. - private boolean isValidBrightnessValue(float brightnessState) { - return brightnessState >= PowerManager.BRIGHTNESS_MIN - && brightnessState <= PowerManager.BRIGHTNESS_MAX; + // Checks whether the brightness is within the valid brightness range, not including off. + private boolean isValidBrightnessValue(float brightness) { + return brightness >= PowerManager.BRIGHTNESS_MIN + && brightness <= PowerManager.BRIGHTNESS_MAX; } private void animateScreenBrightness(float target, float sdrTarget, float rate) { @@ -2006,6 +2004,9 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call } private void putScreenBrightnessSetting(float brightnessValue, boolean updateCurrent) { + if (!isValidBrightnessValue(brightnessValue)) { + return; + } if (updateCurrent) { setCurrentScreenBrightness(brightnessValue); } @@ -2046,8 +2047,7 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call || mPendingScreenBrightnessSetting < 0.0f)) { return false; } - if (BrightnessSynchronizer.floatEquals( - mCurrentScreenBrightnessSetting, mPendingScreenBrightnessSetting)) { + if (mCurrentScreenBrightnessSetting == mPendingScreenBrightnessSetting) { mPendingScreenBrightnessSetting = PowerManager.BRIGHTNESS_INVALID_FLOAT; mTemporaryScreenBrightness = PowerManager.BRIGHTNESS_INVALID_FLOAT; return false; diff --git a/services/core/java/com/android/server/display/DisplayPowerState.java b/services/core/java/com/android/server/display/DisplayPowerState.java index b58dd38348aa0..6af192371e3da 100644 --- a/services/core/java/com/android/server/display/DisplayPowerState.java +++ b/services/core/java/com/android/server/display/DisplayPowerState.java @@ -26,8 +26,6 @@ import android.util.Slog; import android.view.Choreographer; import android.view.Display; -import com.android.internal.display.BrightnessSynchronizer; - import java.io.PrintWriter; /** @@ -166,10 +164,11 @@ final class DisplayPowerState { /** * Sets the display's SDR brightness. * - * @param brightness The brightness, ranges from 0.0f (minimum / off) to 1.0f (brightest). + * @param brightness The brightness, ranges from 0.0f (minimum) to 1.0f (brightest), or is -1f + * (off). */ public void setSdrScreenBrightness(float brightness) { - if (!BrightnessSynchronizer.floatEquals(mSdrScreenBrightness, brightness)) { + if (mSdrScreenBrightness != brightness) { if (DEBUG) { Slog.d(TAG, "setSdrScreenBrightness: brightness=" + brightness); } @@ -192,10 +191,11 @@ final class DisplayPowerState { /** * Sets the display brightness. * - * @param brightness The brightness, ranges from 0.0f (minimum / off) to 1.0f (brightest). + * @param brightness The brightness, ranges from 0.0f (minimum) to 1.0f (brightest), or is -1f + * (off). */ public void setScreenBrightness(float brightness) { - if (!BrightnessSynchronizer.floatEquals(mScreenBrightness, brightness)) { + if (mScreenBrightness != brightness) { if (DEBUG) { Slog.d(TAG, "setScreenBrightness: brightness=" + brightness); } @@ -432,10 +432,8 @@ final class DisplayPowerState { public boolean setState(int state, float brightnessState, float sdrBrightnessState) { synchronized (mLock) { boolean stateChanged = state != mPendingState; - boolean backlightChanged = - !BrightnessSynchronizer.floatEquals(brightnessState, mPendingBacklight) - || !BrightnessSynchronizer.floatEquals( - sdrBrightnessState, mPendingSdrBacklight); + boolean backlightChanged = brightnessState != mPendingBacklight + || sdrBrightnessState != mPendingSdrBacklight; if (stateChanged || backlightChanged) { if (DEBUG) { Slog.d(TAG, "Requesting new screen state: state=" @@ -486,10 +484,8 @@ final class DisplayPowerState { stateChanged = (state != mActualState); brightnessState = mPendingBacklight; sdrBrightnessState = mPendingSdrBacklight; - backlightChanged = - !BrightnessSynchronizer.floatEquals(brightnessState, mActualBacklight) - || !BrightnessSynchronizer.floatEquals( - sdrBrightnessState, mActualSdrBacklight); + backlightChanged = brightnessState != mActualBacklight + || sdrBrightnessState != mActualSdrBacklight; if (!stateChanged) { // State changed applied, notify outer class. postScreenUpdateThreadSafe(); diff --git a/services/core/java/com/android/server/display/LocalDisplayAdapter.java b/services/core/java/com/android/server/display/LocalDisplayAdapter.java index 2f17481f74876..f953cc8c8a271 100644 --- a/services/core/java/com/android/server/display/LocalDisplayAdapter.java +++ b/services/core/java/com/android/server/display/LocalDisplayAdapter.java @@ -648,12 +648,11 @@ final class LocalDisplayAdapter extends DisplayAdapter { public Runnable requestDisplayStateLocked(final int state, final float brightnessState, final float sdrBrightnessState) { // Assume that the brightness is off if the display is being turned off. - assert state != Display.STATE_OFF || BrightnessSynchronizer.floatEquals( - brightnessState, PowerManager.BRIGHTNESS_OFF_FLOAT); + assert state != Display.STATE_OFF + || brightnessState == PowerManager.BRIGHTNESS_OFF_FLOAT; final boolean stateChanged = (mState != state); - final boolean brightnessChanged = - !(BrightnessSynchronizer.floatEquals(mBrightnessState, brightnessState) - && BrightnessSynchronizer.floatEquals(mSdrBrightnessState, sdrBrightnessState)); + final boolean brightnessChanged = mBrightnessState != brightnessState + || mSdrBrightnessState != sdrBrightnessState; if (stateChanged || brightnessChanged) { final long physicalDisplayId = mPhysicalDisplayId; final IBinder token = getDisplayTokenLocked(); @@ -807,8 +806,7 @@ final class LocalDisplayAdapter extends DisplayAdapter { } private float brightnessToBacklight(float brightness) { - if (BrightnessSynchronizer.floatEquals( - brightness, PowerManager.BRIGHTNESS_OFF_FLOAT)) { + if (brightness == PowerManager.BRIGHTNESS_OFF_FLOAT) { return PowerManager.BRIGHTNESS_OFF_FLOAT; } else { return getDisplayDeviceConfig().getBacklightFromBrightness(brightness); diff --git a/services/core/java/com/android/server/display/RampAnimator.java b/services/core/java/com/android/server/display/RampAnimator.java index 20feafa2d19cc..ed3b15fb26619 100644 --- a/services/core/java/com/android/server/display/RampAnimator.java +++ b/services/core/java/com/android/server/display/RampAnimator.java @@ -20,8 +20,6 @@ import android.animation.ValueAnimator; import android.util.FloatProperty; import android.view.Choreographer; -import com.android.internal.display.BrightnessSynchronizer; - /** * A custom animator that progressively updates a property value at * a given variable rate until it reaches a particular target value. @@ -157,10 +155,10 @@ class RampAnimator { } final float oldCurrentValue = mCurrentValue; mCurrentValue = mAnimatedValue; - if (!BrightnessSynchronizer.floatEquals(oldCurrentValue, mCurrentValue)) { + if (oldCurrentValue != mCurrentValue) { mProperty.setValue(mObject, mCurrentValue); } - if (!BrightnessSynchronizer.floatEquals(mTargetValue, mCurrentValue)) { + if (mTargetValue != mCurrentValue) { postAnimationCallback(); } else { mAnimating = false;