commit c2d89d6538f3d79f5f2619bed4f751a12e0437e4
author Dominik Laskowski <domlaskowski@google.com> Fri Jul 12 14:35:00 2024 -0400
committer Android Build Coastguard Worker <android-build-coastguard-worker@google.com> Sat Jul 20 07:04:42 2024 +0000
tree 7a239422d12a4059553b197e3c3765bb45a0abfe
parent e2946321a3fac1f7ae3782397a997accf4e21baf

SF: Remove misleading pacesetter choice fallback

Since Id5cb29c3cbaa8ed455a15d8be3a32e79a470cce5, the pacesetter display
is always specified to Scheduler::promotePacesetterDisplayLocked, so the
fallback to pick the first display as the pacesetter (and std::optional
plumbing) is dead code.

Bug: 329450361
Flag: EXEMPT refactor
Test: presubmit
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:16d1b3ce1588cc2d12ff72957f5807ead487e8e2)
Merged-In: I831de5eef94eaefe0682f9c87ffe76acd50171b0
Change-Id: I831de5eef94eaefe0682f9c87ffe76acd50171b0

services/surfaceflinger/Scheduler/Scheduler.cpp

diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp
index 0ef61b9..9ea3f35 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.cpp
+++ b/services/surfaceflinger/Scheduler/Scheduler.cpp
@@ -117,10 +117,10 @@
     }
 }
 
-void Scheduler::setPacesetterDisplay(std::optional<PhysicalDisplayId> pacesetterIdOpt) {
+void Scheduler::setPacesetterDisplay(PhysicalDisplayId pacesetterId) {
     demotePacesetterDisplay();
 
-    promotePacesetterDisplay(pacesetterIdOpt);
+    promotePacesetterDisplay(pacesetterId);
 }
 
 void Scheduler::registerDisplay(PhysicalDisplayId displayId, RefreshRateSelectorPtr selectorPtr,
@@ -917,22 +917,22 @@
     return mFrameRateOverrideMappings.updateFrameRateOverridesByContent(frameRateOverrides);
 }
 
-void Scheduler::promotePacesetterDisplay(std::optional<PhysicalDisplayId> pacesetterIdOpt) {
+void Scheduler::promotePacesetterDisplay(PhysicalDisplayId pacesetterId) {
     std::shared_ptr<VsyncSchedule> pacesetterVsyncSchedule;
 
     {
         std::scoped_lock lock(mDisplayLock);
-        pacesetterVsyncSchedule = promotePacesetterDisplayLocked(pacesetterIdOpt);
+        pacesetterVsyncSchedule = promotePacesetterDisplayLocked(pacesetterId);
     }
 
     applyNewVsyncSchedule(std::move(pacesetterVsyncSchedule));
 }
 
 std::shared_ptr<VsyncSchedule> Scheduler::promotePacesetterDisplayLocked(
-        std::optional<PhysicalDisplayId> pacesetterIdOpt) {
-    // TODO(b/241286431): Choose the pacesetter display.
-    mPacesetterDisplayId = pacesetterIdOpt.value_or(mDisplays.begin()->first);
-    ALOGI("Display %s is the pacesetter", to_string(*mPacesetterDisplayId).c_str());
+        PhysicalDisplayId pacesetterId) {
+    // TODO: b/241286431 - Choose the pacesetter among mDisplays.
+    mPacesetterDisplayId = pacesetterId;
+    ALOGI("Display %s is the pacesetter", to_string(pacesetterId).c_str());
 
     std::shared_ptr<VsyncSchedule> newVsyncSchedulePtr;
     if (const auto pacesetterOpt = pacesetterDisplayLocked()) {

services/surfaceflinger/Scheduler/Scheduler.h

diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h
index 1a4aa79..4dba6fc 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.h
+++ b/services/surfaceflinger/Scheduler/Scheduler.h
@@ -92,8 +92,8 @@
 
     void startTimers();
 
-    // TODO(b/241285191): Remove this API by promoting pacesetter in onScreen{Acquired,Released}.
-    void setPacesetterDisplay(std::optional<PhysicalDisplayId>) REQUIRES(kMainThreadContext)
+    // TODO: b/241285191 - Remove this API by promoting pacesetter in onScreen{Acquired,Released}.
+    void setPacesetterDisplay(PhysicalDisplayId) REQUIRES(kMainThreadContext)
             EXCLUDES(mDisplayLock);
 
     using RefreshRateSelectorPtr = std::shared_ptr<RefreshRateSelector>;
@@ -377,10 +377,8 @@
     void resyncAllToHardwareVsync(bool allowToEnable) EXCLUDES(mDisplayLock);
     void setVsyncConfig(const VsyncConfig&, Period vsyncPeriod);
 
-    // Chooses a pacesetter among the registered displays, unless `pacesetterIdOpt` is specified.
-    // The new `mPacesetterDisplayId` is never `std::nullopt`.
-    void promotePacesetterDisplay(std::optional<PhysicalDisplayId> pacesetterIdOpt = std::nullopt)
-            REQUIRES(kMainThreadContext) EXCLUDES(mDisplayLock);
+    void promotePacesetterDisplay(PhysicalDisplayId pacesetterId) REQUIRES(kMainThreadContext)
+            EXCLUDES(mDisplayLock);
 
     // Changes to the displays (e.g. registering and unregistering) must be made
     // while mDisplayLock is locked, and the new pacesetter then must be promoted while
@@ -388,8 +386,7 @@
     // MessageQueue and EventThread need to use the new pacesetter's
     // VsyncSchedule, and this must happen while mDisplayLock is *not* locked,
     // or else we may deadlock with EventThread.
-    std::shared_ptr<VsyncSchedule> promotePacesetterDisplayLocked(
-            std::optional<PhysicalDisplayId> pacesetterIdOpt = std::nullopt)
+    std::shared_ptr<VsyncSchedule> promotePacesetterDisplayLocked(PhysicalDisplayId pacesetterId)
             REQUIRES(kMainThreadContext, mDisplayLock);
     void applyNewVsyncSchedule(std::shared_ptr<VsyncSchedule>) EXCLUDES(mDisplayLock);