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

Commit 6180dffc authored by Oleg Petšjonkin's avatar Oleg Petšjonkin Committed by Android (Google) Code Review
Browse files

Merge "DisplayBrightnessController fixing race with updatePowerState in the...

Merge "DisplayBrightnessController fixing race with updatePowerState in the middle of setBrightness" into main
parents b64be1e3 a11c879b
Loading
Loading
Loading
Loading
+1 −1
Original line number Diff line number Diff line
@@ -1028,7 +1028,7 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call
        }

        BrightnessSetting.BrightnessSettingListener brightnessSettingListener = brightnessValue -> {
            Message msg = mHandler.obtainMessage(MSG_UPDATE_BRIGHTNESS, brightnessValue);
            Message msg = mHandler.obtainMessage(MSG_UPDATE_BRIGHTNESS);
            mHandler.sendMessageAtTime(msg, mClock.uptimeMillis());
        };
        mDisplayBrightnessController
+54 −43
Original line number Diff line number Diff line
@@ -65,11 +65,11 @@ public final class DisplayBrightnessController {
    // mCurrentScreenBrightness is updated.
    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
    // and should be considered a user interaction.
    // The screen brightness that has changed but not taken effect yet. If after applying min
    // and max constraints this is different from the current  screen brightness then this
    // is coming from something other than us and should be considered a user interaction.
    @GuardedBy("mLock")
    private float mPendingScreenBrightness;
    private float mPendingUnthrottledScreenBrightness;

    // The last observed screen brightness, either set by us or by the settings app on
    // behalf of the user.
@@ -142,7 +142,7 @@ public final class DisplayBrightnessController {
        mDisplayId = displayId;
        // TODO: b/186428377 update brightness setting when display changes
        mBrightnessSetting = brightnessSetting;
        mPendingScreenBrightness = PowerManager.BRIGHTNESS_INVALID_FLOAT;
        mPendingUnthrottledScreenBrightness = PowerManager.BRIGHTNESS_INVALID_FLOAT;
        mScreenBrightnessDefault = BrightnessUtils.clampAbsoluteBrightness(defaultScreenBrightness);
        mCurrentScreenBrightness = getScreenBrightnessSetting();
        mCurrentUnthrottledBrightness = mCurrentScreenBrightness;
@@ -267,10 +267,9 @@ public final class DisplayBrightnessController {
    public void setAndNotifyCurrentScreenBrightness(float brightnessValue) {
        final boolean hasBrightnessChanged;
        synchronized (mLock) {
            float brightnessAdjusted = MathUtils
                    .constrain(brightnessValue, mCurrentMinBrightness, mCurrentMaxBrightness);
            float brightnessAdjusted = getBrightnessConstrainedLocked(brightnessValue);
            hasBrightnessChanged = (brightnessAdjusted != mCurrentScreenBrightness);
            setCurrentScreenBrightnessLocked(brightnessAdjusted);
            mCurrentScreenBrightness = brightnessAdjusted;
            mCurrentUnthrottledBrightness = brightnessValue;
        }
        if (hasBrightnessChanged) {
@@ -287,12 +286,10 @@ public final class DisplayBrightnessController {
        }
    }

    /**
     * Returns the screen brightness which has changed but has not taken any effect so far.
     */
    public float getPendingScreenBrightness() {
    @VisibleForTesting
    float getPendingScreenBrightness() {
        synchronized (mLock) {
            return mPendingScreenBrightness;
            return mPendingUnthrottledScreenBrightness;
        }
    }

@@ -302,8 +299,7 @@ public final class DisplayBrightnessController {
    public void handleSettingsChange() {
        float brightness = getScreenBrightnessSetting();
        synchronized (mLock) {
            mPendingScreenBrightness = MathUtils.constrain(
                    brightness, mCurrentMinBrightness, mCurrentMaxBrightness);
            mPendingUnthrottledScreenBrightness = brightness;
        }
    }

@@ -353,13 +349,15 @@ public final class DisplayBrightnessController {
     * Returns the current screen brightnessSetting constrained by current min and max values
     */
    public float getScreenBrightnessSettingConstrained() {
        float brightness = getScreenBrightnessSetting();
        float maxBrightness, minBrightness;
        float brightness = mBrightnessSetting.getBrightness();
        if (Float.isNaN(brightness)) {
            brightness = mScreenBrightnessDefault;
        }
        float brightnessAdjusted;
        synchronized (mLock) {
            maxBrightness = mCurrentMaxBrightness;
            minBrightness = mCurrentMinBrightness;
            brightnessAdjusted = getBrightnessConstrainedLocked(brightness);
        }
        return MathUtils.constrain(brightness, minBrightness, maxBrightness);
        return brightnessAdjusted;
    }

    /**
@@ -397,7 +395,6 @@ public final class DisplayBrightnessController {
        float maxBrightness;
        synchronized (mLock) {
            maxBrightness = mCurrentMaxBrightness;
            mCurrentUnthrottledBrightness = brightnessValue;
        }
        setBrightnessInternal(brightnessValue, maxBrightness);
    }
@@ -414,21 +411,34 @@ public final class DisplayBrightnessController {
     */
    public void updateScreenBrightnessSetting(float brightnessValue,
            float minBrightness, float maxBrightness) {
        float adjustedBrightness = MathUtils
        float constrainedBrightness = MathUtils
                .constrain(brightnessValue, minBrightness, maxBrightness);
        boolean constrainedBrightnessChanged = false;
        boolean rawBrightnessChanged = false;
        synchronized (mLock) {
            mCurrentMaxBrightness = maxBrightness;
            mCurrentMinBrightness = minBrightness;
            if (!BrightnessUtils.isValidBrightnessValue(adjustedBrightness)
                    || adjustedBrightness == mCurrentScreenBrightness) {
            if (!BrightnessUtils.isValidBrightnessValue(brightnessValue)) {
                return;
            }
            if (constrainedBrightness != mCurrentScreenBrightness) {
                constrainedBrightnessChanged = true;
            }
            if (brightnessValue != mCurrentUnthrottledBrightness) {
                rawBrightnessChanged = true;
            }

            mCurrentScreenBrightness = constrainedBrightness;
            mCurrentUnthrottledBrightness = brightnessValue;
            setCurrentScreenBrightnessLocked(adjustedBrightness);
        }

        if (constrainedBrightnessChanged) {
            notifyCurrentScreenBrightness();
        }
        if (rawBrightnessChanged) {
            setBrightnessInternal(brightnessValue, maxBrightness);
        }
    }

    /**
     * Sets up the auto brightness and the relevant state for the associated display
@@ -555,7 +565,8 @@ public final class DisplayBrightnessController {
        synchronized (mLock) {
            writer.println("  mCurrentMinBrightness=" + mCurrentMinBrightness);
            writer.println("  mCurrentMaxBrightness=" + mCurrentMaxBrightness);
            writer.println("  mPendingScreenBrightness=" + mPendingScreenBrightness);
            writer.println("  mPendingUnthrottledScreenBrightness="
                    + mPendingUnthrottledScreenBrightness);
            writer.println("  mCurrentScreenBrightness=" + mCurrentScreenBrightness);
            writer.println("  mCurrentUnthrottledBrightness=" + mCurrentUnthrottledBrightness);
            writer.println("  mLastUserSetScreenBrightness="
@@ -578,18 +589,20 @@ public final class DisplayBrightnessController {
    boolean updateUserSetScreenBrightness() {
        mUserSetScreenBrightnessUpdated = false;
        synchronized (mLock) {
            if (!BrightnessUtils.isValidBrightnessValue(mPendingScreenBrightness)) {
            if (!BrightnessUtils.isValidBrightnessValue(mPendingUnthrottledScreenBrightness)) {
                return false;
            }
            if (mCurrentScreenBrightness == mPendingScreenBrightness) {
                mPendingScreenBrightness = PowerManager.BRIGHTNESS_INVALID_FLOAT;
            float pendingBrightnessConstrained = getBrightnessConstrainedLocked(
                    mPendingUnthrottledScreenBrightness);
            mCurrentUnthrottledBrightness = mPendingUnthrottledScreenBrightness;
            mPendingUnthrottledScreenBrightness = PowerManager.BRIGHTNESS_INVALID_FLOAT;
            setTemporaryBrightnessLocked(PowerManager.BRIGHTNESS_INVALID_FLOAT);

            if (mCurrentScreenBrightness == pendingBrightnessConstrained) {
                return false;
            }
            setCurrentScreenBrightnessLocked(mPendingScreenBrightness);
            mLastUserSetScreenBrightness = mPendingScreenBrightness;
            mPendingScreenBrightness = PowerManager.BRIGHTNESS_INVALID_FLOAT;
            setTemporaryBrightnessLocked(PowerManager.BRIGHTNESS_INVALID_FLOAT);
            mCurrentScreenBrightness = pendingBrightnessConstrained;
            mLastUserSetScreenBrightness = pendingBrightnessConstrained;
        }
        notifyCurrentScreenBrightness();
        mUserSetScreenBrightnessUpdated = true;
@@ -677,13 +690,6 @@ public final class DisplayBrightnessController {
                .setTemporaryScreenBrightness(temporaryBrightness);
    }

    @GuardedBy("mLock")
    private void setCurrentScreenBrightnessLocked(float brightnessValue) {
        if (brightnessValue != mCurrentScreenBrightness) {
            mCurrentScreenBrightness = brightnessValue;
        }
    }

    private void notifyCurrentScreenBrightness() {
        mBrightnessChangeExecutor.execute(mOnBrightnessChangeRunnable);
    }
@@ -738,4 +744,9 @@ public final class DisplayBrightnessController {
        return new StrategyExecutionRequest(displayPowerRequest, mCurrentUnthrottledBrightness,
                mUserSetScreenBrightnessUpdated, mIsStylusBeingUsed, offloadSession);
    }

    @GuardedBy("mLock")
    private float getBrightnessConstrainedLocked(float brightness) {
        return MathUtils.constrain(brightness, mCurrentMinBrightness, mCurrentMaxBrightness);
    }
}
+55 −1
Original line number Diff line number Diff line
@@ -40,8 +40,8 @@ import android.os.HandlerExecutor;
import android.os.PowerManager;
import android.view.Display;

import androidx.test.ext.junit.runners.AndroidJUnit4;
import androidx.test.filters.SmallTest;
import androidx.test.runner.AndroidJUnit4;

import com.android.server.display.AutomaticBrightnessController;
import com.android.server.display.BrightnessMappingStrategy;
@@ -64,6 +64,7 @@ import org.mockito.MockitoAnnotations;
@SmallTest
@RunWith(AndroidJUnit4.class)
public final class DisplayBrightnessControllerTest {
    private static final float FLOAT_TOLERANCE = 0.001f;
    private static final int DISPLAY_ID = Display.DEFAULT_DISPLAY;
    private static final float DEFAULT_BRIGHTNESS = 0.15f;

@@ -641,4 +642,57 @@ public final class DisplayBrightnessControllerTest {
        mDisplayBrightnessController.setStylusBeingUsed(true);
        assertTrue(mDisplayBrightnessController.isStylusBeingUsed());
    }

    @Test
    public void testBrightnessSet_withUpdatePowerStateInTheMiddle() {
        TemporaryBrightnessStrategy temporaryBrightnessStrategy = mock(
                TemporaryBrightnessStrategy.class);
        when(mDisplayBrightnessStrategySelector.getTemporaryDisplayBrightnessStrategy()).thenReturn(
                temporaryBrightnessStrategy);
        float currentMin = 0.1f;
        float currentMax = 0.8f;
        float currentBrightness = 0.3f;
        float newBrightness = 0.5f;
        // initial screen brightness
        mDisplayBrightnessController.updateScreenBrightnessSetting(
                currentBrightness, currentMin, currentMax);
        when(mBrightnessSetting.getBrightness()).thenReturn(currentBrightness);
        assertEquals(currentBrightness, mDisplayBrightnessController.getCurrentBrightness(),
                FLOAT_TOLERANCE);
        assertEquals(PowerManager.BRIGHTNESS_INVALID_FLOAT,
                mDisplayBrightnessController.getPendingScreenBrightness(), FLOAT_TOLERANCE);

        // request to change brightness. Persist but don't process.
        mDisplayBrightnessController.setBrightness(newBrightness);
        verify(mBrightnessSetting).setBrightness(newBrightness);
        when(mBrightnessSetting.getBrightness()).thenReturn(newBrightness);
        assertEquals(currentBrightness, mDisplayBrightnessController.getCurrentBrightness(),
                FLOAT_TOLERANCE);
        assertEquals(PowerManager.BRIGHTNESS_INVALID_FLOAT,
                mDisplayBrightnessController.getPendingScreenBrightness(), FLOAT_TOLERANCE);

        // updatePowerState loop, with current brightness
        clearInvocations(mBrightnessSetting, mBrightnessChangeExecutor);
        mDisplayBrightnessController.updateScreenBrightnessSetting(
                currentBrightness, currentMin, currentMax);
        verifyNoMoreInteractions(mBrightnessSetting, mBrightnessChangeExecutor);
        assertEquals(currentBrightness, mDisplayBrightnessController.getCurrentBrightness(),
                FLOAT_TOLERANCE);
        assertEquals(PowerManager.BRIGHTNESS_INVALID_FLOAT,
                mDisplayBrightnessController.getPendingScreenBrightness(), FLOAT_TOLERANCE);

        // process brightness change request
        mDisplayBrightnessController.handleSettingsChange();
        assertEquals(currentBrightness, mDisplayBrightnessController.getCurrentBrightness(),
                FLOAT_TOLERANCE);
        assertEquals(newBrightness, mDisplayBrightnessController.getPendingScreenBrightness(),
                FLOAT_TOLERANCE);

        mDisplayBrightnessController.updateUserSetScreenBrightness();
        assertEquals(newBrightness, mDisplayBrightnessController.getCurrentBrightness(),
                FLOAT_TOLERANCE);
        assertEquals(PowerManager.BRIGHTNESS_INVALID_FLOAT,
                mDisplayBrightnessController.getPendingScreenBrightness(), FLOAT_TOLERANCE);
        verify(mBrightnessChangeExecutor).execute(mOnBrightnessChangeRunnable);
    }
}