wayland: Refactor constraint handling

According to the xdg-shell spec, xdg-toplevel min/max values are just hints, which compositors are free to ignore, and they often do so in several cases (usually when tiling, maximizing, or entering fullscreen). Even with the hints set, manually constraining the surface dimensions for non-maximized, non-fullscreen surfaces is required for consistent behavior.

In the case of maximized windows, the dimensions as specified in the configure event are non-negotiable and must be respected by the client, as not doing so is a protocol violation.

This also eliminates some redundant calls to set the libdecor resizable state, which can cause unnecessary commits internally.

src/video/wayland/SDL_waylandwindow.c

@@ -133,6 +133,60 @@ static void UnsetDrawSurfaceViewport(SDL_Window *window)
     }
 }
 
+static void SetMinMaxDimensions(SDL_Window *window)
+{
+    SDL_WindowData *wind = window->driverdata;
+    SDL_VideoData *viddata = wind->waylandData;
+    int min_width, min_height, max_width, max_height;
+
+    if (window->flags & SDL_WINDOW_FULLSCREEN) {
+        min_width = 0;
+        min_height = 0;
+        max_width = 0;
+        max_height = 0;
+    } else if (window->flags & SDL_WINDOW_RESIZABLE) {
+        min_width = SDL_max(window->min_w, wind->system_min_required_width);
+        min_height = SDL_max(window->min_h, wind->system_min_required_height);
+        max_width = window->max_w;
+        max_height = window->max_h;
+    } else {
+        min_width = wind->wl_window_width;
+        min_height = wind->wl_window_height;
+        max_width = wind->wl_window_width;
+        max_height = wind->wl_window_height;
+    }
+
+#ifdef HAVE_LIBDECOR_H
+    if (wind->shell_surface_type == WAYLAND_SURFACE_LIBDECOR) {
+        if (!wind->shell_surface.libdecor.initial_configure_seen || wind->shell_surface.libdecor.frame == NULL) {
+            return; /* Can't do anything yet, wait for ShowWindow */
+        }
+        /* No need to change these values if the window is non-resizable,
+         * as libdecor will just overwrite them internally.
+         */
+        if (libdecor_frame_has_capability(wind->shell_surface.libdecor.frame, LIBDECOR_ACTION_RESIZE)) {
+            libdecor_frame_set_min_content_size(wind->shell_surface.libdecor.frame,
+                                                min_width,
+                                                min_height);
+            libdecor_frame_set_max_content_size(wind->shell_surface.libdecor.frame,
+                                                max_width,
+                                                max_height);
+        }
+    } else
+#endif
+        if (wind->shell_surface_type == WAYLAND_SURFACE_XDG_TOPLEVEL && viddata->shell.xdg) {
+        if (wind->shell_surface.xdg.roleobj.toplevel == NULL) {
+            return; /* Can't do anything yet, wait for ShowWindow */
+        }
+        xdg_toplevel_set_min_size(wind->shell_surface.xdg.roleobj.toplevel,
+                                  min_width,
+                                  min_height);
+        xdg_toplevel_set_max_size(wind->shell_surface.xdg.roleobj.toplevel,
+                                  max_width,
+                                  max_height);
+    }
+}
+
 static void ConfigureWindowGeometry(SDL_Window *window)
 {
     SDL_WindowData *data = window->driverdata;
@@ -237,6 +291,11 @@ static void ConfigureWindowGeometry(SDL_Window *window)
         }
     }
 
+    /* Update the min/max dimensions, primarily if the state was changed, and for non-resizable
+     * xdg-toplevel windows where the limits should match the window size.
+     */
+    SetMinMaxDimensions(window);
+
     /* Unconditionally send the window and drawable size, the video core will deduplicate when required. */
     SDL_SendWindowEvent(window, SDL_EVENT_WINDOW_RESIZED, window_width, window_height);
     SDL_SendWindowEvent(window, SDL_EVENT_WINDOW_PIXEL_SIZE_CHANGED, data->drawable_width, data->drawable_height);
@@ -278,122 +337,35 @@ static void CommitLibdecorFrame(SDL_Window *window)
 #endif
 }
 
