Merge "Move SurfaceControl Brightness calls into LocalDisplayAdapter"

3 files changed, 116 insertions, 69 deletions

diff --git a/services/core/java/com/android/server/display/LocalDisplayAdapter.java b/services/core/java/com/android/server/display/LocalDisplayAdapter.java
index 74ea2d7114fc..155b246a1408 100644
--- a/services/core/java/com/android/server/display/LocalDisplayAdapter.java
+++ b/services/core/java/com/android/server/display/LocalDisplayAdapter.java
@@ -177,10 +177,10 @@ final class LocalDisplayAdapter extends DisplayAdapter {
 
     private final class LocalDisplayDevice extends DisplayDevice {
         private final long mPhysicalDisplayId;
-        private final LogicalLight mBacklight;
         private final SparseArray<DisplayModeRecord> mSupportedModes = new SparseArray<>();
         private final ArrayList<Integer> mSupportedColorModes = new ArrayList<>();
         private final boolean mIsDefaultDisplay;
+        private final BacklightAdapter mBacklightAdapter;
 
         private DisplayDeviceInfo mInfo;
         private boolean mHavePendingChanges;
@@ -205,7 +205,6 @@ final class LocalDisplayAdapter extends DisplayAdapter {
         private SurfaceControl.DisplayConfig[] mDisplayConfigs;
         private Spline mSystemBrightnessToNits;
         private Spline mNitsToHalBrightness;
-
         private DisplayDeviceConfig mDisplayDeviceConfig;
 
         private DisplayEventReceiver.FrameRateOverride[] mFrameRateOverrides =
@@ -222,12 +221,7 @@ final class LocalDisplayAdapter extends DisplayAdapter {
             updateDisplayPropertiesLocked(info, configs, activeConfigId, configSpecs, colorModes,
                     activeColorMode, hdrCapabilities);
             mSidekickInternal = LocalServices.getService(SidekickInternal.class);
-            if (mIsDefaultDisplay) {
-                LightsManager lights = LocalServices.getService(LightsManager.class);
-                mBacklight = lights.getLight(LightsManager.LIGHT_ID_BACKLIGHT);
-            } else {
-                mBacklight = null;
-            }
+            mBacklightAdapter = new BacklightAdapter(displayToken, isDefaultDisplay);
             mAllmSupported = SurfaceControl.getAutoLowLatencyModeSupport(displayToken);
             mGameContentTypeSupported = SurfaceControl.getGameContentTypeSupport(displayToken);
             mDisplayDeviceConfig = null;
@@ -643,8 +637,7 @@ final class LocalDisplayAdapter extends DisplayAdapter {
                     brightnessState, PowerManager.BRIGHTNESS_OFF_FLOAT);
             final boolean stateChanged = (mState != state);
             final boolean brightnessChanged = (!BrightnessSynchronizer.floatEquals(
-                    mBrightnessState, brightnessState))
-                    && mBacklight != null;
+                    mBrightnessState, brightnessState));
             if (stateChanged || brightnessChanged) {
                 final long physicalDisplayId = mPhysicalDisplayId;
                 final IBinder token = getDisplayTokenLocked();
@@ -712,9 +705,7 @@ final class LocalDisplayAdapter extends DisplayAdapter {
                                     + "id=" + physicalDisplayId
                                     + ", state=" + Display.stateToString(state) + ")");
                         }
-                        if (mBacklight != null) {
-                            mBacklight.setVrMode(isVrEnabled);
-                        }
+                        mBacklightAdapter.setVrMode(isVrEnabled);
                     }
 
                     private void setDisplayState(int state) {
@@ -770,13 +761,8 @@ final class LocalDisplayAdapter extends DisplayAdapter {
                         Trace.traceBegin(Trace.TRACE_TAG_POWER, "setDisplayBrightness("
                                 + "id=" + physicalDisplayId + ", brightness=" + brightness + ")");
                         try {
-                            if (isHalBrightnessRangeSpecified()) {
-                                brightness = displayBrightnessToHalBrightness(
-                                        BrightnessSynchronizer.brightnessFloatToIntRange(brightness));
-                            }
-                            if (mBacklight != null) {
-                                mBacklight.setBrightness(brightness);
-                            }
+                            brightness = displayBrightnessToHalBrightness(brightness);
+                            mBacklightAdapter.setBrightness(brightness);
                             Trace.traceCounter(Trace.TRACE_TAG_POWER,
                                     "ScreenBrightness",
                                     BrightnessSynchronizer.brightnessFloatToInt(brightness));
@@ -785,26 +771,33 @@ final class LocalDisplayAdapter extends DisplayAdapter {
                         }
                     }
 
-                    private boolean isHalBrightnessRangeSpecified() {
-                        return !(mSystemBrightnessToNits == null || mNitsToHalBrightness == null);
-                    }
-
                     /**
                      * Converts brightness range from the framework's brightness space to the
                      * Hal brightness space if the HAL brightness space has been provided via
                      * a display device configuration file.
                      */
                     private float displayBrightnessToHalBrightness(float brightness) {
-                        if (!isHalBrightnessRangeSpecified()) {
-                            return PowerManager.BRIGHTNESS_INVALID_FLOAT;
+                        // TODO: b/171380847 - This needs to be deprecated. The nits-to-brightness
+                        // relationship should be specified in display-config OR config.xml, but not
+                        // both, and no nits-space conversion should be necessary.
+                        //
+                        // Only do a conversion if there exists a unique system brightness and a
+                        // unique HAL brightness-to-nits range defined.
+                        if (mSystemBrightnessToNits == null || mNitsToHalBrightness == null) {
+                            return brightness;
                         }
 
+                        // Sys brightness in this conversion is always specified in the old 1-255
+                        // range, so convert that here before the translation.
+                        final float brightnessInt =
+                                BrightnessSynchronizer.brightnessFloatToIntRange(brightness);
+
                         if (BrightnessSynchronizer.floatEquals(
-                                brightness, PowerManager.BRIGHTNESS_OFF)) {
+                                brightnessInt, PowerManager.BRIGHTNESS_OFF)) {
                             return PowerManager.BRIGHTNESS_OFF_FLOAT;
                         }
 
-                        final float nits = mSystemBrightnessToNits.interpolate(brightness);
+                        final float nits = mSystemBrightnessToNits.interpolate(brightnessInt);
                         final float halBrightness = mNitsToHalBrightness.interpolate(nits);
                         return halBrightness;
                     }
@@ -988,7 +981,7 @@ final class LocalDisplayAdapter extends DisplayAdapter {
             pw.println("mDefaultModeId=" + mDefaultModeId);
             pw.println("mState=" + Display.stateToString(mState));
             pw.println("mBrightnessState=" + mBrightnessState);
-            pw.println("mBacklight=" + mBacklight);
+            pw.println("mBacklightAdapter=" + mBacklightAdapter);
             pw.println("mAllmSupported=" + mAllmSupported);
             pw.println("mAllmRequested=" + mAllmRequested);
             pw.println("mGameContentTypeSupported=" + mGameContentTypeSupported);
@@ -1209,4 +1202,44 @@ final class LocalDisplayAdapter extends DisplayAdapter {
             }
         }
     }
+
+    static class BacklightAdapter {
+        private final IBinder mDisplayToken;
+        private final LogicalLight mBacklight;
+        private final boolean mUseSurfaceControlBrightness;
+
+        BacklightAdapter(IBinder displayToken, boolean isDefaultDisplay) {
+            mDisplayToken = displayToken;
+
+            mUseSurfaceControlBrightness =
+                    SurfaceControl.getDisplayBrightnessSupport(mDisplayToken);
+
+            if (!mUseSurfaceControlBrightness && isDefaultDisplay) {
+                LightsManager lights = LocalServices.getService(LightsManager.class);
+                mBacklight = lights.getLight(LightsManager.LIGHT_ID_BACKLIGHT);
+            } else {
+                mBacklight = null;
+            }
+        }
+
+        public void setBrightness(float brightness) {
+            if (mUseSurfaceControlBrightness) {
+                SurfaceControl.setDisplayBrightness(mDisplayToken, brightness);
+            } else if (mBacklight != null) {
+                mBacklight.setBrightness(brightness);
+            }
+        }
+
+        public void setVrMode(boolean isVrModeEnabled) {
+            if (mBacklight != null) {
+                mBacklight.setVrMode(isVrModeEnabled);
+            }
+        }
+
+        @Override
+        public String toString() {
+            return "BacklightAdapter [useSurfaceControl=" + mUseSurfaceControlBrightness
+                    + ", backlight=" + mBacklight + "]";
+        }
+    }
 }
diff --git a/services/core/java/com/android/server/lights/LightsService.java b/services/core/java/com/android/server/lights/LightsService.java
index d624019bfec8..43c965dde27b 100644
--- a/services/core/java/com/android/server/lights/LightsService.java
+++ b/services/core/java/com/android/server/lights/LightsService.java
@@ -29,14 +29,12 @@ import android.os.Binder;
 import android.os.Handler;
 import android.os.IBinder;
 import android.os.Looper;
-import android.os.PowerManager;
 import android.os.RemoteException;
 import android.os.ServiceManager;
 import android.os.Trace;
 import android.provider.Settings;
 import android.util.Slog;
 import android.util.SparseArray;
-import android.view.SurfaceControl;
 
 import com.android.internal.BrightnessSynchronizer;
 import com.android.internal.annotations.GuardedBy;
@@ -262,25 +260,9 @@ public class LightsService extends SystemService {
     }
 
     private final class LightImpl extends LogicalLight {
-        private final IBinder mDisplayToken;
-        private final int mSurfaceControlMaximumBrightness;
 
         private LightImpl(Context context, HwLight hwLight) {
             mHwLight = hwLight;
-            mDisplayToken = SurfaceControl.getInternalDisplayToken();
-            final boolean brightnessSupport = SurfaceControl.getDisplayBrightnessSupport(
-                    mDisplayToken);
-            if (DEBUG) {
-                Slog.d(TAG, "Display brightness support: " + brightnessSupport);
-            }
-            int maximumBrightness = 0;
-            if (brightnessSupport) {
-                PowerManager pm = context.getSystemService(PowerManager.class);
-                if (pm != null) {
-                    maximumBrightness = pm.getMaximumScreenBrightnessSetting();
-                }
-            }
-            mSurfaceControlMaximumBrightness = maximumBrightness;
         }
 
         @Override
@@ -301,28 +283,10 @@ public class LightsService extends SystemService {
                             + ": brightness=" + brightness);
                     return;
                 }
-                // Ideally, we'd like to set the brightness mode through the SF/HWC as well, but
-                // right now we just fall back to the old path through Lights brightessMode is
-                // anything but USER or the device shouldBeInLowPersistenceMode().
-                if (brightnessMode == BRIGHTNESS_MODE_USER && !shouldBeInLowPersistenceMode()
-                        && mSurfaceControlMaximumBrightness == 255) {
-                    // New system
-                    // TODO: the last check should be mSurfaceControlMaximumBrightness != 0; the
-                    // reason we enforce 255 right now is to stay consistent with the old path. In
-                    // the future, the framework should be refactored so that brightness is a float
-                    // between 0.0f and 1.0f, and the actual number of supported brightness levels
-                    // is determined in the device-specific implementation.
-                    if (DEBUG) {
-                        Slog.d(TAG, "Using new setBrightness path!");
-                    }
-                    SurfaceControl.setDisplayBrightness(mDisplayToken, brightness);
-                } else {
-                    // Old system
-                    int brightnessInt = BrightnessSynchronizer.brightnessFloatToInt(brightness);
-                    int color = brightnessInt & 0x000000ff;
-                    color = 0xff000000 | (color << 16) | (color << 8) | color;
-                    setLightLocked(color, LIGHT_FLASH_NONE, 0, 0, brightnessMode);
-                }
+                int brightnessInt = BrightnessSynchronizer.brightnessFloatToInt(brightness);
+                int color = brightnessInt & 0x000000ff;
+                color = 0xff000000 | (color << 16) | (color << 8) | color;
+                setLightLocked(color, LIGHT_FLASH_NONE, 0, 0, brightnessMode);
             }
         }
 
