tree/view: Send configure before mapping

[kennylevinsen]

Alternative to #8313, issuing the proper transaction earlier rather than going out of our way to not touch the siblings earlier.

Sending 0,0 as configure dimensions indicate that the client is free to pick its own dimensions. When tiling, the client needs to strictly adhere to the tile dimensions. Sway's handling of this has been to send a the appropriate dimensions in a new configure when the surface is mapped, leading to the first buffer most likely being incorrectly sized.

Move the majority of the mapping logic to view_premap, issued on the initial role commit rather than when mapping the view. This allow the first configure to be driven by a tree transaction with the appropriate geometr, and allows container siblings to start preparing earlier as well, reducing the new window latency.

Fixes: #2176

[kennylevinsen]

Arranging floating views does not currently work as intended, as arranging this early leads to the minimum size being erroneously applied.

One solution would be to only call premap early if the window is not expected to be floating, but that's a bit ugly. Alternatively we could teach container_set_floating, view_autoconfigure et.al. to not touch not-yet-mapped windows, delaying centering till the point of map.

[emersion]

Maybe this should be moved together with the wlr-foreign-toplevel stuff below?

But maybe this should only be set up at map time, not at initial commit time? (Not sure…)

[kennylevinsen]

Ah yeah I joined the foreign-toplevel stuff - not sure if it should be early or late though.

[emersion]

I wonder if we can find a better name? Maybe "setup", "initial setup", "prepare"? (I'd be fine with "premap", just trying to see if there are better alternatives.)

[kennylevinsen]

I named it view_setup for now, seems good enough.

[emersion]

Overall this looks nice and simple :)

[cyanreg]

Thanks for looking into it. This has been my number 1 issue.
Works perfectly. Fixes a lot of oddness that some clients have, particularly Firefox with popup windows.

[kennylevinsen]

Two things:

1. Sorting out the floating situation proved a little annoying, due to arranging early before we know about centering, and due to the minimum size logic (a feature I didn't know we had) wanting to trigger.

For now I have implemented a minor hack: Setup bails for surfaces that will be floating, and is just called again after surface map to deal with those.

2. I have discovered a regression with the GNOME keyring ssh agent:

  -> wl_compositor#5.create_surface(new id wl_surface#43)
  -> xdg_wm_base#27.get_xdg_surface(new id xdg_surface#44, wl_surface#43)
  -> xdg_surface#44.get_toplevel(new id xdg_toplevel#35)
  -> xdg_toplevel#35.set_parent(nil)
  -> xdg_toplevel#35.set_title("Unlock private key")
  -> xdg_toplevel#35.set_app_id("gcr-prompter")
  -> wl_surface#43.commit()
  -> org_kde_kwin_server_decoration_manager#11.create(new id org_kde_kwin_server_decoration#38, wl_surface#43)
  -> org_kde_kwin_server_decoration#38.request_mode(2)
  -> wl_pointer#20.set_cursor(2445, wl_surface#25, 4, 4)
  -> wl_surface#25.attach(wl_buffer#39, 0, 0)
  -> wl_surface#25.set_buffer_scale(1)
  -> wl_surface#25.damage(0, 0, 24, 24)
  -> wl_surface#25.commit()
 xdg_wm_base#27.ping(2502)
  -> xdg_wm_base#27.pong(2502)
 org_kde_kwin_server_decoration#38.mode(2)
 xdg_toplevel#35.wm_capabilities(array[8])
 xdg_toplevel#35.configure(1918, 798, array[16])
 xdg_surface#44.configure(2503)
  -> xdg_surface#44.ack_configure(2503)
  -> wl_shm#4.create_pool(new id wl_shm_pool#36, fd 14, 610144)
  -> wl_shm_pool#36.create_buffer(new id wl_buffer#37, 0, 829, 184, 3316, 0)
  -> wl_surface#43.attach(wl_buffer#37, 0, 0)
  -> wl_surface#43.set_buffer_scale(1)
  -> wl_surface#43.damage(0, 0, 829, 184)
  -> xdg_toplevel#35.set_min_size(829, 184)
  -> xdg_toplevel#35.set_max_size(829, 184)
  -> xdg_surface#44.set_window_geometry(0, 0, 829, 184)
  -> wl_compositor#5.create_region(new id wl_region#40)
  -> wl_region#40.add(0, 0, 829, 184)
  -> wl_surface#43.set_opaque_region(wl_region#40)
  -> wl_region#40.destroy()
  -> wl_surface#43.set_input_region(nil)
  -> wl_surface#43.frame(new id wl_callback#41)
  -> wl_surface#43.commit()

This client only sets min/max equal to enforce floating after the window is mapped, which after this change is too late. As a result, the window becomes tiling rather than floating. Rather annoying.

[kennylevinsen]

The last regression might be a general thing in Gtk, as they do not seem to set either app size nor resizable state when creating the surface: https://gitlab.gnome.org/GNOME/gtk/-/blob/8fb946f6e807f5e49a0c1b8f77b13fbe411dfeb3/gdk/wayland/gdktoplevel-wayland.c#L823

[kennylevinsen]

Random minor update so I don't forget myself: The issue with gtk3's delayed min/max size seems fixable with:

commit f35f547214500242a0f835e1867526c432a481e9 (setminmax-3)
Author: Kenny Levinsen <[email protected]>
Date:   Wed Aug 28 23:11:16 2024 +0200

    wayland: Send cached min/max size on role commit

    The compositor will respond to role commit with a toplevel state and
    preferred size. If the client has any min/max size constraints,
    including being non-resizable, this should be set as early as possible
    so that the compositor can take this into account when planning window
    placement and geometry for the first configure event.

diff --git a/gdk/wayland/gdkwindow-wayland.c b/gdk/wayland/gdkwindow-wayland.c
index 7c457dd675..712949781a 100644
--- a/gdk/wayland/gdkwindow-wayland.c
+++ b/gdk/wayland/gdkwindow-wayland.c
@@ -2219,6 +2219,10 @@ G_GNUC_END_IGNORE_DEPRECATIONS
   if (impl->hint == GDK_WINDOW_TYPE_HINT_DIALOG)
     _gdk_wayland_screen_add_orphan_dialog (window);

+  gdk_window_set_geometry_hints (window,
+                                 &impl->geometry_hints,
+                                 impl->geometry_mask);
+
   wl_surface_commit (impl->display_server.wl_surface);
 }

