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

Commit bb22facf authored by petsjonkin's avatar petsjonkin
Browse files

Settings should store unthrottled brightness 

In case of manual brightness, when throttled state is stored user set brightness is lost.
When conditions are back to normal, original brightness state is not restored.

Bug: b/403303379, b/356777411
Test: atest DisplayBrightnessControllerTest, atest DisplayPowerControllerTest,  maual testing, see bug comments
Flag: EXEMPT bugfix
Change-Id: I3507846a04ec5326f958211cace52f4e85ae38c3
parent 2715b6e3

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

+1 −1
Original line number Diff line number Diff line
@@ -149,7 +149,7 @@ public class DisplayOffloadSessionImpl implements DisplayManagerInternal.Display 


    @Override

    public float getBrightness() {

        return mDisplayPowerController.getScreenBrightnessSetting();

        return mDisplayPowerController.getCurrentScreenBrightness();

    }



    @Override

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

+13 −18
Original line number Diff line number Diff line
@@ -596,8 +596,8 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call 
                displayToken, displayDeviceInfo);



        mDisplayBrightnessController =

                new DisplayBrightnessController(context, null,

                        mDisplayId, mLogicalDisplay.getDisplayInfoLocked().brightnessDefault,

                new DisplayBrightnessController(context, mDisplayId,

                        mLogicalDisplay.getDisplayInfoLocked().brightnessDefault,

                        brightnessSetting, () -> postBrightnessChangeRunnable(),

                        new HandlerExecutor(mHandler), flags, mDisplayDeviceConfig);
@@ -1600,8 +1600,9 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call 
            // so that the slider accurately represents the full possible range,

            // even if they range changes what it means in absolute terms.

            mDisplayBrightnessController.updateScreenBrightnessSetting(

                    MathUtils.constrain(unthrottledBrightnessState,

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

                    unthrottledBrightnessState,

                    Math.max(mBrightnessRangeController.getCurrentBrightnessMin(),

                            clampedState.getMinBrightness()),

                    Math.min(mBrightnessRangeController.getCurrentBrightnessMax(),

                            clampedState.getMaxBrightness()));

        }
@@ -2495,9 +2496,7 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call 
    }



    private void handleSettingsChange() {

        mDisplayBrightnessController

                .setPendingScreenBrightness(mDisplayBrightnessController

                        .getScreenBrightnessSetting());

        mDisplayBrightnessController.handleSettingsChange();

        mAutomaticBrightnessStrategy.updatePendingAutoBrightnessAdjustments();

        sendUpdatePowerState();

    }
