Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 7b61b78a authored by Santos Cordon's avatar Santos Cordon
Browse files

Reload RBC config when colorMode changes. 

RBC uses two matrixes depending on color mode: linear or gamma.
When color mode was changing for color correction, we weren't
updating the RBC matrix and this resulted in extra-dark visuals.

This CL updates RBC coefficients when colorMode changes.

Also had to add some changes to set the RBC matrix early
when colorMode does change to reduce the lag. Without this
lag was resulting in some jarring flicker when color
correct was toggled.

Bug: 336354594
Test: atest ColorDisplayServiceTest, DisplayTransformManagerTest
Flag: EXEMPT bugfix
Change-Id: I5e29325c46f6ee716c11acf006b817bbc4da88d4
parent 46f5297a

services/core/java/com/android/server/display/color/ColorDisplayService.java

+20 −5
Original line number Diff line number Diff line
@@ -168,8 +168,7 @@ public final class ColorDisplayService extends SystemService { 
            new NightDisplayTintController();

    private final TintController mGlobalSaturationTintController =

            new GlobalSaturationTintController();

    private final ReduceBrightColorsTintController mReduceBrightColorsTintController =

            new ReduceBrightColorsTintController();

    private final ReduceBrightColorsTintController mReduceBrightColorsTintController;



    @VisibleForTesting

    final Handler mHandler;
@@ -201,7 +200,13 @@ public final class ColorDisplayService extends SystemService { 
    private boolean mEvenDimmerActivated;



    public ColorDisplayService(Context context) {

        this(context, new ReduceBrightColorsTintController());

    }



    @VisibleForTesting

    public ColorDisplayService(Context context, ReduceBrightColorsTintController rbcController) {

        super(context);

        mReduceBrightColorsTintController = rbcController;

        mHandler = new TintHandler(DisplayThread.get().getLooper());

        mVisibleBackgroundUsersEnabled = isVisibleBackgroundUsersEnabled();

        mUserManager = UserManagerService.getInstance();
@@ -571,27 +576,37 @@ public final class ColorDisplayService extends SystemService { 
        return mColorModeCompositionColorSpaces.get(mode, Display.COLOR_MODE_INVALID);

    }



    private void onDisplayColorModeChanged(int mode) {

    @VisibleForTesting

    void onDisplayColorModeChanged(int mode) {

        if (mode == NOT_SET) {

            return;

        }



        mReduceBrightColorsTintController.cancelAnimator();

        mNightDisplayTintController.cancelAnimator();

        mDisplayWhiteBalanceTintController.cancelAnimator();



        if (mNightDisplayTintController.isAvailable(getContext())) {

        final DisplayTransformManager dtm = getLocalService(DisplayTransformManager.class);



        if (mNightDisplayTintController.isAvailable(getContext())) {

            mNightDisplayTintController.setUp(getContext(), dtm.needsLinearColorMatrix(mode));

            mNightDisplayTintController

                    .setMatrix(mNightDisplayTintController.getColorTemperatureSetting());

        }



        if (mReduceBrightColorsTintController.isAvailable(getContext())) {

            // Different color modes may require different coefficients to be loaded for RBC.

            // Re-set up RBC so that it can recalculate its transform matrix with new values.

            mReduceBrightColorsTintController.setUp(getContext(), dtm.needsLinearColorMatrix(mode));

            onReduceBrightColorsStrengthLevelChanged(); // Trigger matrix recalc + updates

        }



        // dtm.setColorMode() needs to be called before

        // updateDisplayWhiteBalanceStatus(), this is because the latter calls

        // DisplayTransformManager.needsLinearColorMatrix(), therefore it is dependent

        // on the state of DisplayTransformManager.

        final DisplayTransformManager dtm = getLocalService(DisplayTransformManager.class);

        dtm.setColorMode(mode, mNightDisplayTintController.getMatrix(),

                mReduceBrightColorsTintController.getMatrix(),

                getCompositionColorSpace(mode));



        if (mDisplayWhiteBalanceTintController.isAvailable(getContext())) {

services/core/java/com/android/server/display/color/DisplayTransformManager.java

+5 −1
Original line number Diff line number Diff line
@@ -265,7 +265,7 @@ public class DisplayTransformManager { 
    /**

     * Sets color mode and updates night display transform values.

     */

    public boolean setColorMode(int colorMode, float[] nightDisplayMatrix,

    public boolean setColorMode(int colorMode, float[] nightDisplayMatrix, float[] rbcMatrix,

            int compositionColorMode) {

        if (colorMode == ColorDisplayManager.COLOR_MODE_NATURAL) {

            applySaturation(COLOR_SATURATION_NATURAL);
@@ -285,7 +285,11 @@ public class DisplayTransformManager { 
            setDisplayColor(colorMode, compositionColorMode);

        }



        // These are close to the setDisplayColor() call to reduce delay between

        // setting these matrixes and updating the color mode. Without this proximity

        // of calls, updates to color mode can result in flicker.

        setColorMatrix(LEVEL_COLOR_MATRIX_NIGHT_DISPLAY, nightDisplayMatrix);

        setColorMatrix(LEVEL_COLOR_MATRIX_REDUCE_BRIGHT_COLORS, rbcMatrix);



        updateConfiguration();

services/tests/displayservicetests/src/com/android/server/display/color/ColorDisplayServiceTest.java

+32 −6
Original line number Diff line number Diff line
@@ -20,10 +20,13 @@ import static com.google.common.truth.Truth.assertThat; 
import static com.google.common.truth.Truth.assertWithMessage;



import static org.mockito.ArgumentMatchers.any;

import static org.mockito.ArgumentMatchers.anyBoolean;

import static org.mockito.ArgumentMatchers.anyInt;

import static org.mockito.ArgumentMatchers.eq;

import static org.mockito.Mockito.doReturn;

import static org.mockito.Mockito.never;

import static org.mockito.Mockito.reset;

import static org.mockito.Mockito.times;

import static org.mockito.Mockito.verify;

import static org.mockito.Mockito.when;
@@ -89,6 +92,7 @@ public class ColorDisplayServiceTest { 
    private ColorDisplayService.BinderService mBinderService;



    private Resources mResourcesSpy;

    private ReduceBrightColorsTintController mRbcSpy;



    private static final int[] MINIMAL_COLOR_MODES = new int[] {

        ColorDisplayManager.COLOR_MODE_NATURAL,
@@ -135,7 +139,8 @@ public class ColorDisplayServiceTest { 
        mLocalServiceKeeperRule.overrideLocalService(

                DisplayManagerInternal.class, mDisplayManagerInternal);



        mCds = new ColorDisplayService(mContext);

        mRbcSpy = Mockito.spy(new ReduceBrightColorsTintController());

        mCds = new ColorDisplayService(mContext, mRbcSpy);

        mBinderService = mCds.new BinderService();

        mLocalServiceKeeperRule.overrideLocalService(

                ColorDisplayService.ColorDisplayServiceInternal.class,
@@ -1106,7 +1111,8 @@ public class ColorDisplayServiceTest { 
        setColorMode(ColorDisplayManager.COLOR_MODE_NATURAL);

        startService();

        verify(mDisplayTransformManager).setColorMode(

                eq(ColorDisplayManager.COLOR_MODE_NATURAL), any(), eq(Display.COLOR_MODE_INVALID));

                eq(ColorDisplayManager.COLOR_MODE_NATURAL), any(), any(),

                eq(Display.COLOR_MODE_INVALID));

    }



    @Test
@@ -1124,7 +1130,8 @@ public class ColorDisplayServiceTest { 
        setColorMode(ColorDisplayManager.COLOR_MODE_NATURAL);

        startService();

        verify(mDisplayTransformManager).setColorMode(

                eq(ColorDisplayManager.COLOR_MODE_NATURAL), any(), eq(Display.COLOR_MODE_INVALID));

                eq(ColorDisplayManager.COLOR_MODE_NATURAL), any(), any(),

                eq(Display.COLOR_MODE_INVALID));

    }



    @Test
@@ -1140,7 +1147,8 @@ public class ColorDisplayServiceTest { 
        setColorMode(ColorDisplayManager.COLOR_MODE_NATURAL);

        startService();

        verify(mDisplayTransformManager).setColorMode(

                eq(ColorDisplayManager.COLOR_MODE_NATURAL), any(), eq(Display.COLOR_MODE_SRGB));

                eq(ColorDisplayManager.COLOR_MODE_NATURAL), any(), any(),

                eq(Display.COLOR_MODE_SRGB));

    }



    @Test
@@ -1156,7 +1164,8 @@ public class ColorDisplayServiceTest { 
        setColorMode(ColorDisplayManager.COLOR_MODE_BOOSTED);

        startService();

        verify(mDisplayTransformManager).setColorMode(

                eq(ColorDisplayManager.COLOR_MODE_BOOSTED), any(), eq(Display.COLOR_MODE_INVALID));

                eq(ColorDisplayManager.COLOR_MODE_BOOSTED), any(), any(),

                eq(Display.COLOR_MODE_INVALID));

    }



    @Test
@@ -1164,10 +1173,27 @@ public class ColorDisplayServiceTest { 
        when(mResourcesSpy.getIntArray(R.array.config_availableColorModes))

                    .thenReturn(new int[] {});

        startService();

        verify(mDisplayTransformManager, never()).setColorMode(anyInt(), any(), anyInt());

        verify(mDisplayTransformManager, never()).setColorMode(anyInt(), any(), any(), anyInt());

        assertThat(mBinderService.getColorMode()).isEqualTo(-1);

    }



    @Test

    public void ensureColorModeChangeTriggersRbcReload() {

        // should set up RBC once at startup

        startService();

        reset(mRbcSpy);



        // Make sure RBC is enabled and available for this test

        doReturn(true).when(mRbcSpy).isAvailable(mContext);



        // When Color Mode changes, RBC needs to re-setup

        // onDisplayColorModeChanged cancels animations, and should be called in handler thread

        mCds.mHandler.runWithScissors(

                () -> mCds.onDisplayColorModeChanged(ColorDisplayManager.COLOR_MODE_NATURAL),

                1000);

        verify(mRbcSpy, times(1)).setUp(eq(mContext), anyBoolean());

    }



    /**

     * Configures Night display to use a custom schedule.

     *

services/tests/displayservicetests/src/com/android/server/display/color/DisplayTransformManagerTest.java

+15 −8
Original line number Diff line number Diff line
@@ -19,6 +19,7 @@ package com.android.server.display.color; 
import static com.android.dx.mockito.inline.extended.ExtendedMockito.anyString;

import static com.android.dx.mockito.inline.extended.ExtendedMockito.doAnswer;

import static com.android.server.display.color.DisplayTransformManager.LEVEL_COLOR_MATRIX_NIGHT_DISPLAY;

import static com.android.server.display.color.DisplayTransformManager.LEVEL_COLOR_MATRIX_REDUCE_BRIGHT_COLORS;

import static com.android.server.display.color.DisplayTransformManager.PERSISTENT_PROPERTY_COMPOSITION_COLOR_MODE;

import static com.android.server.display.color.DisplayTransformManager.PERSISTENT_PROPERTY_DISPLAY_COLOR;

import static com.android.server.display.color.DisplayTransformManager.PERSISTENT_PROPERTY_SATURATION;
@@ -51,12 +52,14 @@ public class DisplayTransformManagerTest { 
    private MockitoSession mSession;

    private DisplayTransformManager mDtm;

    private float[] mNightDisplayMatrix;

    private float[] mRbcMatrix;

    private HashMap<String, String> mSystemProperties;



    @Before

    public void setUp() {

        mDtm = new DisplayTransformManager();

        mNightDisplayMatrix = mDtm.getColorMatrix(LEVEL_COLOR_MATRIX_NIGHT_DISPLAY);

        mRbcMatrix = mDtm.getColorMatrix(LEVEL_COLOR_MATRIX_REDUCE_BRIGHT_COLORS);



        mSession = ExtendedMockito.mockitoSession()

                .initMocks(this)
@@ -81,7 +84,8 @@ public class DisplayTransformManagerTest { 


    @Test

    public void setColorMode_natural() {

        mDtm.setColorMode(ColorDisplayManager.COLOR_MODE_NATURAL, mNightDisplayMatrix, -1);

        mDtm.setColorMode(ColorDisplayManager.COLOR_MODE_NATURAL, mNightDisplayMatrix, mRbcMatrix,

                Display.COLOR_MODE_INVALID);

        assertThat(mSystemProperties.get(PERSISTENT_PROPERTY_DISPLAY_COLOR))

                .isEqualTo("0" /* managed */);

        assertThat(mSystemProperties.get(PERSISTENT_PROPERTY_SATURATION))
@@ -90,7 +94,8 @@ public class DisplayTransformManagerTest { 


    @Test

    public void setColorMode_boosted() {

        mDtm.setColorMode(ColorDisplayManager.COLOR_MODE_BOOSTED, mNightDisplayMatrix, -1);

        mDtm.setColorMode(ColorDisplayManager.COLOR_MODE_BOOSTED, mNightDisplayMatrix, mRbcMatrix,

                Display.COLOR_MODE_INVALID);



        assertThat(mSystemProperties.get(PERSISTENT_PROPERTY_DISPLAY_COLOR))

                .isEqualTo("0" /* managed */);
@@ -100,7 +105,8 @@ public class DisplayTransformManagerTest { 


    @Test

    public void setColorMode_saturated() {

        mDtm.setColorMode(ColorDisplayManager.COLOR_MODE_SATURATED, mNightDisplayMatrix, -1);

        mDtm.setColorMode(ColorDisplayManager.COLOR_MODE_SATURATED, mNightDisplayMatrix, mRbcMatrix,

                Display.COLOR_MODE_INVALID);

        assertThat(mSystemProperties.get(PERSISTENT_PROPERTY_DISPLAY_COLOR))

                .isEqualTo("1" /* unmanaged */);

        assertThat(mSystemProperties.get(PERSISTENT_PROPERTY_SATURATION))
@@ -109,7 +115,8 @@ public class DisplayTransformManagerTest { 


    @Test

    public void setColorMode_automatic() {

        mDtm.setColorMode(ColorDisplayManager.COLOR_MODE_AUTOMATIC, mNightDisplayMatrix, -1);

        mDtm.setColorMode(ColorDisplayManager.COLOR_MODE_AUTOMATIC, mNightDisplayMatrix, mRbcMatrix,

                Display.COLOR_MODE_INVALID);

        assertThat(mSystemProperties.get(PERSISTENT_PROPERTY_DISPLAY_COLOR))

                .isEqualTo("2" /* enhanced */);

        assertThat(mSystemProperties.get(PERSISTENT_PROPERTY_SATURATION))
@@ -118,7 +125,7 @@ public class DisplayTransformManagerTest { 


    @Test

    public void setColorMode_vendor() {

        mDtm.setColorMode(0x100, mNightDisplayMatrix, -1);

        mDtm.setColorMode(0x100, mNightDisplayMatrix, mRbcMatrix, Display.COLOR_MODE_INVALID);

        assertThat(mSystemProperties.get(PERSISTENT_PROPERTY_DISPLAY_COLOR))

                .isEqualTo(Integer.toString(0x100) /* pass-through */);

        assertThat(mSystemProperties.get(PERSISTENT_PROPERTY_SATURATION))
@@ -127,7 +134,7 @@ public class DisplayTransformManagerTest { 


    @Test

    public void setColorMode_outOfBounds() {

        mDtm.setColorMode(0x50, mNightDisplayMatrix, -1);

        mDtm.setColorMode(0x50, mNightDisplayMatrix, mRbcMatrix, Display.COLOR_MODE_INVALID);

        assertThat(mSystemProperties.get(PERSISTENT_PROPERTY_DISPLAY_COLOR))

                .isEqualTo(null);

        assertThat(mSystemProperties.get(PERSISTENT_PROPERTY_SATURATION))
@@ -141,7 +148,7 @@ public class DisplayTransformManagerTest { 
        // Start with a known state, which we expect to remain unmodified

        SystemProperties.set(PERSISTENT_PROPERTY_COMPOSITION_COLOR_MODE, magicPropertyValue);



        mDtm.setColorMode(ColorDisplayManager.COLOR_MODE_NATURAL, mNightDisplayMatrix,

        mDtm.setColorMode(ColorDisplayManager.COLOR_MODE_NATURAL, mNightDisplayMatrix, mRbcMatrix,

                Display.COLOR_MODE_INVALID);

        assertThat(mSystemProperties.get(PERSISTENT_PROPERTY_COMPOSITION_COLOR_MODE))

                .isEqualTo(magicPropertyValue);
@@ -155,7 +162,7 @@ public class DisplayTransformManagerTest { 
        // Start with a known state, which we expect to get modified

        SystemProperties.set(PERSISTENT_PROPERTY_COMPOSITION_COLOR_MODE, magicPropertyValue);



        mDtm.setColorMode(ColorDisplayManager.COLOR_MODE_NATURAL, mNightDisplayMatrix,

        mDtm.setColorMode(ColorDisplayManager.COLOR_MODE_NATURAL, mNightDisplayMatrix, mRbcMatrix,

                testPropertyValue);

        assertThat(mSystemProperties.get(PERSISTENT_PROPERTY_COMPOSITION_COLOR_MODE))

                .isEqualTo(Integer.toString(testPropertyValue));