From 5b6150c9066ff6e1f5d37130d21394eda3851459 Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Sat, 5 Feb 2022 00:05:23 +0000 Subject: [PATCH 1/4] Add hashCode method to BrightnessReason It implements equals, so it almost certainly should implement hashCode. Bug: 217923092 Test: manual Change-Id: I6acaea66d97231bd515df8128a7abd4aa067d599 --- .../android/server/display/DisplayPowerController.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/display/DisplayPowerController.java b/services/core/java/com/android/server/display/DisplayPowerController.java index d71e07ab573f2..f785da3b7377b 100644 --- a/services/core/java/com/android/server/display/DisplayPowerController.java +++ b/services/core/java/com/android/server/display/DisplayPowerController.java @@ -75,6 +75,7 @@ import com.android.server.display.whitebalance.DisplayWhiteBalanceSettings; import com.android.server.policy.WindowManagerPolicy; import java.io.PrintWriter; +import java.util.Objects; /** * Controls the power state of the display. @@ -2824,13 +2825,18 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call @Override public boolean equals(Object obj) { - if (obj == null || !(obj instanceof BrightnessReason)) { + if (!(obj instanceof BrightnessReason)) { return false; } BrightnessReason other = (BrightnessReason) obj; return other.reason == reason && other.modifier == modifier; } + @Override + public int hashCode() { + return Objects.hash(reason, modifier); + } + @Override public String toString() { return toString(0); From 220a254e024bc4e3b6c6fcc12c4c4658160793dd Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Sat, 5 Feb 2022 00:14:04 +0000 Subject: [PATCH 2/4] Replace comments with @GuardedBy annotations. Also fix some IDE / errorprone warnings while I'm here. Bug: 217923092 Test: manual Change-Id: Ic05ee5b2591af15592aadc6745f6890c1ddd6167 --- .../display/DisplayPowerController.java | 50 +++++++++---------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/services/core/java/com/android/server/display/DisplayPowerController.java b/services/core/java/com/android/server/display/DisplayPowerController.java index f785da3b7377b..418e91dd616e4 100644 --- a/services/core/java/com/android/server/display/DisplayPowerController.java +++ b/services/core/java/com/android/server/display/DisplayPowerController.java @@ -237,42 +237,42 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call // True if we should fade the screen while turning it off, false if we should play // a stylish color fade animation instead. - private boolean mColorFadeFadesConfig; + private final boolean mColorFadeFadesConfig; // True if we need to fake a transition to off when coming out of a doze state. // Some display hardware will blank itself when coming out of doze in order to hide // artifacts. For these displays we fake a transition into OFF so that policy can appropriately // blank itself and begin an appropriate power on animation. - private boolean mDisplayBlanksAfterDozeConfig; + private final boolean mDisplayBlanksAfterDozeConfig; // True if there are only buckets of brightness values when the display is in the doze state, // rather than a full range of values. If this is true, then we'll avoid animating the screen // brightness since it'd likely be multiple jarring brightness transitions instead of just one // to reach the final state. - private boolean mBrightnessBucketsInDozeConfig; + private final boolean mBrightnessBucketsInDozeConfig; // The pending power request. // Initially null until the first call to requestPowerState. - // Guarded by mLock. + @GuardedBy("mLock") private DisplayPowerRequest mPendingRequestLocked; // True if a request has been made to wait for the proximity sensor to go negative. - // Guarded by mLock. + @GuardedBy("mLock") private boolean mPendingWaitForNegativeProximityLocked; // True if the pending power request or wait for negative proximity flag // has been changed since the last update occurred. - // Guarded by mLock. + @GuardedBy("mLock") private boolean mPendingRequestChangedLocked; // Set to true when the important parts of the pending power request have been applied. // The important parts are mainly the screen state. Brightness changes may occur // concurrently. - // Guarded by mLock. + @GuardedBy("mLock") private boolean mDisplayReadyLocked; // Set to true if a power state update is required. - // Guarded by mLock. + @GuardedBy("mLock") private boolean mPendingUpdatePowerStateLocked; /* The following state must only be accessed by the handler thread. */ @@ -353,8 +353,8 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call // information. // At the time of this writing, this value is changed within updatePowerState() only, which is // limited to the thread used by DisplayControllerHandler. - private BrightnessReason mBrightnessReason = new BrightnessReason(); - private BrightnessReason mBrightnessReasonTemp = new BrightnessReason(); + private final BrightnessReason mBrightnessReason = new BrightnessReason(); + private final BrightnessReason mBrightnessReasonTemp = new BrightnessReason(); // Brightness animation ramp rates in brightness units per second private float mBrightnessRampRateFastDecrease; @@ -850,6 +850,7 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call } } + @GuardedBy("mLock") private void sendUpdatePowerStateLocked() { if (!mStopped && !mPendingUpdatePowerStateLocked) { mPendingUpdatePowerStateLocked = true; @@ -2319,6 +2320,7 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call return mAutomaticBrightnessController.convertToNits(brightness); } + @GuardedBy("mLock") private void updatePendingProximityRequestsLocked() { mWaitingForNegativeProximity |= mPendingWaitForNegativeProximityLocked; mPendingWaitForNegativeProximityLocked = false; @@ -2422,12 +2424,7 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call pw.println(" mDisplayBlanksAfterDozeConfig=" + mDisplayBlanksAfterDozeConfig); pw.println(" mBrightnessBucketsInDozeConfig=" + mBrightnessBucketsInDozeConfig); - mHandler.runWithScissors(new Runnable() { - @Override - public void run() { - dumpLocal(pw); - } - }, 1000); + mHandler.runWithScissors(() -> dumpLocal(pw), 1000); } private void dumpLocal(PrintWriter pw) { @@ -2459,6 +2456,9 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call pw.println(" mAppliedThrottling=" + mAppliedThrottling); pw.println(" mAppliedScreenBrightnessOverride=" + mAppliedScreenBrightnessOverride); pw.println(" mAppliedTemporaryBrightness=" + mAppliedTemporaryBrightness); + pw.println(" mAppliedTemporaryAutoBrightnessAdjustment=" + + mAppliedTemporaryAutoBrightnessAdjustment); + pw.println(" mAppliedBrightnessBoost=" + mAppliedBrightnessBoost); pw.println(" mDozing=" + mDozing); pw.println(" mSkipRampState=" + skipRampStateToString(mSkipRampState)); pw.println(" mScreenOnBlockStartRealTime=" + mScreenOnBlockStartRealTime); @@ -2466,21 +2466,21 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call pw.println(" mPendingScreenOnUnblocker=" + mPendingScreenOnUnblocker); pw.println(" mPendingScreenOffUnblocker=" + mPendingScreenOffUnblocker); pw.println(" mPendingScreenOff=" + mPendingScreenOff); - pw.println(" mReportedToPolicy=" + - reportedToPolicyToString(mReportedScreenStateToPolicy)); + pw.println(" mReportedToPolicy=" + + reportedToPolicyToString(mReportedScreenStateToPolicy)); if (mScreenBrightnessRampAnimator != null) { - pw.println(" mScreenBrightnessRampAnimator.isAnimating()=" + - mScreenBrightnessRampAnimator.isAnimating()); + pw.println(" mScreenBrightnessRampAnimator.isAnimating()=" + + mScreenBrightnessRampAnimator.isAnimating()); } if (mColorFadeOnAnimator != null) { - pw.println(" mColorFadeOnAnimator.isStarted()=" + - mColorFadeOnAnimator.isStarted()); + pw.println(" mColorFadeOnAnimator.isStarted()=" + + mColorFadeOnAnimator.isStarted()); } if (mColorFadeOffAnimator != null) { - pw.println(" mColorFadeOffAnimator.isStarted()=" + - mColorFadeOffAnimator.isStarted()); + pw.println(" mColorFadeOffAnimator.isStarted()=" + + mColorFadeOffAnimator.isStarted()); } if (mPowerState != null) { @@ -2606,7 +2606,7 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call } } - private final void logHbmBrightnessStats(float brightness, int displayStatsId) { + private void logHbmBrightnessStats(float brightness, int displayStatsId) { synchronized (mHandler) { FrameworkStatsLog.write( FrameworkStatsLog.DISPLAY_HBM_BRIGHTNESS_CHANGED, displayStatsId, brightness); From 9cd2358c94f3787d31d399227e15e188859d7dc1 Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Sat, 5 Feb 2022 00:25:57 +0000 Subject: [PATCH 3/4] Stop shadowing DisplayDeviceConfig. It's already a member on DisplayDevice, so shadowing it with a different reference on the subclass makes it incredibly error prone. Bug: 217923092 Test: manual Change-Id: I64f8681058db6966b9066a538d6f83aeb8ddadf4 --- .../server/display/LocalDisplayAdapter.java | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/services/core/java/com/android/server/display/LocalDisplayAdapter.java b/services/core/java/com/android/server/display/LocalDisplayAdapter.java index a31c2314bd1ff..03fca30a9662d 100644 --- a/services/core/java/com/android/server/display/LocalDisplayAdapter.java +++ b/services/core/java/com/android/server/display/LocalDisplayAdapter.java @@ -182,8 +182,11 @@ final class LocalDisplayAdapter extends DisplayAdapter { private final long mPhysicalDisplayId; private final SparseArray mSupportedModes = new SparseArray<>(); private final ArrayList mSupportedColorModes = new ArrayList<>(); + private final DisplayModeDirector.DesiredDisplayModeSpecs mDisplayModeSpecs = + new DisplayModeDirector.DesiredDisplayModeSpecs(); private final boolean mIsDefaultDisplay; private final BacklightAdapter mBacklightAdapter; + private final SidekickInternal mSidekickInternal; private DisplayDeviceInfo mInfo; private boolean mHavePendingChanges; @@ -200,8 +203,6 @@ final class LocalDisplayAdapter extends DisplayAdapter { private int mActiveDisplayModeAtStartId = INVALID_MODE_ID; private Display.Mode mUserPreferredMode; private int mActiveModeId = INVALID_MODE_ID; - private DisplayModeDirector.DesiredDisplayModeSpecs mDisplayModeSpecs = - new DisplayModeDirector.DesiredDisplayModeSpecs(); private boolean mDisplayModeSpecsInvalid; private int mActiveColorMode; private Display.HdrCapabilities mHdrCapabilities; @@ -210,13 +211,11 @@ final class LocalDisplayAdapter extends DisplayAdapter { private boolean mAllmRequested; private boolean mGameContentTypeRequested; private boolean mSidekickActive; - private SidekickInternal mSidekickInternal; private SurfaceControl.StaticDisplayInfo mStaticDisplayInfo; // The supported display modes according to SurfaceFlinger private SurfaceControl.DisplayMode[] mSfDisplayModes; // The active display mode in SurfaceFlinger private SurfaceControl.DisplayMode mActiveSfDisplayMode; - private DisplayDeviceConfig mDisplayDeviceConfig; private DisplayEventReceiver.FrameRateOverride[] mFrameRateOverrides = new DisplayEventReceiver.FrameRateOverride[0]; @@ -233,7 +232,6 @@ final class LocalDisplayAdapter extends DisplayAdapter { mSidekickInternal = LocalServices.getService(SidekickInternal.class); mBacklightAdapter = new BacklightAdapter(displayToken, isDefaultDisplay, mSurfaceControlProxy); - mDisplayDeviceConfig = null; mActiveDisplayModeAtStartId = dynamicInfo.activeDisplayModeId; } @@ -459,9 +457,6 @@ final class LocalDisplayAdapter extends DisplayAdapter { final Context context = getOverlayContext(); mDisplayDeviceConfig = DisplayDeviceConfig.create(context, mPhysicalDisplayId, mIsDefaultDisplay); - if (mDisplayDeviceConfig == null) { - return; - } // Load brightness HWC quirk mBacklightAdapter.setForceSurfaceControl(mDisplayDeviceConfig.hasQuirk( @@ -1083,8 +1078,8 @@ final class LocalDisplayAdapter extends DisplayAdapter { pw.println("mGameContentTypeRequested=" + mGameContentTypeRequested); pw.println("mStaticDisplayInfo=" + mStaticDisplayInfo); pw.println("mSfDisplayModes="); - for (int i = 0; i < mSfDisplayModes.length; i++) { - pw.println(" " + mSfDisplayModes[i]); + for (SurfaceControl.DisplayMode sfDisplayMode : mSfDisplayModes) { + pw.println(" " + sfDisplayMode); } pw.println("mActiveSfDisplayMode=" + mActiveSfDisplayMode); pw.println("mSupportedModes="); @@ -1238,6 +1233,8 @@ final class LocalDisplayAdapter extends DisplayAdapter { } public static class Injector { + // Native callback. + @SuppressWarnings("unused") private ProxyDisplayEventReceiver mReceiver; public void setDisplayEventListenerLocked(Looper looper, DisplayEventListener listener) { mReceiver = new ProxyDisplayEventReceiver(looper, listener); From 2ecbf1eff2e2fca09d7cb47335ea5794ee9982f5 Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Sat, 5 Feb 2022 00:29:22 +0000 Subject: [PATCH 4/4] Fix display javadoc params. Bug: 217923092 Test: N/A Change-Id: I13cf865d151162328f082bc3f7037988736d4336 --- .../java/com/android/server/display/LogicalDisplayMapper.java | 1 - services/core/java/com/android/server/display/RampAnimator.java | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/services/core/java/com/android/server/display/LogicalDisplayMapper.java b/services/core/java/com/android/server/display/LogicalDisplayMapper.java index 93c73be5021e9..6f5729f89e0f8 100644 --- a/services/core/java/com/android/server/display/LogicalDisplayMapper.java +++ b/services/core/java/com/android/server/display/LogicalDisplayMapper.java @@ -825,7 +825,6 @@ class LogicalDisplayMapper implements DisplayDeviceRepository.Listener { * * @param device The device to associate with the LogicalDisplay. * @param displayId The display ID to give the new display. If invalid, a new ID is assigned. - * @param isDefault Indicates if we are creating the default display. * @return The new logical display if created, null otherwise. */ private LogicalDisplay createNewLogicalDisplayLocked(DisplayDevice device, int displayId) { diff --git a/services/core/java/com/android/server/display/RampAnimator.java b/services/core/java/com/android/server/display/RampAnimator.java index d8672fc076197..2567e43062937 100644 --- a/services/core/java/com/android/server/display/RampAnimator.java +++ b/services/core/java/com/android/server/display/RampAnimator.java @@ -55,7 +55,7 @@ class RampAnimator { * If this is the first time the property is being set or if the rate is 0, * the value jumps directly to the target. * - * @param target The target value. + * @param targetLinear The target value. * @param rate The convergence rate in units per second, or 0 to set the value immediately. * @return True if the target differs from the previous target. */