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

Commit 591a1503 authored by Piotr Wilczyński's avatar Piotr Wilczyński
Browse files

Fix brightness per user

- Don't set user serial to -1. When brightness is set without providing a user serial, the current user serial should be used.

- Make sure that handleBrightnessModeChange is called before a new brightness is set for the new user, otherwise auto-brightness might overwrite the brightness for the new user even if the new user has auto-brightness turned off. To that end, handleBrightnessModeChange won't be posting to the handler thread any more (it doesn't need to - everything happens on the power thread anyway) and DPC will set the brightness for the new user instead of DisplayManagerService.

- Remove the call to updatePowerState from handleBrightnessModeChange - it was only needed if handleBrightnessModeChange was called from SettingsObserver.onChange and it was making DPC tests difficult to write because there would sometimes be two calls to updatePowerState. Now DPC tests are easier to write and we only have to verify single interactions with mocks.

- DisplayPowerControllerTest#testReleaseProxSuspendBlockersOnExit was incorrect - it was checking if wake locks were acquired twice, which is not necessary. It should check if they were released instead because that's what should happen when we stop the DPC.

- Separate handleSettingsChange and handleOnSwitchUser.

- Don't process auto-brightness adjustment when switching users - the adjustment is related to the short-term model. We're resetting the short-term model when switching users.

- Use the default brightness for new users.

Bug: 279109106
Test: Switch users and see what happens to the brightness.
Test: adb shell dumpsys display
Test: atest com.android.server.display
Change-Id: I2469ce8a78c8c3c6eb3f66059cef6318db4428ef
parent d8851476
Loading
Loading
Loading
Loading
+7 −7
Original line number Diff line number Diff line
@@ -695,14 +695,14 @@ public final class DisplayManagerService extends SystemService {
                            logicalDisplay.getPrimaryDisplayDeviceLocked().getUniqueId(),
                            userSerial);
                    dpc.setBrightnessConfiguration(config, /* shouldResetShortTermModel= */ true);
                    // change the brightness value according to the selected user.
                    final DisplayDevice device = logicalDisplay.getPrimaryDisplayDeviceLocked();
                    if (device != null) {
                        dpc.setBrightness(
                                mPersistentDataStore.getBrightness(device, userSerial), userSerial);
                }
                final DisplayDevice device = logicalDisplay.getPrimaryDisplayDeviceLocked();
                float newBrightness = device == null ? PowerManager.BRIGHTNESS_INVALID_FLOAT
                        : mPersistentDataStore.getBrightness(device, userSerial);
                if (Float.isNaN(newBrightness)) {
                    newBrightness = logicalDisplay.getDisplayInfoLocked().brightnessDefault;
                }
                dpc.onSwitchUser(newUserId);
                dpc.onSwitchUser(newUserId, userSerial, newBrightness);
            });
            handleSettingsChange();
        }
