SF: Let DM resize framebuffer on resolution change

The goal of this change is to synchronize resolution switching across SF
and DM. The lack of synchronization causes glitches, which are currently
papered over in HWC by dropping frames whose size don't match the active
resolution.

A mode set involving a resolution switch was finalized by destroying the
DisplayDevice and thus its HWC layers. Remove this special case in favor
of letting DM call SurfaceControl.setDisplaySize right after requesting
the mode set via setDesiredDisplayModeSpecs.

Emit a mode change event to DM as soon as the DisplayModeRequest becomes
the desired mode. In response, DM sends the transaction that resizes the
display via setDisplaySize, so wait until that commit before modesetting
to the new resolution, and resize the framebuffer in that same frame.

Display projection depends on display size, so update the latter first
when committing DisplayDeviceState.

Fixes: 355427258
Flag: com.android.graphics.surfaceflinger.flags.synced_resolution_switch
Test: Internal (caiman) and external displays, with different rotations.
Test: Inner display (comet) with install orientation.
Change-Id: Ifaf300f3b5f907f7cd10b8db2aa6165ad2106530

Author: Dominik Laskowski

libs/gui/include/gui/LayerState.h

@@ -504,12 +504,18 @@ struct DisplayState {
     Rect layerStackSpaceRect = Rect::EMPTY_RECT;
     Rect orientedDisplaySpaceRect = Rect::EMPTY_RECT;
 
-    // Exclusive to virtual displays: The sink surface into which the virtual display is rendered,
-    // and an optional resolution that overrides its default dimensions.
-    sp<IGraphicBufferProducer> surface;
+    // For physical displays, this is the resolution, which must match the active display mode. To
+    // change the resolution, the client must first call SurfaceControl.setDesiredDisplayModeSpecs
+    // with the new DesiredDisplayModeSpecs#defaultMode, then commit the matching width and height.
+    //
+    // For virtual displays, this is an optional resolution that overrides its default dimensions.
+    //
     uint32_t width = 0;
     uint32_t height = 0;
 
+    // For virtual displays, this is the sink surface into which the virtual display is rendered.
+    sp<IGraphicBufferProducer> surface;
+
     status_t write(Parcel& output) const;
     status_t read(const Parcel& input);
 };

services/surfaceflinger/Display/DisplayModeRequest.h

