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

Commit fb3879f2 authored by Marin Shalamanov's avatar Marin Shalamanov
Browse files

Epsilon comparison in RefreshRateRange constructor.

In DisplayModeDirector there may be votes for refresh rate ranges
which are within EPS of each other but do not intersect. For example
[0, 60] and [60+eps, 60+eps]. This CL adds an epsilon check in the
constructor of RefreshRateRange and a unit test for the described
scenario.

Fix: 148310029
Test: atest DisplayModeDirectorTest
Test: Manual with a device with PEAK_REFRESH_RATE = 60, from an app
setting the  preferred mode to a 60fps mode (which appears as 60+eps
on the device).

Change-Id: I24e225edc0d7e388897e9e30f5bce858ac56cb06
parent 2688fe61
Loading
Loading
Loading
Loading
+10 −4
Original line number Diff line number Diff line
@@ -73,7 +73,7 @@ public class DisplayModeDirector {
    private static final int GLOBAL_ID = -1;

    // The tolerance within which we consider something approximately equals.
    private static final float EPSILON = 0.01f;
    private static final float FLOAT_TOLERANCE = 0.01f;

    private final Object mLock = new Object();
    private final Context mContext;
@@ -267,8 +267,8 @@ public class DisplayModeDirector {
            // Some refresh rates are calculated based on frame timings, so they aren't *exactly*
            // equal to expected refresh rate. Given that, we apply a bit of tolerance to this
            // comparison.
            if (refreshRate < (minRefreshRate - EPSILON)
                    || refreshRate > (maxRefreshRate + EPSILON)) {
            if (refreshRate < (minRefreshRate - FLOAT_TOLERANCE)
                    || refreshRate > (maxRefreshRate + FLOAT_TOLERANCE)) {
                if (DEBUG) {
                    Slog.w(TAG, "Discarding mode " + mode.getModeId()
                            + ", outside refresh rate bounds"
@@ -487,12 +487,18 @@ public class DisplayModeDirector {
        public RefreshRateRange() {}

        public RefreshRateRange(float min, float max) {
            if (min < 0 || max < 0 || min > max) {
            if (min < 0 || max < 0 || min > max + FLOAT_TOLERANCE) {
                Slog.e(TAG, "Wrong values for min and max when initializing RefreshRateRange : "
                        + min + " " + max);
                this.min = this.max = 0;
                return;
            }
            if (min > max) {
                // Min and max are within epsilon of each other, but in the wrong order.
                float t = min;
                min = max;
                max = t;
            }
            this.min = min;
            this.max = max;
        }
+45 −32
Original line number Diff line number Diff line
@@ -29,6 +29,12 @@ import androidx.test.InstrumentationRegistry;
import androidx.test.filters.SmallTest;
import androidx.test.runner.AndroidJUnit4;

import com.android.server.display.DisplayModeDirector.DesiredDisplayModeSpecs;
import com.android.server.display.DisplayModeDirector.RefreshRateRange;
import com.android.server.display.DisplayModeDirector.Vote;

import com.google.common.truth.Truth;

import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -37,6 +43,9 @@ import org.mockito.MockitoAnnotations;
@SmallTest
@RunWith(AndroidJUnit4.class)
public class DisplayModeDirectorTest {
    // The tolerance within which we consider something approximately equals.
    private static final float FLOAT_TOLERANCE = 0.01f;

    private Context mContext;

    @Before
@@ -56,30 +65,22 @@ public class DisplayModeDirectorTest {
            modes[i - minFps] = new Display.Mode(
                    /*modeId=*/i, /*width=*/1000, /*height=*/1000, /*refreshRate=*/i);
        }
        SparseArray<Display.Mode[]> supportedModesByDisplay = new SparseArray<Display.Mode[]>();
        SparseArray<Display.Mode[]> supportedModesByDisplay = new SparseArray<>();
        supportedModesByDisplay.put(displayId, modes);
        director.injectSupportedModesByDisplay(supportedModesByDisplay);
        SparseArray<Display.Mode> defaultModesByDisplay = new SparseArray<Display.Mode>();
        SparseArray<Display.Mode> defaultModesByDisplay = new SparseArray<>();
        defaultModesByDisplay.put(displayId, modes[0]);
        director.injectDefaultModeByDisplay(defaultModesByDisplay);
        return director;
    }

    private int[] intRange(int min, int max) {
        int[] range = new int[max - min + 1];
        for (int i = min; i <= max; i++) {
            range[i - min] = i;
        }
        return range;
    }

    @Test
    public void testDisplayModeVoting() {
        int displayId = 0;

        // With no votes present, DisplayModeDirector should allow any refresh rate.
        assertEquals(new DisplayModeDirector.DesiredDisplayModeSpecs(/*baseModeId=*/60,
                             new DisplayModeDirector.RefreshRateRange(0f, Float.POSITIVE_INFINITY)),
        assertEquals(new DesiredDisplayModeSpecs(/*baseModeId=*/60,
                             new RefreshRateRange(0f, Float.POSITIVE_INFINITY)),
                createDisplayModeDirectorWithDisplayFpsRange(60, 90).getDesiredDisplayModeSpecs(
                        displayId));

@@ -93,20 +94,16 @@ public class DisplayModeDirectorTest {
            int maxFps = 90;
            DisplayModeDirector director = createDisplayModeDirectorWithDisplayFpsRange(60, 90);
            assertTrue(2 * numPriorities < maxFps - minFps + 1);
            SparseArray<DisplayModeDirector.Vote> votes =
                    new SparseArray<DisplayModeDirector.Vote>();
            SparseArray<SparseArray<DisplayModeDirector.Vote>> votesByDisplay =
                    new SparseArray<SparseArray<DisplayModeDirector.Vote>>();
            SparseArray<Vote> votes = new SparseArray<>();
            SparseArray<SparseArray<Vote>> votesByDisplay = new SparseArray<>();
            votesByDisplay.put(displayId, votes);
            for (int i = 0; i < numPriorities; i++) {
                int priority = DisplayModeDirector.Vote.MIN_PRIORITY + i;
                votes.put(
                        priority, DisplayModeDirector.Vote.forRefreshRates(minFps + i, maxFps - i));
                int priority = Vote.MIN_PRIORITY + i;
                votes.put(priority, Vote.forRefreshRates(minFps + i, maxFps - i));
                director.injectVotesByDisplay(votesByDisplay);
                assertEquals(
                        new DisplayModeDirector.DesiredDisplayModeSpecs(
                assertEquals(new DesiredDisplayModeSpecs(
                                /*baseModeId=*/minFps + i,
                                new DisplayModeDirector.RefreshRateRange(minFps + i, maxFps - i)),
                                new RefreshRateRange(minFps + i, maxFps - i)),
                        director.getDesiredDisplayModeSpecs(displayId));
            }
        }
@@ -116,19 +113,35 @@ public class DisplayModeDirectorTest {
        {
            assertTrue(numPriorities >= 2);
            DisplayModeDirector director = createDisplayModeDirectorWithDisplayFpsRange(60, 90);
            SparseArray<DisplayModeDirector.Vote> votes =
                    new SparseArray<DisplayModeDirector.Vote>();
            SparseArray<SparseArray<DisplayModeDirector.Vote>> votesByDisplay =
                    new SparseArray<SparseArray<DisplayModeDirector.Vote>>();
            SparseArray<Vote> votes = new SparseArray<>();
            SparseArray<SparseArray<Vote>> votesByDisplay = new SparseArray<>();
            votesByDisplay.put(displayId, votes);
            votes.put(DisplayModeDirector.Vote.MAX_PRIORITY,
                    DisplayModeDirector.Vote.forRefreshRates(65, 85));
            votes.put(DisplayModeDirector.Vote.MIN_PRIORITY,
                    DisplayModeDirector.Vote.forRefreshRates(70, 80));
            votes.put(Vote.MAX_PRIORITY, Vote.forRefreshRates(65, 85));
            votes.put(Vote.MIN_PRIORITY, Vote.forRefreshRates(70, 80));
            director.injectVotesByDisplay(votesByDisplay);
            assertEquals(new DisplayModeDirector.DesiredDisplayModeSpecs(/*baseModeId=*/70,
                                 new DisplayModeDirector.RefreshRateRange(70, 80)),
            assertEquals(new DesiredDisplayModeSpecs(/*baseModeId=*/70,
                                 new RefreshRateRange(70, 80)),
                    director.getDesiredDisplayModeSpecs(displayId));
        }
    }

    @Test
    public void testVotingWithFloatingPointErrors() {
        int displayId = 0;
        DisplayModeDirector director = createDisplayModeDirectorWithDisplayFpsRange(50, 90);
        SparseArray<Vote> votes = new SparseArray<>();
        SparseArray<SparseArray<Vote>> votesByDisplay = new SparseArray<>();
        votesByDisplay.put(displayId, votes);
        float error = FLOAT_TOLERANCE / 4;
        votes.put(Vote.PRIORITY_USER_SETTING_PEAK_REFRESH_RATE, Vote.forRefreshRates(0, 60));
        votes.put(Vote.PRIORITY_APP_REQUEST_SIZE, Vote.forRefreshRates(60 + error, 60 + error));
        votes.put(Vote.PRIORITY_APP_REQUEST_REFRESH_RATE,
                Vote.forRefreshRates(60 - error, 60 - error));
        director.injectVotesByDisplay(votesByDisplay);
        DesiredDisplayModeSpecs desiredSpecs = director.getDesiredDisplayModeSpecs(displayId);

        Truth.assertThat(desiredSpecs.refreshRateRange.min).isWithin(FLOAT_TOLERANCE).of(60);
        Truth.assertThat(desiredSpecs.refreshRateRange.max).isWithin(FLOAT_TOLERANCE).of(60);
        Truth.assertThat(desiredSpecs.baseModeId).isEqualTo(60);
    }
}