+35 −28
Original line number Diff line number Diff line
@@ -685,17 +685,27 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call
    }

    @Override
    public void onSwitchUser(@UserIdInt int newUserId) {
        Message msg = mHandler.obtainMessage(MSG_SWITCH_USER, newUserId);
        mHandler.sendMessage(msg);
    public void onSwitchUser(@UserIdInt int newUserId, int userSerial, float newBrightness) {
        Message msg = mHandler.obtainMessage(MSG_SWITCH_USER, newUserId, userSerial, newBrightness);
        mHandler.sendMessageAtTime(msg, mClock.uptimeMillis());
    }

    private void handleOnSwitchUser(@UserIdInt int newUserId) {
        handleSettingsChange(true /* userSwitch */);
    private void handleOnSwitchUser(@UserIdInt int newUserId, int userSerial, float newBrightness) {
        Slog.i(mTag, "Switching user newUserId=" + newUserId + " userSerial=" + userSerial
                + " newBrightness=" + newBrightness);
        handleBrightnessModeChange();
        if (mBrightnessTracker != null) {
            mBrightnessTracker.onSwitchUser(newUserId);
        }
        setBrightness(newBrightness, userSerial);

        // Don't treat user switches as user initiated change.
        mDisplayBrightnessController.setAndNotifyCurrentScreenBrightness(newBrightness);

        if (mAutomaticBrightnessController != null) {
            mAutomaticBrightnessController.resetShortTermModel();
        }
        sendUpdatePowerState();
    }

    @Nullable
@@ -2394,20 +2404,11 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call
        MetricsLogger.action(log);
    }

    private void handleSettingsChange(boolean userSwitch) {
    private void handleSettingsChange() {
        mDisplayBrightnessController
                .setPendingScreenBrightness(mDisplayBrightnessController
                        .getScreenBrightnessSetting());
        mAutomaticBrightnessStrategy.updatePendingAutoBrightnessAdjustments(userSwitch);
        if (userSwitch) {
            // Don't treat user switches as user initiated change.
            mDisplayBrightnessController
                    .setAndNotifyCurrentScreenBrightness(mDisplayBrightnessController
                            .getPendingScreenBrightness());
            if (mAutomaticBrightnessController != null) {
                mAutomaticBrightnessController.resetShortTermModel();
            }
        }
        mAutomaticBrightnessStrategy.updatePendingAutoBrightnessAdjustments();
        sendUpdatePowerState();
    }

@@ -2416,11 +2417,8 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call
                mContext.getContentResolver(),
                Settings.System.SCREEN_BRIGHTNESS_MODE,
                Settings.System.SCREEN_BRIGHTNESS_MODE_MANUAL, UserHandle.USER_CURRENT);
        mHandler.postAtTime(() -> {
        mAutomaticBrightnessStrategy.setUseAutoBrightness(screenBrightnessModeSetting
                == Settings.System.SCREEN_BRIGHTNESS_MODE_AUTOMATIC);
            updatePowerState();
        }, mClock.uptimeMillis());
    }