-static void SetMinMaxDimensions(SDL_Window *window, SDL_bool commit)
-{
-    SDL_WindowData *wind = window->driverdata;
-    SDL_VideoData *viddata = wind->waylandData;
-    int min_width, min_height, max_width, max_height;
-
-    /* Pop-ups don't get to change size */
-    if (wind->shell_surface_type == WAYLAND_SURFACE_XDG_POPUP) {
-        /* ... but we still want to commit, particularly for ShowWindow */
-        if (commit) {
-            wl_surface_commit(wind->surface);
-        }
-        return;
-    }
-
-    if (window->flags & SDL_WINDOW_FULLSCREEN) {
-        min_width = 0;
-        min_height = 0;
-        max_width = 0;
-        max_height = 0;
-    } else if (window->flags & SDL_WINDOW_RESIZABLE) {
-        min_width = window->min_w;
-        min_height = window->min_h;
-        max_width = window->max_w;
-        max_height = window->max_h;
-    } else {
-        min_width = window->windowed.w;
-        min_height = window->windowed.h;
-        max_width = window->windowed.w;
-        max_height = window->windowed.h;
-    }
-
-#ifdef HAVE_LIBDECOR_H
-    if (wind->shell_surface_type == WAYLAND_SURFACE_LIBDECOR) {
-        if (wind->shell_surface.libdecor.frame == NULL) {
-            return; /* Can't do anything yet, wait for ShowWindow */
-        }
-        libdecor_frame_set_min_content_size(wind->shell_surface.libdecor.frame,
-                                            min_width,
-                                            min_height);
-        libdecor_frame_set_max_content_size(wind->shell_surface.libdecor.frame,
-                                            max_width,
-                                            max_height);
-
-        if (commit) {
-            CommitLibdecorFrame(window);
-            wl_surface_commit(wind->surface);
-        }
-    } else
-#endif
-        if (viddata->shell.xdg) {
-        if (wind->shell_surface.xdg.roleobj.toplevel == NULL) {
-            return; /* Can't do anything yet, wait for ShowWindow */
-        }
-        xdg_toplevel_set_min_size(wind->shell_surface.xdg.roleobj.toplevel,
-                                  min_width,
-                                  min_height);
-        xdg_toplevel_set_max_size(wind->shell_surface.xdg.roleobj.toplevel,
-                                  max_width,
-                                  max_height);
-        if (commit) {
-            wl_surface_commit(wind->surface);
-        }
-    }
-}
-
 static void SetFullscreen(SDL_Window *window, struct wl_output *output)
 {
     SDL_WindowData *wind = window->driverdata;
     SDL_VideoData *viddata = wind->waylandData;
 
-    /* The desktop may try to enforce min/max sizes here, so turn them off for
-     * fullscreen and on (if applicable) for windowed
-     */
-    SetMinMaxDimensions(window, SDL_FALSE);
-
 #ifdef HAVE_LIBDECOR_H
     if (wind->shell_surface_type == WAYLAND_SURFACE_LIBDECOR) {
         if (wind->shell_surface.libdecor.frame == NULL) {
             return; /* Can't do anything yet, wait for ShowWindow */
         }
         if (output) {
-            if (!(window->flags & SDL_WINDOW_RESIZABLE)) {
-                /* Ensure that window is resizable before going into fullscreen.
-                 * This triggers a frame commit internally, so a separate one is not necessary.
-                 */
-                libdecor_frame_set_capabilities(wind->shell_surface.libdecor.frame, LIBDECOR_ACTION_RESIZE);
-                wl_surface_commit(wind->surface);
-            } else {
-                CommitLibdecorFrame(window);
-                wl_surface_commit(wind->surface);
-            }
+            Wayland_SetWindowResizable(SDL_GetVideoDevice(), window, SDL_TRUE);
+            wl_surface_commit(wind->surface);
 
             libdecor_frame_set_fullscreen(wind->shell_surface.libdecor.frame, output);
         } else {
             libdecor_frame_unset_fullscreen(wind->shell_surface.libdecor.frame);
-
-            if (!(window->flags & SDL_WINDOW_RESIZABLE)) {
-                /* restore previous RESIZE capability */
-                libdecor_frame_unset_capabilities(wind->shell_surface.libdecor.frame, LIBDECOR_ACTION_RESIZE);
-                wl_surface_commit(wind->surface);
-            } else {
-                CommitLibdecorFrame(window);
-                wl_surface_commit(wind->surface);
-            }
         }
     } else
 #endif
-        if (viddata->shell.xdg) {
+        if (wind->shell_surface_type == WAYLAND_SURFACE_XDG_TOPLEVEL && viddata->shell.xdg) {
         if (wind->shell_surface.xdg.roleobj.toplevel == NULL) {
             return; /* Can't do anything yet, wait for ShowWindow */
         }
 
-        wl_surface_commit(wind->surface);
-
         if (output) {
+            Wayland_SetWindowResizable(SDL_GetVideoDevice(), window, SDL_TRUE);
+            wl_surface_commit(wind->surface);
+
             xdg_toplevel_set_fullscreen(wind->shell_surface.xdg.roleobj.toplevel, output);
         } else {
             xdg_toplevel_unset_fullscreen(wind->shell_surface.xdg.roleobj.toplevel);
@@ -421,8 +393,6 @@ static void UpdateWindowFullscreen(SDL_Window *window, SDL_bool fullscreen)
             wind->in_fullscreen_transition = SDL_TRUE;
             SDL_SetWindowFullscreen(window, SDL_FALSE);
             wind->in_fullscreen_transition = SDL_FALSE;
-
-            SetMinMaxDimensions(window, SDL_FALSE);
         }
     }
 }
@@ -565,7 +535,21 @@ static void handle_configure_xdg_toplevel(void *data,
                 width = wind->floating_width;
                 height = wind->floating_height;
             }
+        } else {
+            /* If we're a fixed-size window, we know our size for sure.
+             * Always assume the configure is wrong.
+             */
+            width = window->windowed.w;
+            height = window->windowed.h;
+        }
 
