Wayland GNOME: Fix incorrect window size when in some circumstances when switching between windows with window decorations disabled

Only call wl_surface_commit() after a resize when the correct size buffer is
attached to the surface. This is ensured by setting a flag on the window that prevents
all surface commits till it is cleared. The flag is cleared at next
eglSwapBuffers().

I dont actually understand if this guarantees that the
buffer size is always correct. For example, if the back buffer is
latched when wl_egl_resize_window() is called, the backbuffer will be
correct only after two swaps (I think). Or maybe the old back buffer is
discarded, I cant find any documentation about it.

All I can say is that doing it this way seems to fix the issue.

Thanks to @jadahl for his help with tracking down the root cause.

Fixes #4802

docs/changelog.rst

@@ -48,6 +48,8 @@ Detailed list of changes
 
 - Fix cursor position at x=0 changing to x=1 on resize (:iss:`5635`)
 
+- Wayland GNOME: Fix incorrect window size when in some circumstances when switching between windows with window decorations disabled (:iss:`4802`)
+
 0.26.4 [2022-10-17]
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 

glfw/context.c

@@ -478,6 +478,9 @@ GLFWAPI void glfwSwapBuffers(GLFWwindow* handle)
     }
 
     window->context.swapBuffers(window);
+#ifdef _GLFW_WAYLAND
+    _glfwWaylandAfterBufferSwap(window);
+#endif
 }
 
 GLFWAPI void glfwSwapInterval(int interval)

glfw/wl_platform.h

@@ -152,6 +152,7 @@ typedef struct _GLFWwindowWayland
     bool                        hovered;
     bool                        transparent;
     struct wl_surface*          surface;
+    bool                        waiting_for_swap_to_commit;
     struct wl_egl_window*       native;
     struct wl_callback*         callback;
 
@@ -369,6 +370,7 @@ typedef struct _GLFWcursorWayland
 
 
 void _glfwAddOutputWayland(uint32_t name, uint32_t version);
+void _glfwWaylandAfterBufferSwap(_GLFWwindow *window);
 void _glfwSetupWaylandDataDevice(void);
 void _glfwSetupWaylandPrimarySelectionDevice(void);
 void animateCursorImage(id_type timer_id, void *data);

glfw/wl_window.c

@@ -251,6 +251,15 @@ static bool checkScaleChange(_GLFWwindow* window)
     return false;
 }
 
+static void
+commit_window_surface_if_safe(_GLFWwindow *window) {
+    // we only commit if the buffer attached to the surface is the correct size,
+    // which means that at least one frame is drawn after resizeFramebuffer()
+    if (!window->wl.waiting_for_swap_to_commit) {
+        wl_surface_commit(window->wl.surface);
+    }
+}
+
 // Makes the surface considered as XRGB instead of ARGB.
 static void setOpaqueRegion(_GLFWwindow* window, bool commit_surface)
 {
@@ -262,7 +271,7 @@ static void setOpaqueRegion(_GLFWwindow* window, bool commit_surface)
 
     wl_region_add(region, 0, 0, window->wl.width, window->wl.height);
     wl_surface_set_opaque_region(window->wl.surface, region);
-    if (commit_surface) wl_surface_commit(window->wl.surface);
+    if (commit_surface) commit_window_surface_if_safe(window);
     wl_region_destroy(region);
 }
 
@@ -287,9 +296,20 @@ resizeFramebuffer(_GLFWwindow* window) {
     debug("Resizing framebuffer to: %dx%d at scale: %d\n", window->wl.width, window->wl.height, scale);
     wl_egl_window_resize(window->wl.native, scaledWidth, scaledHeight, 0, 0);
     if (!window->wl.transparent) setOpaqueRegion(window, false);
+    window->wl.waiting_for_swap_to_commit = true;
     _glfwInputFramebufferSize(window, scaledWidth, scaledHeight);
 }
 
