Optimized HdrBrightnessModifier behaviour on display change

1) Otimized LightSensorSubscription on display change
- subscribe to new sensor and then unsubscribe from old to avoid SensorEventQueue recreation
2) onDiplayChange is already called in DisplayControllerHandler, no need to call in in handler
3) When new HdrBrightnessModifier is active, HdrClamper could be disabled
4) Fixed LightSensorControllerTest

Bug: b/353474430
Test: manual testing
Flag: com.android.server.display.feature.flags.new_hdr_brightness_modifier
Change-Id: I744ea06413544b553e25aff3d5ede2e995d46d9d

4 files changed, 49 insertions, 39 deletions

diff --git a/services/core/java/com/android/server/display/BrightnessRangeController.java b/services/core/java/com/android/server/display/BrightnessRangeController.java
index 515e70495f9e..8a3e39257145 100644
--- a/services/core/java/com/android/server/display/BrightnessRangeController.java
+++ b/services/core/java/com/android/server/display/BrightnessRangeController.java
@@ -60,7 +60,7 @@ class BrightnessRangeController {
         mModeChangeCallback = modeChangeCallback;
         mHdrClamper = hdrClamper;
         mNormalBrightnessModeController = normalBrightnessModeController;
-        mUseHdrClamper = flags.isHdrClamperEnabled();
+        mUseHdrClamper = flags.isHdrClamperEnabled() && !flags.useNewHdrBrightnessModifier();
         mUseNbmController = flags.isNbmControllerEnabled();
         if (mUseNbmController) {
             mNormalBrightnessModeController.resetNbmData(
diff --git a/services/core/java/com/android/server/display/brightness/clamper/HdrBrightnessModifier.java b/services/core/java/com/android/server/display/brightness/clamper/HdrBrightnessModifier.java
index 5e44cc357b28..ae1801ccea74 100644
--- a/services/core/java/com/android/server/display/brightness/clamper/HdrBrightnessModifier.java
+++ b/services/core/java/com/android/server/display/brightness/clamper/HdrBrightnessModifier.java
@@ -31,6 +31,7 @@ import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.display.BrightnessSynchronizer;
 import com.android.server.display.DisplayBrightnessState;
 import com.android.server.display.DisplayDeviceConfig;
+import com.android.server.display.brightness.BrightnessReason;
 import com.android.server.display.config.HdrBrightnessData;
 
 import java.io.PrintWriter;
@@ -99,7 +100,7 @@ public class HdrBrightnessModifier implements BrightnessStateModifier,
             mMaxBrightness = mPendingMaxBrightness;
             mClamperChangeListener.onChanged();
         };
-        onDisplayChanged(displayData);
+        mHandler.post(() -> onDisplayChanged(displayData));
     }
 
     // Called in DisplayControllerHandler
@@ -120,6 +121,8 @@ public class HdrBrightnessModifier implements BrightnessStateModifier,
 
         stateBuilder.setHdrBrightness(hdrBrightness);
         stateBuilder.setCustomAnimationRate(mTransitionRate);
+        stateBuilder.getBrightnessReason().addModifier(BrightnessReason.MODIFIER_HDR);
+
         // transition rate applied, reset
         mTransitionRate = CUSTOM_ANIMATION_RATE_NOT_SET;
     }
@@ -168,10 +171,18 @@ public class HdrBrightnessModifier implements BrightnessStateModifier,
         }
     }
 
+    // Called in DisplayControllerHandler
     @Override
     public void onDisplayChanged(BrightnessClamperController.DisplayDeviceData displayData) {
-        mHandler.post(() -> onDisplayChanged(displayData.mDisplayToken, displayData.mWidth,
-                displayData.mHeight, displayData.mDisplayDeviceConfig));
+        mDisplayDeviceConfig = displayData.mDisplayDeviceConfig;
+        mScreenSize = (float) displayData.mWidth * displayData.mHeight;
+        HdrBrightnessData data = mDisplayDeviceConfig.getHdrBrightnessData();
+        if (data == null) {
+            unregisterHdrListener();
+        } else {
+            registerHdrListener(displayData.mDisplayToken);
+        }
+        recalculate(data, mMaxDesiredHdrRatio);
     }
 
     // Called in DisplayControllerHandler, when any modifier state changes
@@ -215,20 +226,6 @@ public class HdrBrightnessModifier implements BrightnessStateModifier,
     }
 
     // Called in DisplayControllerHandler