+        /* The content limits are only a hint, which the compositor is free to ignore,
+         * so apply them manually when appropriate.
+         *
+         * Per the spec, maximized windows must have their exact dimensions respected,
+         * thus they must not be resized, or a protocol violation can occur.
+         */
+        if (!maximized) {
             if (window->max_w > 0) {
                 width = SDL_min(width, window->max_w);
             }
@@ -575,18 +559,6 @@ static void handle_configure_xdg_toplevel(void *data,
                 height = SDL_min(height, window->max_h);
             }
             height = SDL_max(height, window->min_h);
-        } else if (floating) {
-            /* If we're a fixed-size window, we know our size for sure.
-             * Always assume the configure is wrong.
-             */
-            width = window->windowed.w;
-            height = window->windowed.h;
-        }
-
-        /* Store current floating dimensions for restoring */
-        if (floating) {
-            wind->floating_width = width;
-            wind->floating_height = height;
         }
 
         /* Always send a maximized/restore event; if the event is redundant it will
@@ -602,6 +574,12 @@ static void handle_configure_xdg_toplevel(void *data,
                                 maximized ? SDL_EVENT_WINDOW_MAXIMIZED : SDL_EVENT_WINDOW_RESTORED,
                                 0, 0);
         }
+
+        /* Store current floating dimensions for restoring */
+        if (floating) {
+            wind->floating_width = width;
+            wind->floating_height = height;
+        }
     } else {
         /* Unconditionally set the output for exclusive fullscreen windows when entering
          * fullscreen from a compositor event, as where the compositor will actually
@@ -739,10 +717,10 @@ static void OverrideLibdecorLimits(SDL_Window *window)
 {
 #ifdef SDL_VIDEO_DRIVER_WAYLAND_DYNAMIC_LIBDECOR
     if (libdecor_frame_get_min_content_size == NULL) {
-        SetMinMaxDimensions(window, SDL_FALSE);
+        libdecor_frame_set_min_content_size(window->driverdata->shell_surface.libdecor.frame, window->min_w, window->min_h);
     }
 #elif !defined(SDL_HAVE_LIBDECOR_GET_MIN_MAX)
-    SetMinMaxDimensions(window, SDL_FALSE);
+    libdecor_frame_set_min_content_size(window->driverdata->shell_surface.libdecor.frame, window->min_w, window->min_h);
 #endif
 }
 
@@ -776,6 +754,7 @@ static void decoration_frame_configure(struct libdecor_frame *frame,
     enum libdecor_window_state window_state;
     int width, height;
 
+    SDL_bool prev_fullscreen = wind->is_fullscreen;
     SDL_bool focused = SDL_FALSE;
     SDL_bool fullscreen = SDL_FALSE;
     SDL_bool maximized = SDL_FALSE;
@@ -842,38 +821,58 @@ static void decoration_frame_configure(struct libdecor_frame *frame,
             width = 0;
             height = 0;
         }
-    } else if (!(window->flags & SDL_WINDOW_RESIZABLE)) {
-        width = window->windowed.w;
-        height = window->windowed.h;
-
-        OverrideLibdecorLimits(window);
     } else {
-        /*
-         * XXX: libdecor can send bogus content sizes that are +/- the height
-         *      of the title bar when hiding a window or transitioning from
-         *      non-floating to floating state, which distorts the window size.
-         *
-         *      Ignore any size values from libdecor in these scenarios in
-         *      favor of the cached window size.
+        if (!(window->flags & SDL_WINDOW_RESIZABLE)) {
+            width = window->windowed.w;
+            height = window->windowed.h;
+
+            OverrideLibdecorLimits(window);
+        } else {
+            /*
+             * XXX: libdecor can send bogus content sizes that are +/- the height
+             *      of the title bar when hiding a window or transitioning from
+             *      non-floating to floating state, which distorts the window size.
+             *
+             *      Ignore any size values from libdecor in these scenarios in
+             *      favor of the cached window size.
+             *
+             *      https://gitlab.gnome.org/jadahl/libdecor/-/issues/40
+             */
+            const SDL_bool use_cached_size = !maximized && !tiled &&
+                                             ((floating && !wind->floating) ||
+                                              (window->is_hiding || (window->flags & SDL_WINDOW_HIDDEN)));
+
+            /* This will never set 0 for width/height unless the function returns false */
+            if (use_cached_size || !libdecor_configuration_get_content_size(configuration, frame, &width, &height)) {
+                if (floating) {
+                    /* This usually happens when we're being restored from a
+                     * non-floating state, so use the cached floating size here.
+                     */
+                    width = wind->floating_width;
+                    height = wind->floating_height;
+                } else {
+                    width = window->w;
+                    height = window->h;
+                }
+            }
+        }
+
+        /* The content limits are only a hint, which the compositor is free to ignore,
+         * so apply them manually when appropriate.
          *
-         *      https://gitlab.gnome.org/jadahl/libdecor/-/issues/40
+         * Per the spec, maximized windows must have their exact dimensions respected,
+         * thus they must not be resized, or a protocol violation can occur.
          */
