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

Commit 3f2d389a authored by petsjonkin's avatar petsjonkin Committed by Android Build Coastguard Worker
Browse files

Properly persisting brightness set to max for multiscreen devices 

This also fixes regression with extra IO call when switching from high to low brightness screen on max brightness

Bug: b/330254981 b/345623960
Test: atest DisplayBrightnessControllerTest, manual testing
Flag: EXEMPT bugfix
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:68cbb9234a50701314f4c447297b8101e7b4a424)
Merged-In: I4632e74170bb92f84e416a9425e788edcfef84e5
Change-Id: I4632e74170bb92f84e416a9425e788edcfef84e5
parent c1353ff4

services/core/java/com/android/server/display/BrightnessSetting.java

+19 −0
Original line number Diff line number Diff line
@@ -130,6 +130,25 @@ public class BrightnessSetting { 
        }

    }



    /**

     * Sets the brightness. Does not send update event to listeners.

     * @param brightness The value to which the brightness is to be set.

     */

    public void setBrightnessNoNotify(float brightness) {

        if (Float.isNaN(brightness)) {

            Slog.w(TAG, "Attempting to init invalid brightness");

            return;

        }

        synchronized (mSyncRoot) {

            if (brightness != mBrightness) {

                mPersistentDataStore.setBrightness(mLogicalDisplay.getPrimaryDisplayDeviceLocked(),

                        brightness, mUserSerial

                );

            }

            mBrightness = brightness;

        }

    }



    /**

     * @return The brightness for the default display in nits. Used when the underlying display

     * device has changed but we want to persist the nit value.

services/core/java/com/android/server/display/DisplayPowerController.java

+13 −3
Original line number Diff line number Diff line
@@ -1565,7 +1565,9 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call 
            // even if they range changes what it means in absolute terms.

            mDisplayBrightnessController.updateScreenBrightnessSetting(

                    MathUtils.constrain(unthrottledBrightnessState,

                            clampedState.getMinBrightness(), clampedState.getMaxBrightness()));

                            clampedState.getMinBrightness(), clampedState.getMaxBrightness()),

                    Math.min(mBrightnessRangeController.getCurrentBrightnessMax(),

                            clampedState.getMaxBrightness()));

        }



        // The current brightness to use has been calculated at this point, and HbmController should
@@ -2455,12 +2457,20 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call 


    @Override

    public void setBrightness(float brightness) {

        mDisplayBrightnessController.setBrightness(clampScreenBrightness(brightness));

        // After HBMController and NBMController migration to Clampers framework

        // currentBrightnessMax should be taken from clampers controller

        // TODO(b/263362199)

        mDisplayBrightnessController.setBrightness(clampScreenBrightness(brightness),

                mBrightnessRangeController.getCurrentBrightnessMax());

    }



    @Override

    public void setBrightness(float brightness, int userSerial) {

        mDisplayBrightnessController.setBrightness(clampScreenBrightness(brightness), userSerial);

        // After HBMController and NBMController migration to Clampers framework

        // currentBrightnessMax should be taken from clampers controller

        // TODO(b/263362199)

        mDisplayBrightnessController.setBrightness(clampScreenBrightness(brightness), userSerial,

                mBrightnessRangeController.getCurrentBrightnessMax());

    }



    @Override

services/core/java/com/android/server/display/brightness/DisplayBrightnessController.java

+12 −7
Original line number Diff line number Diff line
@@ -313,13 +313,18 @@ public final class DisplayBrightnessController { 
    /**

     * Notifies the brightnessSetting to persist the supplied brightness value.

     */

    public void setBrightness(float brightnessValue) {

    public void setBrightness(float brightnessValue, float maxBrightness) {

        // Update the setting, which will eventually call back into DPC to have us actually

        // update the display with the new value.

        mBrightnessSetting.setBrightness(brightnessValue);

        if (mDisplayId == Display.DEFAULT_DISPLAY && mPersistBrightnessNitsForDefaultDisplay) {

            float nits = convertToNits(brightnessValue);

            if (nits >= 0) {

            float currentlyStoredNits = mBrightnessSetting.getBrightnessNitsForDefaultDisplay();

            // Don't override settings if the brightness is set to max, but the currently

            // stored value is greater. On multi-screen device, when switching between a

            // screen with a wider brightness range and one with a narrower brightness range,

            // the stored value shouldn't change.

            if (nits >= 0 && !(brightnessValue == maxBrightness && currentlyStoredNits > nits)) {

                mBrightnessSetting.setBrightnessNitsForDefaultDisplay(nits);

            }

        }
@@ -328,15 +333,15 @@ public final class DisplayBrightnessController { 
    /**

     * Notifies the brightnessSetting to persist the supplied brightness value for a user.

     */

    public void setBrightness(float brightnessValue, int userSerial) {

    public void setBrightness(float brightnessValue, int userSerial, float maxBrightness) {

        mBrightnessSetting.setUserSerial(userSerial);

        setBrightness(brightnessValue);

        setBrightness(brightnessValue, maxBrightness);

    }



    /**

     * Sets the current screen brightness, and notifies the BrightnessSetting about the change.

     */

    public void updateScreenBrightnessSetting(float brightnessValue) {

    public void updateScreenBrightnessSetting(float brightnessValue, float maxBrightness) {

        synchronized (mLock) {

            if (!BrightnessUtils.isValidBrightnessValue(brightnessValue)

                    || brightnessValue == mCurrentScreenBrightness) {
@@ -345,7 +350,7 @@ public final class DisplayBrightnessController { 
            setCurrentScreenBrightnessLocked(brightnessValue);

        }

        notifyCurrentScreenBrightness();

        setBrightness(brightnessValue);

        setBrightness(brightnessValue, maxBrightness);

    }



    /**
@@ -582,7 +587,7 @@ public final class DisplayBrightnessController { 
                float brightnessForDefaultDisplay = getBrightnessFromNits(

                        brightnessNitsForDefaultDisplay);

                if (BrightnessUtils.isValidBrightnessValue(brightnessForDefaultDisplay)) {

                    mBrightnessSetting.setBrightness(brightnessForDefaultDisplay);

                    mBrightnessSetting.setBrightnessNoNotify(brightnessForDefaultDisplay);

                    currentBrightnessSetting = brightnessForDefaultDisplay;

                }

            }

services/tests/displayservicetests/src/com/android/server/display/brightness/DisplayBrightnessControllerTest.java

+75 −11
Original line number Diff line number Diff line
@@ -20,6 +20,7 @@ import static org.junit.Assert.assertEquals; 
import static org.junit.Assert.assertFalse;

import static org.junit.Assert.assertTrue;

import static org.mockito.ArgumentMatchers.any;

import static org.mockito.ArgumentMatchers.anyFloat;

import static org.mockito.ArgumentMatchers.eq;

import static org.mockito.Mockito.clearInvocations;

import static org.mockito.Mockito.mock;
@@ -255,7 +256,8 @@ public final class DisplayBrightnessControllerTest { 
    @Test

    public void setBrightnessSetsInBrightnessSetting() {

        float brightnessValue = 0.3f;

        mDisplayBrightnessController.setBrightness(brightnessValue);

        float maxBrightnessValue = 0.65f;

        mDisplayBrightnessController.setBrightness(brightnessValue, maxBrightnessValue);

        verify(mBrightnessSetting).setBrightness(brightnessValue);

    }
@@ -266,20 +268,24 @@ public final class DisplayBrightnessControllerTest { 


        // Sets the appropriate value when valid, and not equal to the current brightness

        float brightnessValue = 0.3f;

        mDisplayBrightnessController.updateScreenBrightnessSetting(brightnessValue);

        float maxBrightnessValue = 0.65f;

        mDisplayBrightnessController.updateScreenBrightnessSetting(brightnessValue,

                maxBrightnessValue);

        assertEquals(mDisplayBrightnessController.getCurrentBrightness(), brightnessValue, 0.0f);

        verify(mBrightnessChangeExecutor).execute(mOnBrightnessChangeRunnable);

        verify(mBrightnessSetting).setBrightness(brightnessValue);



        // Does nothing if the value is invalid

        mDisplayBrightnessController.updateScreenBrightnessSetting(Float.NaN);

        clearInvocations(mBrightnessSetting);

        mDisplayBrightnessController.updateScreenBrightnessSetting(Float.NaN, maxBrightnessValue);

        verifyNoMoreInteractions(mBrightnessChangeExecutor, mBrightnessSetting);



        // Does nothing if the value is same as the current brightness

        brightnessValue = 0.2f;

        mDisplayBrightnessController.setAndNotifyCurrentScreenBrightness(brightnessValue);

        verify(mBrightnessChangeExecutor, times(2)).execute(mOnBrightnessChangeRunnable);

        mDisplayBrightnessController.updateScreenBrightnessSetting(brightnessValue);

        mDisplayBrightnessController.updateScreenBrightnessSetting(brightnessValue,

                maxBrightnessValue);

        verifyNoMoreInteractions(mBrightnessChangeExecutor, mBrightnessSetting);

    }
@@ -366,7 +372,7 @@ public final class DisplayBrightnessControllerTest { 
        when(mBrightnessSetting.getBrightnessNitsForDefaultDisplay()).thenReturn(nits);

        mDisplayBrightnessController.setAutomaticBrightnessController(

                automaticBrightnessController);

        verify(mBrightnessSetting).setBrightness(brightness);

        verify(mBrightnessSetting).setBrightnessNoNotify(brightness);

        assertEquals(brightness, mDisplayBrightnessController.getCurrentBrightness(), 0.01f);

        clearInvocations(automaticBrightnessController, mBrightnessSetting);
@@ -378,7 +384,7 @@ public final class DisplayBrightnessControllerTest { 
        when(mBrightnessSetting.getBrightness()).thenReturn(brightness);

        mDisplayBrightnessController.setAutomaticBrightnessController(

                automaticBrightnessController);

        verify(mBrightnessSetting, never()).setBrightness(brightness);

        verify(mBrightnessSetting, never()).setBrightnessNoNotify(brightness);

        assertEquals(brightness, mDisplayBrightnessController.getCurrentBrightness(), 0.01f);

        clearInvocations(automaticBrightnessController, mBrightnessSetting);
@@ -395,15 +401,73 @@ public final class DisplayBrightnessControllerTest { 
        assertEquals(brightness, mDisplayBrightnessController.getCurrentBrightness(), 0.01f);

        verifyZeroInteractions(automaticBrightnessController);

        verify(mBrightnessSetting, never()).getBrightnessNitsForDefaultDisplay();

        verify(mBrightnessSetting, never()).setBrightness(brightness);

        verify(mBrightnessSetting, never()).setBrightnessNoNotify(brightness);

    }



    @Test

    public void testDoesNotSetBrightnessNits_settingMaxBrightnessAndStoredValueGreater() {

        float brightnessValue = 0.65f;

        float maxBrightness = 0.65f;

        float nits = 200f;

        float storedNits = 300f;



        when(mBrightnessSetting.getBrightnessNitsForDefaultDisplay()).thenReturn(storedNits);

        AutomaticBrightnessController automaticBrightnessController = mock(

                AutomaticBrightnessController.class);

        when(automaticBrightnessController.convertToNits(brightnessValue)).thenReturn(nits);

        mDisplayBrightnessController.mAutomaticBrightnessController = automaticBrightnessController;



        mDisplayBrightnessController.setBrightness(brightnessValue, maxBrightness);



        verify(mBrightnessSetting, never()).setBrightnessNitsForDefaultDisplay(anyFloat());

    }



    @Test

    public void testSetsBrightnessNits_storedValueLower() {

        float brightnessValue = 0.65f;

        float maxBrightness = 0.65f;

        float nits = 200f;

        float storedNits = 100f;



        when(mBrightnessSetting.getBrightnessNitsForDefaultDisplay()).thenReturn(storedNits);

        AutomaticBrightnessController automaticBrightnessController = mock(

                AutomaticBrightnessController.class);

        when(automaticBrightnessController.convertToNits(brightnessValue)).thenReturn(nits);

        mDisplayBrightnessController.mAutomaticBrightnessController = automaticBrightnessController;



        mDisplayBrightnessController.setBrightness(brightnessValue, maxBrightness);



        verify(mBrightnessSetting).setBrightnessNitsForDefaultDisplay(nits);

    }



    @Test

    public void testSetsBrightnessNits_lowerThanMax() {

        float brightnessValue = 0.60f;

        float maxBrightness = 0.65f;

        float nits = 200f;

        float storedNits = 300f;



        when(mBrightnessSetting.getBrightnessNitsForDefaultDisplay()).thenReturn(storedNits);

        AutomaticBrightnessController automaticBrightnessController = mock(

                AutomaticBrightnessController.class);

        when(automaticBrightnessController.convertToNits(brightnessValue)).thenReturn(nits);

        mDisplayBrightnessController.mAutomaticBrightnessController = automaticBrightnessController;



        mDisplayBrightnessController.setBrightness(brightnessValue, maxBrightness);



        verify(mBrightnessSetting).setBrightnessNitsForDefaultDisplay(nits);

    }





    @Test

    public void testChangeBrightnessNitsWhenUserChanges() {

        float brightnessValue1 = 0.3f;

        float nits1 = 200f;

        int userSerial1 = 1;

        float brightnessValue2 = 0.5f;

        float nits2 = 300f;

        int userSerial2 = 2;

        float maxBrightness = 0.65f;

        AutomaticBrightnessController automaticBrightnessController = mock(

                AutomaticBrightnessController.class);

        when(automaticBrightnessController.convertToNits(brightnessValue1)).thenReturn(nits1);
@@ -417,13 +481,13 @@ public final class DisplayBrightnessControllerTest { 
        verify(automaticBrightnessStrategy)

                .setAutomaticBrightnessController(automaticBrightnessController);



        mDisplayBrightnessController.setBrightness(brightnessValue1, 1 /* user-serial */);

        verify(mBrightnessSetting).setUserSerial(1);

        mDisplayBrightnessController.setBrightness(brightnessValue1, userSerial1, maxBrightness);

        verify(mBrightnessSetting).setUserSerial(userSerial1);

        verify(mBrightnessSetting).setBrightness(brightnessValue1);

        verify(mBrightnessSetting).setBrightnessNitsForDefaultDisplay(nits1);



        mDisplayBrightnessController.setBrightness(brightnessValue2, 2 /* user-serial */);

        verify(mBrightnessSetting).setUserSerial(2);

        mDisplayBrightnessController.setBrightness(brightnessValue2, userSerial2, maxBrightness);

        verify(mBrightnessSetting).setUserSerial(userSerial2);

        verify(mBrightnessSetting).setBrightness(brightnessValue2);

        verify(mBrightnessSetting).setBrightnessNitsForDefaultDisplay(nits2);

    }