Trying the same at gtk4 is still WIP as my test client was gtk3 - if we're lucky it will all work out just as easily.

[kennylevinsen]

Wiring the same up for gtk4 is more annoying because a lot of stuff got rewired, but it's not an issue: Both gtk3 and gtk4 support xdg_dialog_v1, so once we get that merged the issue is gone. EDIT: It's totally still an issue because xdg-dialog is always constructed by Gtk. sigh.

(Tested that with https://gitlab.freedesktop.org/wlroots/wlroots/-/merge_requests/4798 and a quick hackjob on the sway side.)

[cyanreg]

I think I found an issue. When doing stabbed/tabbed frames, the tab name for mpv becomes empty. It may be an mpv issue, though I don't think there's anything that can go wrong with the code there.

[cyanreg]

Another issue, certain clients can crash the compositor.
To test, simply try playing this file: https://files.lynne.ee/sway_premap_crash.nut

[kennylevinsen]

Another issue, certain clients can crash the compositor. To test, simply try playing this file: https://files.lynne.ee/sway_premap_crash.nut

Is it wlr_scene_rect_set_size: Assertion width >= 0 && height >= 0' failed.``, with a stack looking like the following? If so, I started seeing this sometime after this patch too, but just reproduced it on master twice:

(gdb) bt
#0  __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at pthread_kill.c:44
#1  0x00007bba09c3b463 in __pthread_kill_internal (threadid=<optimized out>, signo=6) at pthread_kill.c:78
#2  0x00007bba09be2120 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
#3  0x00007bba09bc94c3 in __GI_abort () at abort.c:79
#4  0x00007bba09bc93df in __assert_fail_base (fmt=0x7bba09d59c20 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=assertion@entry=0x6478930fde52 "width >= 0 && height >= 0", file=file@entry=0x6478930f969e "types/scene/wlr_scene.c", line=line@entry=717, function=function@entry=0x64789312d6f0 <__PRETTY_FUNCTION__.11> "wlr_scene_rect_set_size") at assert.c:94
#5  0x00007bba09bda177 in __assert_fail (assertion=assertion@entry=0x6478930fde52 "width >= 0 && height >= 0", file=file@entry=0x6478930f969e "types/scene/wlr_scene.c", line=line@entry=717, function=function@entry=0x64789312d6f0 <__PRETTY_FUNCTION__.11> "wlr_scene_rect_set_size") at assert.c:103
#6  0x00006478930a6656 in wlr_scene_rect_set_size (rect=<optimized out>, width=<optimized out>, height=<optimized out>) at ../subprojects/wlroots/types/scene/wlr_scene.c:717
#7  wlr_scene_rect_set_size (rect=<optimized out>, width=<optimized out>, height=<optimized out>) at ../subprojects/wlroots/types/scene/wlr_scene.c:712
#8  0x000064789302fc73 in arrange_container (con=0x6478b0944800, width=<optimized out>, height=<optimized out>, title_bar=<optimized out>, gaps=<optimized out>) at ../sway/desktop/transaction.c:430
#9  0x000064789303045c in arrange_children (layout=<optimized out>, children=0x6478b0bfc400, active=<optimized out>, content=0x6478b0967b10, width=0, height=<optimized out>, gaps=0) at ../sway/desktop/transaction.c:362
#10 0x000064789302fd9e in arrange_container (con=0x6478b096f3a0, width=0, height=2160, title_bar=<optimized out>, gaps=0) at ../sway/desktop/transaction.c:454
#11 0x0000647893030010 in arrange_children (layout=<optimized out>, children=0x6478b0968030, active=<optimized out>, content=0x6478b06649c0, width=<optimized out>, height=2160, gaps=0) at ../sway/desktop/transaction.c:374
#12 0x0000647893031775 in arrange_workspace_tiling (ws=0x6478b08357d0, width=<optimized out>, height=<optimized out>) at ../sway/desktop/transaction.c:534
#13 arrange_output (output=<optimized out>, width=3840, height=2160) at ../sway/desktop/transaction.c:596
#14 arrange_root (root=<optimized out>) at ../sway/desktop/transaction.c:681
#15 transaction_progress () at ../sway/desktop/transaction.c:739
#16 0x00006478930317e7 in handle_timeout (data=0x6478b075c4d0) at ../sway/desktop/transaction.c:757
#17 0x00007bba09fda3a6 in wl_timer_heap_dispatch (timers=0x6478aff77618) at ../wayland-1.23.0/src/event-loop.c:527
#18 wl_event_loop_dispatch (loop=0x6478aff775c0, timeout=<optimized out>, timeout@entry=-1) at ../wayland-1.23.0/src/event-loop.c:1098
#19 0x00007bba09fdc10f in wl_display_run (display=0x6478aff774d0) at ../wayland-1.23.0/src/wayland-server.c:1530
#20 0x000064789301dd85 in server_run (server=<optimized out>) at ../sway/server.c:507
#21 main (argc=4, argv=0x7fff194780b8) at ../sway/main.c:373