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

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

Update BrightnessSynchronizer Logic

There was an error where if two auto-brightness changes came in quick
succession, the first could be treated as a user-initiated brightness
change.

1. First float change comes in: Xf
2. We write the brightness to the int-setting: Xi
3. Second float change comes in: Yf
4. Notification of the Xi change comes back to us and because X is
   different from Y, we treat it as a user-change.

The result of this is that auto-brightness learned from these erroneous
"user inputs" and both the short and long term model get affected
negatively for the user.  Often manifesting in brightness learning to
be too low.

The quick succession changes can happen in various situations, but
were most prominent on startup before and after a valid lux is read.

The fix is the revamp the data synchronization algorithm we use in
BrightnessSynchronizer to have updates + confirmation callback be
treated as an atomic operation before executing any other updates
that come in.  In the example above, that means we run (1) (2) and (4)
before handling the new update found in (3).

Fixes: 227483176
Test: Manually verify through debug logging.
Test: atest BrightnessSynchronizerTest
Change-Id: I93856fd74e50bcda04e88a7b7e736e02faa76c8e
parent 9568bd96
Loading
Loading
Loading
Loading
+6 −0
Original line number Diff line number Diff line
@@ -51,6 +51,12 @@ filegroup {
    srcs: ["android/tracing/TraceReportParams.aidl"],
}

filegroup {
    name: "framework-internal-display-sources",
    srcs: ["com/android/internal/display/BrightnessSynchronizer.java"],
    visibility: ["//frameworks/base/services/tests/mockingservicestests"],
}

// These are subset of framework-core-sources that are needed by the
// android.test.mock library. The implementation of android.test.mock references
// private members of various components to allow mocking of classes that cannot
+318 −111
Original line number Diff line number Diff line
@@ -26,57 +26,60 @@ import android.os.Handler;
import android.os.Looper;
import android.os.Message;
import android.os.PowerManager;
import android.os.SystemClock;
import android.os.UserHandle;
import android.provider.Settings;
import android.util.MathUtils;
import android.util.Slog;
import android.view.Display;

import com.android.internal.annotations.VisibleForTesting;

import java.io.PrintWriter;

/**
 * BrightnessSynchronizer helps convert between the int (old) system and float
 * (new) system for storing the brightness. It has methods to convert between the two and also
 * observes for when one of the settings is changed and syncs this with the other.
 */
public class BrightnessSynchronizer {

    private static final int MSG_UPDATE_FLOAT = 1;
    private static final int MSG_UPDATE_INT = 2;
    private static final int MSG_UPDATE_BOTH = 3;

    private static final String TAG = "BrightnessSynchronizer";

    private static final boolean DEBUG = false;
    private static final Uri BRIGHTNESS_URI =
            Settings.System.getUriFor(Settings.System.SCREEN_BRIGHTNESS);

    private static final long WAIT_FOR_RESPONSE_MILLIS = 200;

    private static final int MSG_RUN_UPDATE = 1;

    // The tolerance within which we consider brightness values approximately equal to eachother.
    // This value is approximately 1/3 of the smallest possible brightness value.
    public static final float EPSILON = 0.001f;

    private DisplayManager mDisplayManager;
    private static int sBrightnessUpdateCount = 1;

    private final Context mContext;
    private final BrightnessSyncObserver mBrightnessSyncObserver;
    private final Clock mClock;
    private final Handler mHandler;

