From 41b3b73eceddac289a51b6f70b3ccf4d8d631885 Mon Sep 17 00:00:00 2001
From: Marin Shalamanov <shalamanov@google.com>
Date: Wed, 10 Mar 2021 21:35:39 +0100
Subject: [PATCH] DMD: Also consider no votes when calculating specs

Currently when DisplayModeDirector is calculating desired
display mode specs, we consider different ranges of vote
priorities. If we can't find allowed modes with the broadest
range we increase the lowest priority. However we always
take into account the vote with highes priority. It is
possible that even when we consider only that vote, we
end up with no allowed modes. This currently happens only in
tests when AlwaysRespectAppRequest is ON.

This CL changes the failure behaviour to from throwing ISE
to falling back to the default mode.

Bug: 182387031
Test: atest DisplayModeDirectorTest
Change-Id: I708d6bce0ceddd3c439e66eecd5193f82e272109
---
 .../server/display/DisplayModeDirector.java   | 13 +++++--
 .../display/DisplayModeDirectorTest.java      | 34 +++++++++++++++++++
 2 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/services/core/java/com/android/server/display/DisplayModeDirector.java b/services/core/java/com/android/server/display/DisplayModeDirector.java
index 645ca7ac33e04..4bbd33817d28a 100644
--- a/services/core/java/com/android/server/display/DisplayModeDirector.java
+++ b/services/core/java/com/android/server/display/DisplayModeDirector.java
@@ -263,6 +263,8 @@ public class DisplayModeDirector {
                 highestConsideredPriority = Vote.PRIORITY_APP_REQUEST_SIZE;
             }
 
+            // We try to find a range of priorities which define a non-empty set of allowed display
+            // modes. Each time we fail we increase the lowest priority.
             while (lowestConsideredPriority <= highestConsideredPriority) {
                 summarizeVotes(
                         votes, lowestConsideredPriority, highestConsideredPriority, primarySummary);
@@ -343,8 +345,15 @@ public class DisplayModeDirector {
             }
 
             if (baseModeId == INVALID_DISPLAY_MODE_ID) {
-                throw new IllegalStateException("Can't select a base display mode for display "
-                        + displayId + ". The votes are " + mVotesByDisplay.valueAt(displayId));
+                Slog.w(TAG, "Can't find a set of allowed modes which satisfies the votes. Falling"
+                        + " back to the default mode. Display = " + displayId + ", votes = " + votes
+                        + ", supported modes = " + Arrays.toString(modes));
+
+                float fps = defaultMode.getRefreshRate();
+                return new DesiredDisplayModeSpecs(defaultMode.getModeId(),
+                        /*allowGroupSwitching */ false,
+                        new RefreshRateRange(fps, fps),
+                        new RefreshRateRange(fps, fps));
             }
 
             if (mModeSwitchingType == DisplayManager.SWITCHING_TYPE_NONE) {
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 15ada896512b4..c8099e2467a1d 100644
--- a/services/tests/servicestests/src/com/android/server/display/DisplayModeDirectorTest.java
+++ b/services/tests/servicestests/src/com/android/server/display/DisplayModeDirectorTest.java
@@ -463,6 +463,40 @@ public class DisplayModeDirectorTest {
         assertThat(specs.baseModeId).isEqualTo(defaultModeId);
     }
 
+    @Test
+    public void testStaleAppRequestSize() {
+        DisplayModeDirector director =
+                new DisplayModeDirector(mContext, mHandler, mInjector);
+        Display.Mode[] modes = new Display.Mode[] {
+                new Display.Mode(1, 1280, 720, 60),
+        };
+        Display.Mode defaultMode = modes[0];
+
+        // Inject supported modes
+        SparseArray<Display.Mode[]> supportedModesByDisplay = new SparseArray<>();
+        supportedModesByDisplay.put(DISPLAY_ID, modes);
+        director.injectSupportedModesByDisplay(supportedModesByDisplay);
+
+        // Inject default mode
+        SparseArray<Display.Mode> defaultModesByDisplay = new SparseArray<>();
+        defaultModesByDisplay.put(DISPLAY_ID, defaultMode);
+        director.injectDefaultModeByDisplay(defaultModesByDisplay);
+
+        // Inject votes
+        SparseArray<Vote> votes = new SparseArray<>();
+        votes.put(Vote.PRIORITY_APP_REQUEST_SIZE, Vote.forSize(1920, 1080));
+        votes.put(Vote.PRIORITY_APP_REQUEST_REFRESH_RATE, Vote.forRefreshRates(50, 50));
+        SparseArray<SparseArray<Vote>> votesByDisplay = new SparseArray<>();
+        votesByDisplay.put(DISPLAY_ID, votes);
+        director.injectVotesByDisplay(votesByDisplay);
+
+        director.setShouldAlwaysRespectAppRequestedMode(true);
+
+        // We should return the only available mode
+        DesiredDisplayModeSpecs specs = director.getDesiredDisplayModeSpecs(DISPLAY_ID);
+        assertThat(specs.baseModeId).isEqualTo(defaultMode.getModeId());
+    }
+
     @Test
     public void testBrightnessObserverGetsUpdatedRefreshRatesForZone() {
         DisplayModeDirector director =