@@ -26,7 +26,8 @@ namespace android::display {
 struct DisplayModeRequest {
     scheduler::FrameRateMode mode;
 
-    // Whether to emit DisplayEventReceiver::DISPLAY_EVENT_MODE_CHANGE.
+    // Whether to emit DisplayEventReceiver::DISPLAY_EVENT_MODE_CHANGE for a change in refresh rate
+    // or render rate. Ignored for resolution changes, which always emit the event.
     bool emitEvent = false;
 
     // Whether to force the request to be applied, even if the mode is unchanged.

services/surfaceflinger/DisplayDevice.cpp

@@ -223,9 +223,7 @@ void DisplayDevice::setFlags(uint32_t flags) {
     mFlags = flags;
 }
 
-void DisplayDevice::setDisplaySize(int width, int height) {
-    LOG_FATAL_IF(!isVirtual(), "Changing the display size is supported only for virtual displays.");
-    const auto size = ui::Size(width, height);
+void DisplayDevice::setDisplaySize(ui::Size size) {
     mCompositionDisplay->setDisplaySize(size);
     if (mRefreshRateOverlay) {
         mRefreshRateOverlay->setViewport(size);

services/surfaceflinger/DisplayDevice.h

@@ -98,7 +98,7 @@ class DisplayDevice : public RefBase {
     ui::Size getSize() const { return {getWidth(), getHeight()}; }
 
     void setLayerFilter(ui::LayerFilter);
-    void setDisplaySize(int width, int height);
+    void setDisplaySize(ui::Size);
     void setProjection(ui::Rotation orientation, Rect viewport, Rect frame);
     void stageBrightness(float brightness) REQUIRES(kMainThreadContext);
     void persistBrightness(bool needsComposite) REQUIRES(kMainThreadContext);

services/surfaceflinger/Scheduler/include/scheduler/FrameRateMode.h

@@ -33,6 +33,10 @@ struct FrameRateMode {
     }
 
     bool operator!=(const FrameRateMode& other) const { return !(*this == other); }
+
+    bool matchesResolution(const FrameRateMode& other) const {
+        return modePtr->getResolution() == other.modePtr->getResolution();
+    }
 };
 
 inline std::string to_string(const FrameRateMode& mode) {

services/surfaceflinger/SurfaceFlinger.cpp

@@ -1366,7 +1366,8 @@ void SurfaceFlinger::setDesiredMode(display::DisplayModeRequest&& desiredMode) {
             const auto selectorPtr = mDisplayModeController.selectorPtrFor(displayId);
             if (!selectorPtr) break;
 
-            const Fps renderRate = selectorPtr->getActiveMode().fps;
+            const auto activeMode = selectorPtr->getActiveMode();
+            const Fps renderRate = activeMode.fps;
 
             // DisplayModeController::setDesiredMode updated the render rate, so inform Scheduler.
             mScheduler->setRenderRate(displayId, renderRate, true /* applyImmediately */);
@@ -1385,6 +1386,15 @@ void SurfaceFlinger::setDesiredMode(display::DisplayModeRequest&& desiredMode) {
 
             mScheduler->updatePhaseConfiguration(displayId, mode.fps);
             mScheduler->setModeChangePending(true);
+
+            // The mode set to switch resolution is not initiated until the display transaction that
+            // resizes the display. DM sends this transaction in response to a mode change event, so
+            // emit the event now, not when finalizing the mode change as for a refresh rate switch.
+            if (FlagManager::getInstance().synced_resolution_switch() &&
+                !mode.matchesResolution(activeMode)) {
+                mScheduler->onDisplayModeChanged(displayId, mode,
+                                                 /*clearContentRequirements*/ true);
+            }
             break;
         }
         case DesiredModeAction::InitiateRenderRateSwitch:
@@ -1463,27 +1473,34 @@ bool SurfaceFlinger::finalizeDisplayModeChange(PhysicalDisplayId displayId) {
     }
 
     const auto& activeMode = pendingModeOpt->mode;
-
-    if (const auto oldResolution =
-                mDisplayModeController.getActiveMode(displayId).modePtr->getResolution();
-        oldResolution != activeMode.modePtr->getResolution()) {
-        auto& state = mCurrentState.displays.editValueFor(getPhysicalDisplayTokenLocked(displayId));
-        // We need to generate new sequenceId in order to recreate the display (and this
-        // way the framebuffer).
-        state.sequenceId = DisplayDeviceState{}.sequenceId;
-        state.physical->activeMode = activeMode.modePtr.get();
-        processDisplayChangesLocked();
-
-        // The DisplayDevice has been destroyed, so abort the commit for the now dead FrameTargeter.
-        return false;
+    const bool resolutionMatch = !FlagManager::getInstance().synced_resolution_switch() ||
+            activeMode.matchesResolution(mDisplayModeController.getActiveMode(displayId));
+
+    if (!FlagManager::getInstance().synced_resolution_switch()) {
+        if (const auto oldResolution =
+                    mDisplayModeController.getActiveMode(displayId).modePtr->getResolution();
+            oldResolution != activeMode.modePtr->getResolution()) {
+            auto& state =
+                    mCurrentState.displays.editValueFor(getPhysicalDisplayTokenLocked(displayId));
+            // We need to generate new sequenceId in order to recreate the display (and this
+            // way the framebuffer).
+            state.sequenceId = DisplayDeviceState{}.sequenceId;
+            state.physical->activeMode = activeMode.modePtr.get();
+            processDisplayChangesLocked();
+
+            // The DisplayDevice has been destroyed, so abort the commit for the now dead
+            // FrameTargeter.
+            return false;
+        }
     }
 
     mDisplayModeController.finalizeModeChange(displayId, activeMode.modePtr->getId(),
                                               activeMode.modePtr->getVsyncRate(), activeMode.fps);
 
     mScheduler->updatePhaseConfiguration(displayId, activeMode.fps);
 
-    if (pendingModeOpt->emitEvent) {
+    // Skip for resolution changes, since the event was already emitted on setting the desired mode.
+    if (resolutionMatch && pendingModeOpt->emitEvent) {
         mScheduler->onDisplayModeChanged(displayId, activeMode, /*clearContentRequirements*/ true);
     }
 
@@ -1535,8 +1552,9 @@ void SurfaceFlinger::initiateDisplayModeChanges() {
               to_string(displayModePtrOpt->get()->getVsyncRate()).c_str(),
               to_string(displayId).c_str());
 
-        if ((!FlagManager::getInstance().connected_display() || !desiredModeOpt->force) &&
-            mDisplayModeController.getActiveMode(displayId) == desiredModeOpt->mode) {
+        const auto activeMode = mDisplayModeController.getActiveMode(displayId);
+
+        if (!desiredModeOpt->force && desiredModeOpt->mode == activeMode) {
             applyActiveMode(displayId);
             continue;
         }
@@ -1557,6 +1575,15 @@ void SurfaceFlinger::initiateDisplayModeChanges() {
         constraints.seamlessRequired = false;
         hal::VsyncPeriodChangeTimeline outTimeline;
 
+        // When initiating a resolution change, wait until the commit that resizes the display.
+        if (FlagManager::getInstance().synced_resolution_switch() &&
+            !activeMode.matchesResolution(desiredModeOpt->mode)) {
+            const auto display = getDisplayDeviceLocked(displayId);
+            if (display->getSize() != desiredModeOpt->mode.modePtr->getResolution()) {
+                continue;
+            }
+        }
+
         const auto error =
                 mDisplayModeController.initiateModeChange(displayId, std::move(*desiredModeOpt),
                                                           constraints, outTimeline);
@@ -4096,6 +4123,35 @@ void SurfaceFlinger::processDisplayChanged(const wp<IBinder>& displayToken,
         if (currentState.flags != drawingState.flags) {
             display->setFlags(currentState.flags);
         }
+
+        const auto updateDisplaySize = [&]() {
+            if (currentState.width != drawingState.width ||
+                currentState.height != drawingState.height) {
+                const ui::Size resolution = ui::Size(currentState.width, currentState.height);
+
+                // Resize the framebuffer. For a virtual display, always do so. For a physical
+                // display, only do so if it has a pending modeset for the matching resolution.
+                if (!currentState.physical ||
+                    (FlagManager::getInstance().synced_resolution_switch() &&
+                     mDisplayModeController.getDesiredMode(display->getPhysicalId())
+                             .transform([resolution](const auto& request) {
+                                 return resolution == request.mode.modePtr->getResolution();
+                             })
+                             .value_or(false))) {
+                    display->setDisplaySize(resolution);
+                }
+
+                if (display->getId() == mActiveDisplayId) {
+                    onActiveDisplaySizeChanged(*display);
+                }
+            }
+        };
+
+        if (FlagManager::getInstance().synced_resolution_switch()) {
+            // Update display size first, as display projection below depends on it.
+            updateDisplaySize();
+        }
+
         if ((currentState.orientation != drawingState.orientation) ||
             (currentState.layerStackSpaceRect != drawingState.layerStackSpaceRect) ||
             (currentState.orientedDisplaySpaceRect != drawingState.orientedDisplaySpaceRect)) {
@@ -4107,13 +4163,9 @@ void SurfaceFlinger::processDisplayChanged(const wp<IBinder>& displayToken,
                         ui::Transform::toRotationFlags(display->getOrientation());
             }
         }
-        if (currentState.width != drawingState.width ||
-            currentState.height != drawingState.height) {
-            display->setDisplaySize(currentState.width, currentState.height);
 
-            if (display->getId() == mActiveDisplayId) {
-                onActiveDisplaySizeChanged(*display);
-            }
+        if (!FlagManager::getInstance().synced_resolution_switch()) {
+            updateDisplaySize();
         }
     }
 }

services/surfaceflinger/common/FlagManager.cpp

@@ -171,6 +171,7 @@ void FlagManager::dump(std::string& result) const {
     DUMP_ACONFIG_FLAG(restore_blur_step);
     DUMP_ACONFIG_FLAG(skip_invisible_windows_in_input);
     DUMP_ACONFIG_FLAG(stable_edid_ids);
+    DUMP_ACONFIG_FLAG(synced_resolution_switch);
     DUMP_ACONFIG_FLAG(trace_frame_rate_override);
     DUMP_ACONFIG_FLAG(true_hdr_screenshots);
     DUMP_ACONFIG_FLAG(use_known_refresh_rate_for_fps_consistency);
@@ -295,6 +296,7 @@ FLAG_MANAGER_ACONFIG_FLAG(skip_invisible_windows_in_input, "");
 FLAG_MANAGER_ACONFIG_FLAG(begone_bright_hlg, "debug.sf.begone_bright_hlg");
 FLAG_MANAGER_ACONFIG_FLAG(window_blur_kawase2, "");
 FLAG_MANAGER_ACONFIG_FLAG(reject_dupe_layerstacks, "");
+FLAG_MANAGER_ACONFIG_FLAG(synced_resolution_switch, "");
 
 /// Trunk stable server (R/W) flags ///
 FLAG_MANAGER_ACONFIG_FLAG(refresh_rate_overlay_on_external_display, "")

services/surfaceflinger/common/include/common/FlagManager.h

@@ -107,6 +107,7 @@ class FlagManager {
     bool restore_blur_step() const;
     bool skip_invisible_windows_in_input() const;
     bool stable_edid_ids() const;
+    bool synced_resolution_switch() const;
     bool trace_frame_rate_override() const;
     bool true_hdr_screenshots() const;
     bool use_known_refresh_rate_for_fps_consistency() const;

services/surfaceflinger/surfaceflinger_flags_new.aconfig

@@ -264,6 +264,13 @@ flag {
   is_fixed_read_only: true
 } # stable_edid_ids
 
+flag {
+  name: "synced_resolution_switch"
+  namespace: "core_graphics"
+  description: "Synchronize resolution modeset with framebuffer resizing"
+  bug: "355427258"
+} # synced_resolution_switch
+
 flag {
   name: "true_hdr_screenshots"
   namespace: "core_graphics"