GitLab

Submitted by Daniel Stone @daniels

Assigned to Daniel Stone @daniels

Link to original bug (#7621)

Description

The switch_mode hook, and its implementations in the backends, are pretty badly misconceived.

• Checking for output/mode being NULL are insane; just make them asserts
• DRM drops all its buffers on the floor, meaning we can potentially release buffers too early; the new state is not actually applied until we hit a repaint cycle
• DRM again: if the new creation fails, we are stuck in no mans' land
• DRM again again: drm_output_release_fb has no conception of its surfaces being able to change, meaning we can do things like call gbm_surface_release_buffer with a gbm_bo from a different surface, or release the wrong Pixman buffer
• DRM again again again: it's much more likely that drmModeSetCrtc will fail than creating a GBM/Pixman surface, yet as the state is not actually applied until the next repaint cycle, we'll just fail silently with repaint being dead, and not feed back to the client
• The Wayland backend synchronously throws itself through a repaint cycle in order to provide a success/failure (good!), but the view is moved immediately after, meaning that we go through a second repaint cycle with potentially different content
• The RDP backend is the same as the Wayland backend above
• The Wayland backend seems like it can get stuck in an inconsistent state: if zwp_fullscreen_shell_v1_present_surface_for_mode but recreating the backend fails, we do not appear to have anything to handle undoing the present_surface_for_mode.

It would be far, far better if this were atomic, i.e. if we construct a coherent view list and just fly through the normal repaint handler, with the backends responsible for checking for mode changes. fullscreen-shell could then use the success/fail status of the repaint hook to determine whether or not the changes were OK, or if it needs to roll back. The backends should then be fixed to behave sensibly.