@@ -2430,9 +2428,13 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call
    }

    @Override
    public void setBrightness(float brightnessValue, int userSerial) {
        mDisplayBrightnessController.setBrightness(clampScreenBrightness(brightnessValue),
                userSerial);
    public void setBrightness(float brightness) {
        mDisplayBrightnessController.setBrightness(clampScreenBrightness(brightness));
    }

    @Override
    public void setBrightness(float brightness, int userSerial) {
        mDisplayBrightnessController.setBrightness(clampScreenBrightness(brightness), userSerial);
    }

    @Override
@@ -2966,7 +2968,7 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call
                    if (mStopped) {
                        return;
                    }
                    handleSettingsChange(false /*userSwitch*/);
                    handleSettingsChange();
                    break;

                case MSG_UPDATE_RBC:
@@ -2985,7 +2987,9 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call
                    break;

                case MSG_SWITCH_USER:
                    handleOnSwitchUser(msg.arg1);
                    float newBrightness = msg.obj instanceof Float ? (float) msg.obj
                            : PowerManager.BRIGHTNESS_INVALID_FLOAT;
                    handleOnSwitchUser(msg.arg1, msg.arg2, newBrightness);
                    break;

                case MSG_BOOT_COMPLETED:
@@ -3023,7 +3027,10 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call
        @Override
        public void onChange(boolean selfChange, Uri uri) {
            if (uri.equals(Settings.System.getUriFor(Settings.System.SCREEN_BRIGHTNESS_MODE))) {
                mHandler.postAtTime(() -> {
                    handleBrightnessModeChange();
                    updatePowerState();
                }, mClock.uptimeMillis());
            } else if (uri.equals(Settings.System.getUriFor(
                    Settings.System.SCREEN_BRIGHTNESS_FOR_ALS))) {
                int preset = Settings.System.getIntForUser(mContext.getContentResolver(),
@@ -3035,7 +3042,7 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call
                setUpAutoBrightness(mContext, mHandler);
                sendUpdatePowerState();
            } else {
                handleSettingsChange(false /* userSwitch */);
                handleSettingsChange();
            }
        }
    }
+5 −7
Original line number Diff line number Diff line
@@ -30,7 +30,6 @@ import java.io.PrintWriter;
 * An interface to manage the display's power state and brightness
 */
public interface DisplayPowerControllerInterface {
    int DEFAULT_USER_SERIAL = -1;
    /**
     * Notified when the display is changed.
     *
@@ -100,15 +99,12 @@ public interface DisplayPowerControllerInterface {
     * Set the screen brightness of the associated display
     * @param brightness The value to which the brightness is to be set
     */
    default void setBrightness(float brightness) {
        setBrightness(brightness, DEFAULT_USER_SERIAL);
    }
    void setBrightness(float brightness);

    /**
     * Set the screen brightness of the associated display
     * @param brightness The value to which the brightness is to be set
     * @param userSerial The user for which the brightness value is to be set. Use userSerial = -1,
     * if brightness needs to be updated for the current user.
     * @param userSerial The user for which the brightness value is to be set.
     */
    void setBrightness(float brightness, int userSerial);

@@ -188,8 +184,10 @@ public interface DisplayPowerControllerInterface {
    /**
     * Handles the changes to be done to update the brightness when the user is changed
     * @param newUserId The new userId
     * @param userSerial The serial number of the new user
     * @param newBrightness The brightness for the new user
     */
    void onSwitchUser(int newUserId);
    void onSwitchUser(int newUserId, int userSerial, float newBrightness);

    /**
     * Get the ID of the display associated with this DPC.
+8 −9
Original line number Diff line number Diff line
@@ -41,7 +41,6 @@ import java.io.PrintWriter;
 * display. Applies the chosen brightness.
 */
public final class DisplayBrightnessController {
    private static final int DEFAULT_USER_SERIAL = -1;

    // The ID of the display tied to this DisplayBrightnessController
    private final int mDisplayId;
@@ -302,16 +301,8 @@ public final class DisplayBrightnessController {
     * Notifies the brightnessSetting to persist the supplied brightness value.
     */
    public void setBrightness(float brightnessValue) {
        setBrightness(brightnessValue, DEFAULT_USER_SERIAL);
    }

    /**
     * Notifies the brightnessSetting to persist the supplied brightness value for a user.
     */
    public void setBrightness(float brightnessValue, int userSerial) {
        // Update the setting, which will eventually call back into DPC to have us actually
        // update the display with the new value.
        mBrightnessSetting.setUserSerial(userSerial);
        mBrightnessSetting.setBrightness(brightnessValue);
        if (mDisplayId == Display.DEFAULT_DISPLAY && mPersistBrightnessNitsForDefaultDisplay) {
            float nits = convertToNits(brightnessValue);
@@ -321,6 +312,14 @@ public final class DisplayBrightnessController {
        }
    }

    /**
     * Notifies the brightnessSetting to persist the supplied brightness value for a user.
     */
    public void setBrightness(float brightnessValue, int userSerial) {
        mBrightnessSetting.setUserSerial(userSerial);
        setBrightness(brightnessValue);
    }

    /**
     * Sets the current screen brightness, and notifies the BrightnessSetting about the change.
     */
+1 −4
Original line number Diff line number Diff line
@@ -203,14 +203,11 @@ public class AutomaticBrightnessStrategy {
     * Sets the pending auto-brightness adjustments in the system settings. Executed
     * when there is a change in the brightness system setting, or when there is a user switch.
     */
    public void updatePendingAutoBrightnessAdjustments(boolean userSwitch) {
    public void updatePendingAutoBrightnessAdjustments() {
        final float adj = Settings.System.getFloatForUser(mContext.getContentResolver(),
                Settings.System.SCREEN_AUTO_BRIGHTNESS_ADJ, 0.0f, UserHandle.USER_CURRENT);
        mPendingAutoBrightnessAdjustment = Float.isNaN(adj) ? Float.NaN
                : BrightnessUtils.clampBrightnessAdjustment(adj);
        if (userSwitch) {
            processPendingAutoBrightnessAdjustments();
        }
    }

    /**
Loading