    private final Handler mHandler = new Handler(Looper.getMainLooper()) {
        @Override
        public void handleMessage(Message msg) {
            switch (msg.what) {
                case MSG_UPDATE_FLOAT:
                    updateBrightnessFloatFromInt(msg.arg1);
                    break;
                case MSG_UPDATE_INT:
                    updateBrightnessIntFromFloat(Float.intBitsToFloat(msg.arg1));
                    break;
                case MSG_UPDATE_BOTH:
                    updateBoth(Float.intBitsToFloat(msg.arg1));
                    break;
                default:
                    super.handleMessage(msg);
            }
    private DisplayManager mDisplayManager;
    private int mLatestIntBrightness;
    private float mLatestFloatBrightness;
    private BrightnessUpdate mCurrentUpdate;
    private BrightnessUpdate mPendingUpdate;

    public BrightnessSynchronizer(Context context) {
        this(context, Looper.getMainLooper(), SystemClock::uptimeMillis);
    }
    };

    private float mPreferredSettingValue;

    public BrightnessSynchronizer(Context context) {
    @VisibleForTesting
    public BrightnessSynchronizer(Context context, Looper looper, Clock clock) {
        mContext = context;
        mClock = clock;
        mBrightnessSyncObserver = new BrightnessSyncObserver();
        mHandler = new BrightnessSynchronizerHandler(looper);
    }

    /**
@@ -90,24 +93,41 @@ public class BrightnessSynchronizer {
        if (mDisplayManager == null) {
            mDisplayManager = mContext.getSystemService(DisplayManager.class);
        }

        final BrightnessSyncObserver brightnessSyncObserver;
        brightnessSyncObserver = new BrightnessSyncObserver();
        brightnessSyncObserver.startObserving();

        final float currentFloatBrightness = getScreenBrightnessFloat();
        final int currentIntBrightness = getScreenBrightnessInt(mContext);

        if (!Float.isNaN(currentFloatBrightness)) {
            updateBrightnessIntFromFloat(currentFloatBrightness);
        } else if (currentIntBrightness != -1) {
            updateBrightnessFloatFromInt(currentIntBrightness);
        if (mBrightnessSyncObserver.isObserving()) {
            Slog.wtf(TAG, "Brightness sync observer requesting synchronization a second time.");
            return;
        }
        mLatestFloatBrightness = getScreenBrightnessFloat();
        mLatestIntBrightness = getScreenBrightnessInt();
        Slog.i(TAG, "Initial brightness readings: " + mLatestIntBrightness + "(int), "
                + mLatestFloatBrightness + "(float)");

        if (!Float.isNaN(mLatestFloatBrightness)) {
            mPendingUpdate = new BrightnessUpdate(BrightnessUpdate.TYPE_FLOAT,
                    mLatestFloatBrightness);
        } else if (mLatestIntBrightness != PowerManager.BRIGHTNESS_INVALID) {
            mPendingUpdate = new BrightnessUpdate(BrightnessUpdate.TYPE_INT,
                    mLatestIntBrightness);
        } else {
            final float defaultBrightness = mContext.getResources().getFloat(
                    com.android.internal.R.dimen.config_screenBrightnessSettingDefaultFloat);
            mDisplayManager.setBrightness(Display.DEFAULT_DISPLAY, defaultBrightness);
            mPendingUpdate = new BrightnessUpdate(BrightnessUpdate.TYPE_FLOAT, defaultBrightness);
            Slog.i(TAG, "Setting initial brightness to default value of: " + defaultBrightness);
        }

        mBrightnessSyncObserver.startObserving();
        mHandler.sendEmptyMessageAtTime(MSG_RUN_UPDATE, mClock.uptimeMillis());
    }

    /**
     * Prints data on dumpsys.
     */
    public void dump(PrintWriter pw) {
        pw.println("BrightnessSynchronizer");
        pw.println("  mLatestIntBrightness=" + mLatestIntBrightness);
        pw.println("  mLatestFloatBrightness=" + mLatestFloatBrightness);
        pw.println("  mCurrentUpdate=" + mCurrentUpdate);
        pw.println("  mPendingUpdate=" + mPendingUpdate);
    }

    /**
@@ -155,83 +175,93 @@ public class BrightnessSynchronizer {
    }

    /**
     * Gets the stored screen brightness float value from the display brightness setting.
     * @return brightness
     * Consumes a brightness change event for the float-based brightness.
     *
     * @param brightness Float brightness.
     */
    private float getScreenBrightnessFloat() {
        return mDisplayManager.getBrightness(Display.DEFAULT_DISPLAY);
    private void handleBrightnessChangeFloat(float brightness) {
        mLatestFloatBrightness = brightness;
        handleBrightnessChange(BrightnessUpdate.TYPE_FLOAT, brightness);
    }

    /**
     * Gets the stored screen brightness int from the system settings.
     * @param context for accessing settings
     * Consumes a brightness change event for the int-based brightness.
     *
     * @return brightness
     * @param brightness Int brightness.
     */
    private static int getScreenBrightnessInt(Context context) {
        return Settings.System.getIntForUser(context.getContentResolver(),
                Settings.System.SCREEN_BRIGHTNESS, PowerManager.BRIGHTNESS_INVALID,
                UserHandle.USER_CURRENT);
    private void handleBrightnessChangeInt(int brightness) {
        mLatestIntBrightness = brightness;
        handleBrightnessChange(BrightnessUpdate.TYPE_INT, brightness);
    }

    /**
     * Updates the settings based on a passed in int value. This is called whenever the int
     * setting changes. mPreferredSettingValue holds the most recently updated brightness value
     * as a float that we would like the display to be set to.
     *
     * We then schedule an update to both the int and float settings, but, remove all the other
     * messages to update all, to prevent us getting stuck in a loop.
     * Consumes a brightness change event.
     *
     * @param value Brightness value as int to store in the float setting.
     * @param type Type of the brightness change (int/float)
     * @param brightness brightness.
     */
    private void updateBrightnessFloatFromInt(int value) {
        if (brightnessFloatToInt(mPreferredSettingValue) == value) {
            return;
    private void handleBrightnessChange(int type, float brightness) {
        boolean swallowUpdate = mCurrentUpdate != null
                && mCurrentUpdate.swallowUpdate(type, brightness);
        BrightnessUpdate prevUpdate = null;
        if (!swallowUpdate) {
            prevUpdate = mPendingUpdate;
            mPendingUpdate = new BrightnessUpdate(type, brightness);
        }
        runUpdate();

        mPreferredSettingValue = brightnessIntToFloat(value);
        final int newBrightnessAsIntBits = Float.floatToIntBits(mPreferredSettingValue);
        mHandler.removeMessages(MSG_UPDATE_BOTH);
        mHandler.obtainMessage(MSG_UPDATE_BOTH, newBrightnessAsIntBits, 0).sendToTarget();
        // If we created a new update and it is still pending after the update, add a log.
        if (!swallowUpdate && mPendingUpdate != null) {
            Slog.i(TAG, "New PendingUpdate: " + mPendingUpdate + ", prev=" + prevUpdate);
        }
    }

    /**
     * Updates the settings based on a passed in float value. This is called whenever the float
     * setting changes. mPreferredSettingValue holds the most recently updated brightness value
     * as a float that we would like the display to be set to.
     *
     * We then schedule an update to both the int and float settings, but, remove all the other
     * messages to update all, to prevent us getting stuck in a loop.
     *
     * @param value Brightness setting as float to store in int setting.
     * Runs updates for current and pending BrightnessUpdates.
     */
    private void updateBrightnessIntFromFloat(float value) {
        if (floatEquals(mPreferredSettingValue, value)) {
            return;
    private void runUpdate() {
        if (DEBUG) {
            Slog.d(TAG, "Running update mCurrent="  + mCurrentUpdate
                    + ", mPending=" + mPendingUpdate);
        }

        mPreferredSettingValue = value;
        final int newBrightnessAsIntBits = Float.floatToIntBits(mPreferredSettingValue);
        mHandler.removeMessages(MSG_UPDATE_BOTH);
        mHandler.obtainMessage(MSG_UPDATE_BOTH, newBrightnessAsIntBits, 0).sendToTarget();
        // do-while instead of while to allow mCurrentUpdate to get set if there's a pending update.
        do {
            if (mCurrentUpdate != null) {
                mCurrentUpdate.update();
                if (mCurrentUpdate.isRunning()) {
                    break; // current update is still running, nothing to do.
                } else if (mCurrentUpdate.isCompleted()) {
                    if (mCurrentUpdate.madeUpdates()) {
                        Slog.i(TAG, "Completed Update: " + mCurrentUpdate);
                    }
                    mCurrentUpdate = null;
                }
            }
            // No current update any more, lets start the next update if there is one.
            if (mCurrentUpdate == null && mPendingUpdate != null) {
                mCurrentUpdate = mPendingUpdate;
                mPendingUpdate = null;
            }
        } while (mCurrentUpdate != null);
    }


    /**
     * Updates both setting values if they have changed
     * mDisplayManager.setBrightness automatically checks for changes
     * Settings.System.putIntForUser needs to be checked, to prevent an extra callback to this class
     *
     * @param newBrightnessFloat Brightness setting as float to store in both settings
     * Gets the stored screen brightness float value from the display brightness setting.
     * @return brightness
     */
    private void updateBoth(float newBrightnessFloat) {
        int newBrightnessInt = brightnessFloatToInt(newBrightnessFloat);
        mDisplayManager.setBrightness(Display.DEFAULT_DISPLAY, newBrightnessFloat);
        if (getScreenBrightnessInt(mContext) != newBrightnessInt) {
            Settings.System.putIntForUser(mContext.getContentResolver(),
                    Settings.System.SCREEN_BRIGHTNESS, newBrightnessInt, UserHandle.USER_CURRENT);
    private float getScreenBrightnessFloat() {
        return mDisplayManager.getBrightness(Display.DEFAULT_DISPLAY);
    }

    /**
     * Gets the stored screen brightness int from the system settings.
     * @return brightness
     */
    private int getScreenBrightnessInt() {
        return Settings.System.getIntForUser(mContext.getContentResolver(),
                Settings.System.SCREEN_BRIGHTNESS, PowerManager.BRIGHTNESS_INVALID,
                UserHandle.USER_CURRENT);
    }

    /**
@@ -253,7 +283,192 @@ public class BrightnessSynchronizer {
        }
    }

    /**
     * Encapsulates a brightness change event and contains logic for synchronizing the appropriate
     * settings for the specified brightness change.
     */
    @VisibleForTesting
    public class BrightnessUpdate {
        static final int TYPE_INT = 0x1;
        static final int TYPE_FLOAT = 0x2;

        private static final int STATE_NOT_STARTED = 1;
        private static final int STATE_RUNNING = 2;
        private static final int STATE_COMPLETED = 3;

        private final int mSourceType;
        private final float mBrightness;

        private long mTimeUpdated;
        private int mState;
        private int mUpdatedTypes;
        private int mConfirmedTypes;
        private int mId;

        BrightnessUpdate(int sourceType, float brightness) {
            mId = sBrightnessUpdateCount++;
            mSourceType = sourceType;
            mBrightness = brightness;
            mTimeUpdated = 0;
            mUpdatedTypes = 0x0;
            mConfirmedTypes = 0x0;
            mState = STATE_NOT_STARTED;
        }

        @Override
        public String toString() {
            return "{[" + mId + "] " + toStringLabel(mSourceType, mBrightness)
                    + ", mUpdatedTypes=" + mUpdatedTypes + ", mConfirmedTypes=" + mConfirmedTypes
                    + ", mTimeUpdated=" + mTimeUpdated + "}";
        }

        /**
         * Runs the synchronization process, moving forward through the internal state machine.
         */
        void update() {
            if (mState == STATE_NOT_STARTED) {
                mState = STATE_RUNNING;

                // check if we need to update int
                int brightnessInt = getBrightnessAsInt();
                if (mLatestIntBrightness != brightnessInt) {
                    Settings.System.putIntForUser(mContext.getContentResolver(),
                            Settings.System.SCREEN_BRIGHTNESS, brightnessInt,
                            UserHandle.USER_CURRENT);
                    mLatestIntBrightness = brightnessInt;
                    mUpdatedTypes |= TYPE_INT;
                }

                // check if we need to update float
                float brightnessFloat = getBrightnessAsFloat();
                if (!floatEquals(mLatestFloatBrightness, brightnessFloat)) {
                    mDisplayManager.setBrightness(Display.DEFAULT_DISPLAY, brightnessFloat);
                    mLatestFloatBrightness = brightnessFloat;
                    mUpdatedTypes |= TYPE_FLOAT;
                }

                // If we made updates, lets wait for responses.
                if (mUpdatedTypes != 0x0) {
                    // Give some time for our updates to return a confirmation response. If they
                    // don't return by that time, MSG_RUN_UPDATE will get sent and we will stop
                    // listening for responses and mark this update as complete.
                    if (DEBUG) {
                        Slog.d(TAG, "Sending MSG_RUN_UPDATE for "
                                + toStringLabel(mSourceType, mBrightness));
                    }
                    Slog.i(TAG, "[" + mId + "] New Update "
                            + toStringLabel(mSourceType, mBrightness) + " set brightness values: "
                            + toStringLabel(mUpdatedTypes & TYPE_FLOAT, brightnessFloat) + " "
                            + toStringLabel(mUpdatedTypes & TYPE_INT, brightnessInt));

                    mHandler.sendEmptyMessageAtTime(MSG_RUN_UPDATE,
                            mClock.uptimeMillis() + WAIT_FOR_RESPONSE_MILLIS);
                }
                mTimeUpdated = mClock.uptimeMillis();
            }

            if (mState == STATE_RUNNING) {
                // If we're not waiting on any more confirmations or the time has expired, move to
                // completed state.
                if (mConfirmedTypes == mUpdatedTypes
                        || (mTimeUpdated + WAIT_FOR_RESPONSE_MILLIS) < mClock.uptimeMillis()) {
                    mState = STATE_COMPLETED;
                }
            }
        }

        /**
         * Attempts to consume the specified brightness change if it is determined that the change
         * is a notification of a change previously made by this class.
         *
         * @param type The type of change (int|float)
         * @param brightness The brightness value.
         * @return True if the change was caused by this class, thus swallowed.
         */
        boolean swallowUpdate(int type, float brightness) {
            if ((mUpdatedTypes & type) != type || (mConfirmedTypes & type) != 0x0) {
                // It's either a type we didn't update, or one we've already confirmed.
                return false;
            }

            final boolean floatUpdateConfirmed =
                    type == TYPE_FLOAT && floatEquals(getBrightnessAsFloat(), brightness);
            final boolean intUpdateConfirmed =
                    type == TYPE_INT && getBrightnessAsInt() == (int) brightness;

            if (floatUpdateConfirmed || intUpdateConfirmed) {
                mConfirmedTypes |= type;
                Slog.i(TAG, "Swallowing update of " + toStringLabel(type, brightness)
                        + " by update: " + this);
                return true;
            }
            return false;
        }

        boolean isRunning() {
            return mState == STATE_RUNNING;
        }

        boolean isCompleted() {
            return mState == STATE_COMPLETED;
        }

        boolean madeUpdates() {
            return mUpdatedTypes != 0x0;
        }

        private int getBrightnessAsInt() {
            if (mSourceType == TYPE_INT) {
                return (int) mBrightness;
            }
            return brightnessFloatToInt(mBrightness);
        }

        private float getBrightnessAsFloat() {
            if (mSourceType == TYPE_FLOAT) {
                return mBrightness;
            }
            return brightnessIntToFloat((int) mBrightness);
        }

        private String toStringLabel(int type, float brightness) {
            return (type == TYPE_INT) ? ((int) brightness) + "(i)"
                    : ((type == TYPE_FLOAT) ? brightness + "(f)"
                    : "");
        }
    }

    /** Functional interface for providing time. */
    @VisibleForTesting
    public interface Clock {
        /** @return system uptime in milliseconds. */
        long uptimeMillis();
    }

    class BrightnessSynchronizerHandler extends Handler {
        BrightnessSynchronizerHandler(Looper looper) {
            super(looper);
        }

        @Override
        public void handleMessage(Message msg) {
            switch (msg.what) {
                case MSG_RUN_UPDATE:
                    if (DEBUG) {
                        Slog.d(TAG, "MSG_RUN_UPDATE");
                    }
                    runUpdate();
                    break;
                default:
                    super.handleMessage(msg);
            }

        }
    };

    private class BrightnessSyncObserver {
        private boolean mIsObserving;

        private final DisplayListener mListener = new DisplayListener() {
            @Override
            public void onDisplayAdded(int displayId) {}
@@ -263,10 +478,7 @@ public class BrightnessSynchronizer {

            @Override
            public void onDisplayChanged(int displayId) {
                float currentFloat = getScreenBrightnessFloat();
                int toSend = Float.floatToIntBits(currentFloat);
                mHandler.removeMessages(MSG_UPDATE_INT);
                mHandler.obtainMessage(MSG_UPDATE_INT, toSend, 0).sendToTarget();
                handleBrightnessChangeFloat(getScreenBrightnessFloat());
            }
        };

@@ -277,28 +489,23 @@ public class BrightnessSynchronizer {
                    return;
                }
                if (BRIGHTNESS_URI.equals(uri)) {
                    int currentBrightness = getScreenBrightnessInt(mContext);
                    mHandler.removeMessages(MSG_UPDATE_FLOAT);
                    mHandler.obtainMessage(MSG_UPDATE_FLOAT, currentBrightness, 0).sendToTarget();
                    handleBrightnessChangeInt(getScreenBrightnessInt());
                }
            }
        };

        public void startObserving() {
        boolean isObserving() {
            return mIsObserving;
        }

        void startObserving() {
            final ContentResolver cr = mContext.getContentResolver();
            cr.unregisterContentObserver(mContentObserver);
            cr.registerContentObserver(BRIGHTNESS_URI, false, mContentObserver,
                    UserHandle.USER_ALL);

            mDisplayManager.registerDisplayListener(mListener, mHandler,
                    DisplayManager.EVENT_FLAG_DISPLAY_CHANGED
                        | DisplayManager.EVENT_FLAG_DISPLAY_BRIGHTNESS);
                    DisplayManager.EVENT_FLAG_DISPLAY_BRIGHTNESS);
            mIsObserving = true;
        }

        public void stopObserving() {
            final ContentResolver cr = mContext.getContentResolver();
            cr.unregisterContentObserver(mContentObserver);
            mDisplayManager.unregisterDisplayListener(mListener);
        }
    }
}
+1 −0
Original line number Diff line number Diff line
@@ -2503,6 +2503,7 @@ public final class DisplayManagerService extends SystemService {
        }
        pw.println();
        mDisplayModeDirector.dump(pw);
        mBrightnessSynchronizer.dump(pw);
    }

    private static float[] getFloatArray(TypedArray array) {
+1 −0
Original line number Diff line number Diff line
@@ -37,6 +37,7 @@ android_test {
    srcs: [
        "src/**/*.java",
        "src/**/*.kt",
        ":framework-internal-display-sources",
    ],

    static_libs: [