From 2435a8ccfd17e800db7f4c54f044125510eb51b8 Mon Sep 17 00:00:00 2001 From: Kovid Goyal Date: Mon, 31 Oct 2022 21:59:01 +0530 Subject: [PATCH] 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 | 2 ++ glfw/context.c | 3 ++ glfw/wl_platform.h | 2 ++ glfw/wl_window.c | 69 +++++++++++++++++++++++++++++----------------- kitty/glfw.c | 10 +------ 5 files changed, 52 insertions(+), 34 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index d8954492a..92a829f08 100644 --- a/docs/changelog.rst +++ b/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] ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/glfw/context.c b/glfw/context.c index 67e5dd402..44e4d1a2c 100644 --- a/glfw/context.c +++ b/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) diff --git a/glfw/wl_platform.h b/glfw/wl_platform.h index 5749be720..44565a2fb 100644 --- a/glfw/wl_platform.h +++ b/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); diff --git a/glfw/wl_window.c b/glfw/wl_window.c index 0f3de081e..372bf9052 100644 --- a/glfw/wl_window.c +++ b/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"); - - // 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); + commit_window_surface_if_safe(window); 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; - 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); - wl_surface_commit(window->wl.surface); + if (window->wl.waiting_for_swap_to_commit) { + callback(id); + window->wl.frameCallbackData.id = 0; + window->wl.frameCallbackData.callback = NULL; + window->wl.frameCallbackData.current_wl_callback = NULL; + } 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); + } } } diff --git a/kitty/glfw.c b/kitty/glfw.c index 3fa0bc236..0096a54b0 100644 --- a/kitty/glfw.c +++ b/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); } }