+void
+_glfwWaylandAfterBufferSwap(_GLFWwindow* window) {
+    if (window->wl.waiting_for_swap_to_commit) {
+        debug("Waiting for swap to commit: swap has happened\n");
+        window->wl.waiting_for_swap_to_commit = false;
+        // this is not really needed, since I think eglSwapBuffers() calls wl_surface_commit()
+        // but lets be safe. See https://gitlab.freedesktop.org/mesa/mesa/-/blob/main/src/egl/drivers/dri2/platform_wayland.c#L1510
+        wl_surface_commit(window->wl.surface);
+    }
+}
 
 static const char*
 clipboard_mime(void) {
@@ -301,9 +321,9 @@ clipboard_mime(void) {
 }
 
 static bool
-dispatchChangesAfterConfigure(_GLFWwindow *window, int32_t width, int32_t height, bool *scale_changed) {
+dispatchChangesAfterConfigure(_GLFWwindow *window, int32_t width, int32_t height) {
     bool size_changed = width != window->wl.width || height != window->wl.height;
-    *scale_changed = checkScaleChange(window);
+    bool scale_changed = checkScaleChange(window);
 
     if (size_changed) {
         _glfwInputWindowSize(window, width, height);
@@ -311,7 +331,7 @@ dispatchChangesAfterConfigure(_GLFWwindow *window, int32_t width, int32_t height
         resizeFramebuffer(window);
     }
 
-    if (*scale_changed) {
+    if (scale_changed) {
         debug("Scale changed to %d in dispatchChangesAfterConfigure\n", window->wl.scale);
         if (!size_changed) resizeFramebuffer(window);
         _glfwInputWindowContentScale(window, window->wl.scale, window->wl.scale);
@@ -319,7 +339,7 @@ dispatchChangesAfterConfigure(_GLFWwindow *window, int32_t width, int32_t height
 
     _glfwInputWindowDamage(window);
 
-    return size_changed || *scale_changed;
+    return size_changed || scale_changed;
 }
 
 static void
@@ -600,11 +620,10 @@ static void xdgSurfaceHandleConfigure(void* data,
     }
 
     bool resized = false;
-    bool scale_changed = false;
     if (window->wl.pending_state) {
         int width = window->wl.pending.width, height = window->wl.pending.height;
         set_csd_window_geometry(window, &width, &height);
-        resized = dispatchChangesAfterConfigure(window, width, height, &scale_changed);
+        resized = dispatchChangesAfterConfigure(window, width, height);
         if (window->wl.decorations.serverSide) {
             free_csd_surfaces(window);
         } else {
@@ -614,14 +633,7 @@ static void xdgSurfaceHandleConfigure(void* data,
     }
 
     inform_compositor_of_window_geometry(window, "configure");
-
+    commit_window_surface_if_safe(window);
-    // we need to swap buffers here to ensure the buffer attached to the surface is a multiple
-    // of the new scale. See https://github.com/kovidgoyal/kitty/issues/5467
-    if (scale_changed) swap_buffers(window);
-
-    // if a resize happened there will be a commit at the next render frame so
-    // dont commit here, GNOME doesnt like it and its not really needed anyway
-    if (!resized) wl_surface_commit(window->wl.surface);
     window->wl.pending_state = 0;
 }
 
@@ -1021,7 +1033,7 @@ void _glfwPlatformSetWindowSize(_GLFWwindow* window, int width, int height)
         window->wl.width = w; window->wl.height = h;
         resizeFramebuffer(window);
         ensure_csd_resources(window);
-        wl_surface_commit(window->wl.surface);
+        commit_window_surface_if_safe(window);
         inform_compositor_of_window_geometry(window, "SetWindowSize");
     }
 }
@@ -1038,7 +1050,7 @@ void _glfwPlatformSetWindowSizeLimits(_GLFWwindow* window,
             maxwidth = maxheight = 0;
         xdg_toplevel_set_min_size(window->wl.xdg.toplevel, minwidth, minheight);
         xdg_toplevel_set_max_size(window->wl.xdg.toplevel, maxwidth, maxheight);
-        wl_surface_commit(window->wl.surface);
+        commit_window_surface_if_safe(window);
     }
 }
 
@@ -1273,7 +1285,7 @@ void _glfwPlatformSetWindowMousePassthrough(_GLFWwindow* window, bool enabled)
     }
     else
         wl_surface_set_input_region(window->wl.surface, 0);
-    wl_surface_commit(window->wl.surface);
+    commit_window_surface_if_safe(window);
 }
 
 float _glfwPlatformGetWindowOpacity(_GLFWwindow* window UNUSED)
@@ -1337,7 +1349,7 @@ void _glfwPlatformSetCursorPos(_GLFWwindow* window, double x, double y)
         zwp_locked_pointer_v1_set_cursor_position_hint(
             window->wl.pointerLock.lockedPointer,
             wl_fixed_from_double(x), wl_fixed_from_double(y));
-        wl_surface_commit(window->wl.surface);
+        commit_window_surface_if_safe(window);
     }
 }
 