-        const SDL_bool use_cached_size = !maximized && !tiled &&
-                                         ((floating && !wind->floating) ||
-                                          (window->is_hiding || (window->flags & SDL_WINDOW_HIDDEN)));
-
-        /* This will never set 0 for width/height unless the function returns false */
-        if (use_cached_size || !libdecor_configuration_get_content_size(configuration, frame, &width, &height)) {
-            if (floating) {
-                /* This usually happens when we're being restored from a
-                 * non-floating state, so use the cached floating size here.
-                 */
-                width = wind->floating_width;
-                height = wind->floating_height;
-            } else {
-                width = window->w;
-                height = window->h;
+        if (!maximized) {
+            if (window->max_w > 0) {
+                width = SDL_min(width, window->max_w);
+            }
+            width = SDL_max(width, window->min_w);
+
+            if (window->max_h > 0) {
+                height = SDL_min(height, window->max_h);
             }
+            height = SDL_max(height, window->min_h);
         }
     }
 
@@ -904,13 +903,16 @@ static void decoration_frame_configure(struct libdecor_frame *frame,
         wind->surface_status = WAYLAND_SURFACE_STATUS_WAITING_FOR_FRAME;
     }
 
-    /* Update the resize capability. Since this will change the capabilities and
-     * commit a new frame state with the last known content dimension, this has
-     * to be called after the new state has been committed and the new content
-     * dimensions were updated.
+    /* Update the resize capability if this config event was the result of the
+     * compositor taking a window out of fullscreen. Since this will change the
+     * capabilities and commit a new frame state with the last known content
+     * dimension, this has to be called after the new state has been committed
+     * and the new content dimensions were updated.
      */
-    Wayland_SetWindowResizable(SDL_GetVideoDevice(), window,
-                               window->flags & SDL_WINDOW_RESIZABLE);
+    if (prev_fullscreen && !wind->is_fullscreen) {
+        Wayland_SetWindowResizable(SDL_GetVideoDevice(), window,
+                                   !!(window->flags & SDL_WINDOW_RESIZABLE));
+    }
 }
 
 static void decoration_frame_close(struct libdecor_frame *frame, void *user_data)