-    private void onDisplayChanged(IBinder displayToken, int width, int height,
-            DisplayDeviceConfig config) {
-        mDisplayDeviceConfig = config;
-        mScreenSize = (float) width * height;
-        HdrBrightnessData data = config.getHdrBrightnessData();
-        if (data == null) {
-            unregisterHdrListener();
-        } else {
-            registerHdrListener(displayToken);
-        }
-        recalculate(data, mMaxDesiredHdrRatio);
-    }
-
-    // Called in DisplayControllerHandler
     private void recalculate(@Nullable HdrBrightnessData data, float maxDesiredHdrRatio) {
         Mode newMode = recalculateMode(data);
         // if HDR mode changed, notify changed
@@ -258,6 +255,10 @@ public class HdrBrightnessModifier implements BrightnessStateModifier,
         if (data == null) {
             return Mode.NO_HDR;
         }
+        // no HDR layer present
+        if (mHdrLayerSize == DEFAULT_HDR_LAYER_SIZE) {
+            return Mode.NO_HDR;
+        }
         // HDR layer < minHdr % for Nbm
         if (mHdrLayerSize < mScreenSize * data.minimumHdrPercentOfScreenForNbm) {
             return Mode.NO_HDR;
diff --git a/services/core/java/com/android/server/display/brightness/clamper/LightSensorController.java b/services/core/java/com/android/server/display/brightness/clamper/LightSensorController.java
index d89dd28c4a89..b219cb1bc15c 100644
--- a/services/core/java/com/android/server/display/brightness/clamper/LightSensorController.java
+++ b/services/core/java/com/android/server/display/brightness/clamper/LightSensorController.java
@@ -62,6 +62,9 @@ public class LightSensorController {
     private final SensorEventListener mLightSensorEventListener = new SensorEventListener() {
         @Override
         public void onSensorChanged(SensorEvent event) {
+            if (event.sensor != mRegisteredLightSensor) {
+                return;
+            }
             long now = mInjector.getTime();
             mAmbientFilter.addValue(TimeUnit.NANOSECONDS.toMillis(event.timestamp),
                     event.values[0]);
@@ -95,15 +98,13 @@ public class LightSensorController {
         if (mRegisteredLightSensor == mLightSensor) {
             return;
         }
+        if (mLightSensor != null) {
+            mSensorManager.registerListener(mLightSensorEventListener,
+                    mLightSensor, mLightSensorRate * 1000, mHandler);
+        }
         if (mRegisteredLightSensor != null) {
             stop();
         }
-        if (mLightSensor == null) {
-            return;
-        }
-
-        mSensorManager.registerListener(mLightSensorEventListener,
-                mLightSensor, mLightSensorRate * 1000, mHandler);
         mRegisteredLightSensor = mLightSensor;
 
         if (DEBUG) {
@@ -115,7 +116,7 @@ public class LightSensorController {
         if (mRegisteredLightSensor == null) {
             return;
         }
-        mSensorManager.unregisterListener(mLightSensorEventListener);
+        mSensorManager.unregisterListener(mLightSensorEventListener, mRegisteredLightSensor);
         mRegisteredLightSensor = null;
         mAmbientFilter.clear();
         mLightSensorListener.onAmbientLuxChange(INVALID_LUX);
diff --git a/services/tests/displayservicetests/src/com/android/server/display/brightness/clamper/LightSensorControllerTest.kt b/services/tests/displayservicetests/src/com/android/server/display/brightness/clamper/LightSensorControllerTest.kt
index f59e1275d2ce..79b99b5294d7 100644
--- a/services/tests/displayservicetests/src/com/android/server/display/brightness/clamper/LightSensorControllerTest.kt
+++ b/services/tests/displayservicetests/src/com/android/server/display/brightness/clamper/LightSensorControllerTest.kt
@@ -29,6 +29,7 @@ import com.android.server.display.config.SensorData
 import com.android.server.display.config.createSensorData
 import com.android.server.display.utils.AmbientFilter
 import org.junit.Before
+import org.junit.Test
 import org.mockito.kotlin.any
 import org.mockito.kotlin.argumentCaptor
 import org.mockito.kotlin.eq
@@ -62,31 +63,35 @@ class LightSensorControllerTest {
             mockLightSensorListener, mockHandler, testInjector)
     }
 
-    fun `does not register light sensor if is not configured`() {
+    @Test
+    fun doesNotRegisterLightSensorIfNotConfigured() {
         controller.restart()
 
         verifyNoMoreInteractions(mockSensorManager, mockAmbientFilter, mockLightSensorListener)
     }
 
-    fun `does not register light sensor if missing`() {
+    @Test
+    fun doesNotRegisterLightSensorIfMissing() {
         controller.configure(dummySensorData, DISPLAY_ID)
         controller.restart()
 
         verifyNoMoreInteractions(mockSensorManager, mockAmbientFilter, mockLightSensorListener)
     }
 
-    fun `register light sensor if configured and present`() {
+    @Test
+    fun registerLightSensorIfConfiguredAndPresent() {
         testInjector.lightSensor = TestUtils
                 .createSensor(Sensor.TYPE_LIGHT, Sensor.STRING_TYPE_LIGHT)
         controller.configure(dummySensorData, DISPLAY_ID)
         controller.restart()
 
         verify(mockSensorManager).registerListener(any(),
-            testInjector.lightSensor, LIGHT_SENSOR_RATE * 1000, mockHandler)
+            eq(testInjector.lightSensor), eq(LIGHT_SENSOR_RATE * 1000), eq(mockHandler))
         verifyNoMoreInteractions(mockSensorManager, mockAmbientFilter, mockLightSensorListener)
     }
 
-    fun `register light sensor once if not changed`() {
+    @Test
+    fun registerLightSensorOnceIfNotChanged() {
         testInjector.lightSensor = TestUtils
                 .createSensor(Sensor.TYPE_LIGHT, Sensor.STRING_TYPE_LIGHT)
         controller.configure(dummySensorData, DISPLAY_ID)
@@ -95,11 +100,12 @@ class LightSensorControllerTest {
         controller.restart()
 
         verify(mockSensorManager).registerListener(any(),
-            testInjector.lightSensor, LIGHT_SENSOR_RATE * 1000, mockHandler)
+            eq(testInjector.lightSensor), eq(LIGHT_SENSOR_RATE * 1000), eq(mockHandler))
         verifyNoMoreInteractions(mockSensorManager, mockAmbientFilter, mockLightSensorListener)
     }
 
-    fun `register new light sensor and unregister old if changed`() {
+    @Test
+    fun registerNewAndUnregisterOldLightSensorIfChanged() {
         val lightSensor1 = TestUtils
                 .createSensor(Sensor.TYPE_LIGHT, Sensor.STRING_TYPE_LIGHT)
         testInjector.lightSensor = lightSensor1
@@ -112,19 +118,21 @@ class LightSensorControllerTest {
         controller.configure(dummySensorData, DISPLAY_ID)
         controller.restart()
 
-        inOrder {
+        inOrder(mockSensorManager, mockAmbientFilter, mockLightSensorListener) {
             verify(mockSensorManager).registerListener(any(),
-                lightSensor1, LIGHT_SENSOR_RATE * 1000, mockHandler)
-            verify(mockSensorManager).unregisterListener(any<SensorEventListener>())
+                eq(lightSensor1), eq(LIGHT_SENSOR_RATE * 1000), eq(mockHandler))
+            verify(mockSensorManager).registerListener(any(),
+                eq(lightSensor2), eq(LIGHT_SENSOR_RATE * 1000), eq(mockHandler))
+            verify(mockSensorManager).unregisterListener(any<SensorEventListener>(),
+                eq(lightSensor1))
             verify(mockAmbientFilter).clear()
             verify(mockLightSensorListener).onAmbientLuxChange(LightSensorController.INVALID_LUX)
-            verify(mockSensorManager).registerListener(any(),
-                lightSensor2, LIGHT_SENSOR_RATE * 1000, mockHandler)
         }
         verifyNoMoreInteractions(mockSensorManager, mockAmbientFilter, mockLightSensorListener)
     }
 
-    fun `notifies listener on ambient lux change`() {
+    @Test
+    fun notifiesListenerOnAmbientLuxChange() {
         val expectedLux = 40f
         val eventLux = 50
         val eventTime = 60L
@@ -141,7 +149,7 @@ class LightSensorControllerTest {
         listener.onSensorChanged(TestUtils.createSensorEvent(testInjector.lightSensor,
             eventLux, eventTime * 1_000_000))
 
-        inOrder {
+        inOrder(mockAmbientFilter, mockLightSensorListener) {
             verify(mockAmbientFilter).addValue(eventTime, eventLux.toFloat())
             verify(mockLightSensorListener).onAmbientLuxChange(expectedLux)
         }