From 83097c6db4a5349ceaa49ee3da0b0e56e549dfc5 Mon Sep 17 00:00:00 2001
From: Michael Wright <michaelwr@google.com>
Date: Sat, 23 Apr 2022 02:56:30 +0100
Subject: [PATCH] Invoke BrightnessSetting callbacks even with identical
 brightness.

Even if the brightness is the same we still need to invoke any listeners
as just the act of setting the brightness has side effects, like
clearing the temporary brightness. Skipping these callbacks mean we can
possibly get stuck in a state where we don't adjust the brightness at
all until the next time a user touches the brightness slider -- this
persists even through screen off.

Fixes: 218600709
Test: Manual
Change-Id: I918636a61424a51f9d22d32fd692beb6eb2b8f2d
---
 .../server/display/BrightnessSetting.java     | 12 +++++++-----
 .../server/display/DisplayManagerService.java |  2 +-
 .../display/DisplayPowerController.java       | 19 ++++++++++---------
 3 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/services/core/java/com/android/server/display/BrightnessSetting.java b/services/core/java/com/android/server/display/BrightnessSetting.java
index ca7b789da47d1..74486113dcf9b 100644
--- a/services/core/java/com/android/server/display/BrightnessSetting.java
+++ b/services/core/java/com/android/server/display/BrightnessSetting.java
@@ -102,13 +102,15 @@ public class BrightnessSetting {
             return;
         }
         synchronized (mSyncRoot) {
-            if (brightness == mBrightness) {
-                return;
+            // If the brightness is the same, we still need to update any listeners as the act of
+            // setting the brightness alone has side effects, like clearing any temporary
+            // brightness. We can skip persisting to disk, however, since it hasn't actually
+            // changed.
+            if (brightness != mBrightness) {
+                mPersistentDataStore.setBrightness(mLogicalDisplay.getPrimaryDisplayDeviceLocked(),
+                        brightness);
             }
-
             mBrightness = brightness;
-            mPersistentDataStore.setBrightness(mLogicalDisplay.getPrimaryDisplayDeviceLocked(),
-                    brightness);
             int toSend = Float.floatToIntBits(mBrightness);
             Message msg = mHandler.obtainMessage(MSG_BRIGHTNESS_CHANGED, toSend, 0);
             mHandler.sendMessage(msg);
diff --git a/services/core/java/com/android/server/display/DisplayManagerService.java b/services/core/java/com/android/server/display/DisplayManagerService.java
index ec7ccc4843c9f..6285ef1fdabd5 100644
--- a/services/core/java/com/android/server/display/DisplayManagerService.java
+++ b/services/core/java/com/android/server/display/DisplayManagerService.java
@@ -3349,7 +3349,7 @@ public final class DisplayManagerService extends SystemService {
                 synchronized (mSyncRoot) {
                     DisplayPowerController dpc = mDisplayPowerControllers.get(displayId);
                     if (dpc != null) {
-                        dpc.putScreenBrightnessSetting(brightness);
+                        dpc.setBrightness(brightness);
                     }
                     mPersistentDataStore.saveIfNeeded();
                 }
diff --git a/services/core/java/com/android/server/display/DisplayPowerController.java b/services/core/java/com/android/server/display/DisplayPowerController.java
index d13a9a3ec27ef..f39c412eed13a 100644
--- a/services/core/java/com/android/server/display/DisplayPowerController.java
+++ b/services/core/java/com/android/server/display/DisplayPowerController.java
@@ -1345,7 +1345,7 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call
                 if (mAppliedAutoBrightness && !autoBrightnessAdjustmentChanged) {
                     slowChange = true; // slowly adapt to auto-brightness
                 }
-                updateScreenBrightnessSetting = true;
+                updateScreenBrightnessSetting = mCurrentScreenBrightnessSetting != brightnessState;
                 mAppliedAutoBrightness = true;
                 mBrightnessReasonTemp.setReason(BrightnessReason.REASON_AUTOMATIC);
             } else {
@@ -1415,7 +1415,7 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call
             // before applying the low power or dim transformations so that the slider
             // accurately represents the full possible range, even if they range changes what
             // it means in absolute terms.
-            putScreenBrightnessSetting(brightnessState, /* updateCurrent */ true);
+            updateScreenBrightnessSetting(brightnessState);
         }
 
         // Apply dimming by at least some minimum amount when user activity
@@ -2288,17 +2288,18 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call
         return clampScreenBrightnessForVr(brightnessFloat);
     }
 
-    void putScreenBrightnessSetting(float brightnessValue) {
-        putScreenBrightnessSetting(brightnessValue, false);
+    void setBrightness(float brightnessValue) {
+        // Update the setting, which will eventually call back into DPC to have us actually update
+        // the display with the new value.
+        mBrightnessSetting.setBrightness(brightnessValue);
     }
 
-    private void putScreenBrightnessSetting(float brightnessValue, boolean updateCurrent) {
-        if (!isValidBrightnessValue(brightnessValue)) {
+    private void updateScreenBrightnessSetting(float brightnessValue) {
+        if (!isValidBrightnessValue(brightnessValue)
+                || brightnessValue == mCurrentScreenBrightnessSetting) {
             return;
         }
-        if (updateCurrent) {
-            setCurrentScreenBrightness(brightnessValue);
-        }
+        setCurrentScreenBrightness(brightnessValue);
         mBrightnessSetting.setBrightness(brightnessValue);
     }