@@ -2517,7 +2516,11 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call 
    }



    public float getScreenBrightnessSetting() {

        return mDisplayBrightnessController.getScreenBrightnessSetting();

        return mDisplayBrightnessController.getScreenBrightnessSettingConstrained();

    }



    public float getCurrentScreenBrightness() {

        return mDisplayBrightnessController.getCurrentBrightness();

    }



    public float getDozeBrightnessForOffload() {
@@ -2529,19 +2532,11 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call 
    }



    public void setBrightness(float 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());

        mDisplayBrightnessController.setBrightness(brightness);

    }



    public void setBrightness(float brightness, int 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());

        mDisplayBrightnessController.setBrightness(brightness, userSerial);

    }



    public int getDisplayId() {

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

+84 −32
Original line number Diff line number Diff line
@@ -24,6 +24,7 @@ import android.os.Handler; 
import android.os.HandlerExecutor;

import android.os.PowerManager;

import android.util.IndentingPrintWriter;

import android.util.MathUtils;

import android.view.Display;



import com.android.internal.annotations.GuardedBy;
@@ -62,7 +63,7 @@ public final class DisplayBrightnessController { 
    // A runnable to update the clients registered via DisplayManagerGlobal

    // .EVENT_DISPLAY_BRIGHTNESS_CHANGED about the brightness change. Called when

    // mCurrentScreenBrightness is updated.

    private Runnable mOnBrightnessChangeRunnable;

    private final Runnable mOnBrightnessChangeRunnable;



    // The screen brightness that has changed but not taken effect yet. If this is different

    // from the current screen brightness then this is coming from something other than us
@@ -75,6 +76,15 @@ public final class DisplayBrightnessController { 
    @GuardedBy("mLock")

    private float mCurrentScreenBrightness;



    @GuardedBy("mLock")

    private float mCurrentMaxBrightness;



    @GuardedBy("mLock")

    private float mCurrentMinBrightness;



    @GuardedBy("mLock")

    private float mCurrentUnthrottledBrightness;



    // The last brightness that was set by the user and not temporary. Set to

    // PowerManager.BRIGHTNESS_INVALID_FLOAT when a brightness has yet to be recorded.

    @GuardedBy("mLock")
@@ -90,7 +100,7 @@ public final class DisplayBrightnessController { 


    // Selects an appropriate strategy based on the request provided by the clients.

    @GuardedBy("mLock")

    private DisplayBrightnessStrategySelector mDisplayBrightnessStrategySelector;

    private final DisplayBrightnessStrategySelector mDisplayBrightnessStrategySelector;



    // Currently selected DisplayBrightnessStrategy.

    @GuardedBy("mLock")
@@ -116,19 +126,28 @@ public final class DisplayBrightnessController { 
    /**

     * The constructor of DisplayBrightnessController.

     */

    public DisplayBrightnessController(Context context, Injector injector, int displayId,

    public DisplayBrightnessController(Context context, int displayId,

            float defaultScreenBrightness, BrightnessSetting brightnessSetting,

            Runnable onBrightnessChangeRunnable, HandlerExecutor brightnessChangeExecutor,

            DisplayManagerFlags flags, DisplayDeviceConfig config) {

        if (injector == null) {

            injector = new Injector();

        this(context, new Injector(), displayId, defaultScreenBrightness, brightnessSetting,

                onBrightnessChangeRunnable, brightnessChangeExecutor, flags, config);

    }



    @VisibleForTesting

    DisplayBrightnessController(Context context, Injector injector, int displayId,

            float defaultScreenBrightness, BrightnessSetting brightnessSetting,

            Runnable onBrightnessChangeRunnable, HandlerExecutor brightnessChangeExecutor,

            DisplayManagerFlags flags, DisplayDeviceConfig config) {

        mDisplayId = displayId;

        // TODO: b/186428377 update brightness setting when display changes

        mBrightnessSetting = brightnessSetting;

        mPendingScreenBrightness = PowerManager.BRIGHTNESS_INVALID_FLOAT;

        mScreenBrightnessDefault = BrightnessUtils.clampAbsoluteBrightness(defaultScreenBrightness);

        mCurrentScreenBrightness = getScreenBrightnessSetting();

        mCurrentUnthrottledBrightness = mCurrentScreenBrightness;

        mCurrentMaxBrightness = PowerManager.BRIGHTNESS_MAX;

        mCurrentMinBrightness = PowerManager.BRIGHTNESS_MIN;

        mOnBrightnessChangeRunnable = onBrightnessChangeRunnable;

        mDisplayBrightnessStrategySelector = injector.getDisplayBrightnessStrategySelector(context,

                displayId, flags, config);
@@ -158,8 +177,8 @@ public final class DisplayBrightnessController { 
                    constructStrategySelectionRequest(displayPowerRequest, targetDisplayState,

                            displayOffloadSession, isBedtimeModeWearEnabled));

            state = mDisplayBrightnessStrategy

                        .updateBrightness(constructStrategyExecutionRequest(displayPowerRequest,

                                displayOffloadSession));

                        .updateBrightness(constructStrategyExecutionRequestLocked(

                                displayPowerRequest, displayOffloadSession));

        }



        // This is a temporary measure until AutomaticBrightnessStrategy works as a traditional
@@ -248,8 +267,11 @@ public final class DisplayBrightnessController { 
    public void setAndNotifyCurrentScreenBrightness(float brightnessValue) {

        final boolean hasBrightnessChanged;

        synchronized (mLock) {

            hasBrightnessChanged = (brightnessValue != mCurrentScreenBrightness);

            setCurrentScreenBrightnessLocked(brightnessValue);

            float brightnessAdjusted = MathUtils

                    .constrain(brightnessValue, mCurrentMinBrightness, mCurrentMaxBrightness);

            hasBrightnessChanged = (brightnessAdjusted != mCurrentScreenBrightness);

            setCurrentScreenBrightnessLocked(brightnessAdjusted);

            mCurrentUnthrottledBrightness = brightnessValue;

        }

        if (hasBrightnessChanged) {

            notifyCurrentScreenBrightness();
@@ -275,13 +297,11 @@ public final class DisplayBrightnessController { 
    }



    /**

     * Sets the pending screen brightness setting, representing a value which is requested, but not

     * yet processed.

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

     * Updates pending brightness with value from settings

     */

    public void setPendingScreenBrightness(float brightnessValue) {

    public void handleSettingsChange() {

        synchronized (mLock) {

            mPendingScreenBrightness = brightnessValue;

            mPendingScreenBrightness = getScreenBrightnessSettingConstrained();

        }

    }
@@ -319,20 +339,31 @@ public final class DisplayBrightnessController { 
     * Returns the current screen brightnessSetting which is responsible for saving the brightness

     * in the persistent store

     */

    public float getScreenBrightnessSetting() {

    private float getScreenBrightnessSetting() {

        float brightness = mBrightnessSetting.getBrightness();

        synchronized (mLock) {

        if (Float.isNaN(brightness)) {

            brightness = mScreenBrightnessDefault;

        }

        return BrightnessUtils.clampAbsoluteBrightness(brightness);

    }



    /**

     * Returns the current screen brightnessSetting constrained by current min and max values

     */

    public float getScreenBrightnessSettingConstrained() {

        float brightness = getScreenBrightnessSetting();

        float maxBrightness, minBrightness;

        synchronized (mLock) {

            maxBrightness = mCurrentMaxBrightness;

            minBrightness = mCurrentMinBrightness;

        }

        return MathUtils.constrain(brightness, minBrightness, maxBrightness);

    }



    /**

     * Notifies the brightnessSetting to persist the supplied brightness value.

     */

    public void setBrightness(float brightnessValue, float maxBrightness) {

    private void setBrightnessInternal(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);
@@ -343,7 +374,7 @@ public final class DisplayBrightnessController { 
            // 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)) {

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

                mBrightnessSetting.setBrightnessNitsForDefaultDisplay(nits);

            }

        }
@@ -352,9 +383,21 @@ public final class DisplayBrightnessController { 
    /**

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

     */

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

    public void setBrightness(float brightnessValue, int userSerial) {

        mBrightnessSetting.setUserSerial(userSerial);

        setBrightness(brightnessValue, maxBrightness);

        setBrightness(brightnessValue);

    }



    /**

     * Notifies the brightnessSetting to persist the supplied brightness value.

     */

    public void setBrightness(float brightnessValue) {

        float maxBrightness;

        synchronized (mLock) {

            maxBrightness = mCurrentMaxBrightness;

            mCurrentUnthrottledBrightness = brightnessValue;

        }

        setBrightnessInternal(brightnessValue, maxBrightness);

    }



    /**
@@ -367,16 +410,22 @@ public final class DisplayBrightnessController { 
    /**

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

     */

    public void updateScreenBrightnessSetting(float brightnessValue, float maxBrightness) {

    public void updateScreenBrightnessSetting(float brightnessValue,

            float minBrightness, float maxBrightness) {

        float adjustedBrightness = MathUtils

                .constrain(brightnessValue, minBrightness, maxBrightness);

        synchronized (mLock) {

            if (!BrightnessUtils.isValidBrightnessValue(brightnessValue)

                    || brightnessValue == mCurrentScreenBrightness) {

            mCurrentMaxBrightness = maxBrightness;

            mCurrentMinBrightness = minBrightness;

            if (!BrightnessUtils.isValidBrightnessValue(adjustedBrightness)

                    || adjustedBrightness == mCurrentScreenBrightness) {

                return;

            }

            setCurrentScreenBrightnessLocked(brightnessValue);

            mCurrentUnthrottledBrightness = brightnessValue;

            setCurrentScreenBrightnessLocked(adjustedBrightness);

        }

        notifyCurrentScreenBrightness();

        setBrightness(brightnessValue, maxBrightness);

        setBrightnessInternal(brightnessValue, maxBrightness);

    }



    /**
@@ -502,8 +551,11 @@ public final class DisplayBrightnessController { 
        writer.println("  mIsStylusBeingUsed="

                + mIsStylusBeingUsed);

        synchronized (mLock) {

            writer.println("  mCurrentMinBrightness=" + mCurrentMinBrightness);

            writer.println("  mCurrentMaxBrightness=" + mCurrentMaxBrightness);

            writer.println("  mPendingScreenBrightness=" + mPendingScreenBrightness);

            writer.println("  mCurrentScreenBrightness=" + mCurrentScreenBrightness);

            writer.println("  mCurrentUnthrottledBrightness=" + mCurrentUnthrottledBrightness);

            writer.println("  mLastUserSetScreenBrightness="

                    + mLastUserSetScreenBrightness);

            if (mDisplayBrightnessStrategy != null) {
@@ -677,11 +729,11 @@ public final class DisplayBrightnessController { 
                mIsStylusBeingUsed, isBedtimeModeEnabled);

    }



    private StrategyExecutionRequest constructStrategyExecutionRequest(

    @GuardedBy("mLock")

    private StrategyExecutionRequest constructStrategyExecutionRequestLocked(

            DisplayManagerInternal.DisplayPowerRequest displayPowerRequest,

            DisplayManagerInternal.DisplayOffloadSession offloadSession) {

        float currentScreenBrightness = getCurrentBrightness();

        return new StrategyExecutionRequest(displayPowerRequest, currentScreenBrightness,

        return new StrategyExecutionRequest(displayPowerRequest, mCurrentUnthrottledBrightness,

                mUserSetScreenBrightnessUpdated, mIsStylusBeingUsed, offloadSession);

    }

}

services/tests/displayservicetests/src/com/android/server/display/DisplayPowerControllerTest.java

+10 −3
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@ 
package com.android.server.display;



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

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

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

import static com.android.server.display.AutomaticBrightnessController.AUTO_BRIGHTNESS_MODE_BEDTIME_WEAR;

import static com.android.server.display.AutomaticBrightnessController.AUTO_BRIGHTNESS_MODE_DEFAULT;
@@ -104,6 +105,7 @@ import org.junit.Test; 
import org.junit.runner.RunWith;

import org.mockito.ArgumentCaptor;

import org.mockito.Captor;

import org.mockito.InOrder;

import org.mockito.Mock;

import org.mockito.quality.Strictness;

import org.mockito.stubbing.Answer;
@@ -1349,14 +1351,19 @@ public final class DisplayPowerControllerTest { 
    }



    @Test

    public void testSetBrightness_BrightnessShouldBeClamped() {

    public void testSetBrightness_BrightnessShouldBeUnlamped() {

        float clampedBrightness = 0.6f;

        float unclampedBrightness = 0.8f;

        int userSerial = 123;

        when(mHolder.hbmController.getCurrentBrightnessMax()).thenReturn(clampedBrightness);



        mHolder.dpc.setBrightness(PowerManager.BRIGHTNESS_MAX);

        mHolder.dpc.setBrightness(0.8f, /* userSerial= */ 123);

        mHolder.dpc.setBrightness(unclampedBrightness, userSerial);



        verify(mHolder.brightnessSetting, times(2)).setBrightness(clampedBrightness);

        InOrder inOrder = inOrder(mHolder.brightnessSetting);

        inOrder.verify(mHolder.brightnessSetting).setBrightness(PowerManager.BRIGHTNESS_MAX);

        inOrder.verify(mHolder.brightnessSetting).setUserSerial(userSerial);

        inOrder.verify(mHolder.brightnessSetting).setBrightness(unclampedBrightness);

    }



    @Test

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

+83 −17

File changed.

Preview size limit exceeded, changes collapsed.