diff --git a/services/tests/mockingservicestests/src/com/android/server/display/LocalDisplayAdapterTest.java b/services/tests/mockingservicestests/src/com/android/server/display/LocalDisplayAdapterTest.java
index dbdee979b6a4..af4130d92cfe 100644
--- a/services/tests/mockingservicestests/src/com/android/server/display/LocalDisplayAdapterTest.java
+++ b/services/tests/mockingservicestests/src/com/android/server/display/LocalDisplayAdapterTest.java
@@ -19,11 +19,14 @@ package com.android.server.display;
 import static com.android.dx.mockito.inline.extended.ExtendedMockito.doReturn;
 import static com.android.dx.mockito.inline.extended.ExtendedMockito.mockitoSession;
 import static com.android.dx.mockito.inline.extended.ExtendedMockito.spyOn;
+import static com.android.dx.mockito.inline.extended.ExtendedMockito.verify;
 
 import static com.google.common.truth.Truth.assertThat;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
+import static org.mockito.ArgumentMatchers.anyFloat;
+import static org.mockito.Mockito.never;
 
 import android.content.Context;
 import android.content.res.Resources;
@@ -40,7 +43,9 @@ import androidx.test.runner.AndroidJUnit4;
 
 import com.android.dx.mockito.inline.extended.StaticMockitoSession;
 import com.android.server.LocalServices;
