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

2 files changed, 55 insertions, 36 deletions

diff --git a/services/core/java/com/android/server/display/DisplayModeDirector.java b/services/core/java/com/android/server/display/DisplayModeDirector.java
index 96532f4a21ac..99dca34364f7 100644
--- a/services/core/java/com/android/server/display/DisplayModeDirector.java
+++ b/services/core/java/com/android/server/display/DisplayModeDirector.java
@@ -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;
         }
diff --git a/services/tests/servicestests/src/com/android/server/display/DisplayModeDirectorTest.java b/services/tests/servicestests/src/com/android/server/display/DisplayModeDirectorTest.java
index 25d077823a3f..feae1e173f52 100644
--- a/services/tests/servicestests/src/com/android/server/display/DisplayModeDirectorTest.java
+++ b/services/tests/servicestests/src/com/android/server/display/DisplayModeDirectorTest.java
@@ -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);
+    }
 }