@@ -2241,12 +2253,19 @@ GLFWAPI void glfwRequestWaylandFrameEvent(GLFWwindow *handle, unsigned long long
     _GLFWwindow* window = (_GLFWwindow*) handle;
     static const struct wl_callback_listener frame_listener = { .done = frame_handle_redraw };
     if (window->wl.frameCallbackData.current_wl_callback) wl_callback_destroy(window->wl.frameCallbackData.current_wl_callback);
-    window->wl.frameCallbackData.id = id;
+    if (window->wl.waiting_for_swap_to_commit) {
-    window->wl.frameCallbackData.callback = callback;
+        callback(id);
-    window->wl.frameCallbackData.current_wl_callback = wl_surface_frame(window->wl.surface);
+        window->wl.frameCallbackData.id = 0;
-    if (window->wl.frameCallbackData.current_wl_callback) {
+        window->wl.frameCallbackData.callback = NULL;
-        wl_callback_add_listener(window->wl.frameCallbackData.current_wl_callback, &frame_listener, window);
+        window->wl.frameCallbackData.current_wl_callback = NULL;
-        wl_surface_commit(window->wl.surface);
+    } else {
+        window->wl.frameCallbackData.id = id;
+        window->wl.frameCallbackData.callback = callback;
+        window->wl.frameCallbackData.current_wl_callback = wl_surface_frame(window->wl.surface);
+        if (window->wl.frameCallbackData.current_wl_callback) {
+            wl_callback_add_listener(window->wl.frameCallbackData.current_wl_callback, &frame_listener, window);
+            commit_window_surface_if_safe(window);
+        }
     }
 }
 

kitty/glfw.c

@@ -312,14 +312,6 @@ dpi_change_callback(GLFWwindow *w, float x_scale UNUSED, float y_scale UNUSED) {
     window->live_resize.last_resize_event_at = monotonic();
     global_state.callback_os_window = NULL;
     request_tick_callback();
-    if (global_state.is_wayland) {
-        // because of the stupidity of Wayland design, the GLFW wayland backend
-        // will need to swap buffers immediately after a scale change to ensure
-        // the buffer size is a multiple of the scale. So blank the new buffer to
-        // ensure we dont leak any unintialized pixels to the screen. The OpenGL viewport
-        // will already have been resized to its new size in framebuffer_size_callback
-        blank_os_window(window);
-    }
 }
 
 static void
@@ -1539,8 +1531,8 @@ request_frame_render(OSWindow *w) {
     // Some Wayland compositors are too fragile to handle multiple
     // render frame requests, see https://github.com/kovidgoyal/kitty/issues/2329
     if (w->render_state != RENDER_FRAME_REQUESTED) {
-        glfwRequestWaylandFrameEvent(w->handle, w->id, wayland_frame_request_callback);
         w->render_state = RENDER_FRAME_REQUESTED;
+        glfwRequestWaylandFrameEvent(w->handle, w->id, wayland_frame_request_callback);
     }
 }