@@ -1366,13 +1368,6 @@ void Wayland_ShowWindow(_THIS, SDL_Window *window)
      */
 #ifdef HAVE_LIBDECOR_H
     if (data->shell_surface_type == WAYLAND_SURFACE_LIBDECOR) {
-        /* ... but don't call it redundantly for libdecor, the decorator
-         * may not interpret a redundant call nicely and cause weird stuff to happen
-         */
-        if (data->shell_surface.libdecor.frame && window->flags & SDL_WINDOW_BORDERLESS) {
-            Wayland_SetWindowBordered(_this, window, SDL_FALSE);
-        }
-
         /* Libdecor plugins can enforce minimum window sizes, so adjust if the initial window size is too small. */
         if (window->windowed.w < data->system_min_required_width ||
             window->windowed.h < data->system_min_required_height) {
@@ -1386,11 +1381,11 @@ void Wayland_ShowWindow(_THIS, SDL_Window *window)
             data->wl_window_height = SDL_max(window->windowed.h, data->system_min_required_height);
             CommitLibdecorFrame(window);
         }
-    } else
-#endif
-    {
-        Wayland_SetWindowBordered(_this, window, !(window->flags & SDL_WINDOW_BORDERLESS));
     }
+#endif
+    Wayland_SetWindowResizable(_this, window, !!(window->flags & SDL_WINDOW_RESIZABLE));
+    Wayland_SetWindowBordered(_this, window, !(window->flags & SDL_WINDOW_BORDERLESS));
+
 
     /* We're finally done putting the window together, raise if possible */
     if (c->activation_manager) {
@@ -1815,16 +1810,21 @@ void Wayland_SetWindowResizable(_THIS, SDL_Window *window, SDL_bool resizable)
         if (wind->shell_surface.libdecor.frame == NULL) {
             return; /* Can't do anything yet, wait for ShowWindow */
         }
-        if (resizable) {
+        if (libdecor_frame_has_capability(wind->shell_surface.libdecor.frame, LIBDECOR_ACTION_RESIZE)) {
+            if (!resizable) {
+                libdecor_frame_unset_capabilities(wind->shell_surface.libdecor.frame, LIBDECOR_ACTION_RESIZE);
+            }
+        } else if (resizable) {
             libdecor_frame_set_capabilities(wind->shell_surface.libdecor.frame, LIBDECOR_ACTION_RESIZE);
-        } else {
-            libdecor_frame_unset_capabilities(wind->shell_surface.libdecor.frame, LIBDECOR_ACTION_RESIZE);
         }
-    } else
-#endif
-    {
-        SetMinMaxDimensions(window, SDL_TRUE);
     }
+#endif
+
+    /* When changing the resize capability on libdecor windows, the limits must always
+     * be reapplied, as when libdecor changes states, it overwrites the values internally.
+     */
+    SetMinMaxDimensions(window);
+    CommitLibdecorFrame(window);
 }
 
 void Wayland_MaximizeWindow(_THIS, SDL_Window *window)
@@ -2078,12 +2078,14 @@ int Wayland_CreateWindow(_THIS, SDL_Window *window)
 
 void Wayland_SetWindowMinimumSize(_THIS, SDL_Window *window)
 {
-    SetMinMaxDimensions(window, SDL_TRUE);
+    /* Will be committed when Wayland_SetWindowSize() is called by the video core. */
+    SetMinMaxDimensions(window);
 }
 
 void Wayland_SetWindowMaximumSize(_THIS, SDL_Window *window)
 {
-    SetMinMaxDimensions(window, SDL_TRUE);
+    /* Will be committed when Wayland_SetWindowSize() is called by the video core. */
+    SetMinMaxDimensions(window);
 }
 
 void Wayland_SetWindowPosition(_THIS, SDL_Window *window)
@@ -2115,14 +2117,18 @@ void Wayland_SetWindowSize(_THIS, SDL_Window *window)
     wind->floating_width = window->windowed.w;
     wind->floating_height = window->windowed.h;
 
-    /* Don't change the size of static (non-floating) windows. */
-    if (wind->floating) {
+    /* Fullscreen windows do not get explicitly resized, and not strictly
+     * obeying the size of maximized windows is a protocol violation.
+     */
+    if (!(window->flags & (SDL_WINDOW_FULLSCREEN | SDL_WINDOW_MAXIMIZED))) {
         wind->requested_window_width = window->windowed.w;
         wind->requested_window_height = window->windowed.h;
 
         ConfigureWindowGeometry(window);
-        CommitLibdecorFrame(window);
     }
+
+    /* Always commit, as this may be in response to a min/max limit change. */
+    CommitLibdecorFrame(window);
 }
 
 void Wayland_GetWindowSizeInPixels(_THIS, SDL_Window *window, int *w, int *h)