+import com.android.server.display.LocalDisplayAdapter.BacklightAdapter;
 import com.android.server.lights.LightsManager;
+import com.android.server.lights.LogicalLight;
 
 import com.google.common.truth.Truth;
 
@@ -78,6 +83,8 @@ public class LocalDisplayAdapterTest {
     private Resources mMockedResources;
     @Mock
     private LightsManager mMockedLightsManager;
+    @Mock
+    private LogicalLight mMockedBacklight;
 
     private Handler mHandler;
 
@@ -492,6 +499,49 @@ public class LocalDisplayAdapterTest {
         waitForHandlerToComplete(mHandler, HANDLER_WAIT_MS);
     }
 
+    @Test
+    public void testBacklightAdapter_withSurfaceControlSupport() {
+        final Binder displayToken = new Binder();
+        doReturn(true).when(() -> SurfaceControl.getDisplayBrightnessSupport(displayToken));
+
+        // Test as default display
+        BacklightAdapter ba = new BacklightAdapter(displayToken, true /*isDefault*/);
+        ba.setBrightness(0.514f);
+        verify(() -> SurfaceControl.setDisplayBrightness(displayToken, 0.514f));
+
+        // Test as not default display
+        BacklightAdapter ba2 = new BacklightAdapter(displayToken,
+                false /*isDefault*/);
+        ba2.setBrightness(0.323f);
+        verify(() -> SurfaceControl.setDisplayBrightness(displayToken, 0.323f));
+    }
+
+    @Test
+    public void testBacklightAdapter_withoutSourceControlSupport_defaultDisplay() {
+        final Binder displayToken = new Binder();
+        doReturn(false).when(() -> SurfaceControl.getDisplayBrightnessSupport(displayToken));
+        doReturn(mMockedBacklight).when(mMockedLightsManager)
+                .getLight(LightsManager.LIGHT_ID_BACKLIGHT);
+
+        BacklightAdapter ba = new BacklightAdapter(displayToken, true /*isDefault*/);
+        ba.setBrightness(0.123f);
+        verify(mMockedBacklight).setBrightness(0.123f);
+    }
+
+    @Test
+    public void testBacklightAdapter_withoutSourceControlSupport_nonDefaultDisplay() {
+        final Binder displayToken = new Binder();
+        doReturn(false).when(() -> SurfaceControl.getDisplayBrightnessSupport(displayToken));
+        doReturn(mMockedBacklight).when(mMockedLightsManager)
+                .getLight(LightsManager.LIGHT_ID_BACKLIGHT);
+
+        BacklightAdapter ba = new BacklightAdapter(displayToken, false /*isDefault*/);
+        ba.setBrightness(0.456f);
+
+        // Adapter does not forward any brightness in this case.
+        verify(mMockedBacklight, never()).setBrightness(anyFloat());
+    }
+
     private void assertDisplayDpi(DisplayDeviceInfo info, int expectedPort,
                                   float expectedXdpi,
                                   float expectedYDpi,