Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

First xdg-shell configure event is invalid #2176

Open
emersion opened this issue Jun 30, 2018 · 9 comments · May be fixed by #8314
Open

First xdg-shell configure event is invalid #2176

emersion opened this issue Jun 30, 2018 · 9 comments · May be fixed by #8314
Labels
bug Not working as intended

Comments

@emersion
Copy link
Member

When a client creates a new xdg-surface and commits a NULL buffer, we're supposed to send a configure event with the size of the window. We currently send it with a 0x0 size (which lets the client decides). We only send the first "real" configure event when the client maps (ie. after it has drawn its first frame).

We should send the correct configure event from the beginning.

@emersion emersion added the bug Not working as intended label Jun 30, 2018
@RyanDwyer
Copy link
Member

My understanding is that the view can create itself but choose not to map immediately. During this delay the user might focus another container, in which case the size it needs could be different. So if we did try to preempt the size when it's created then we'd have to arrange it again when it sends the map request. The positive would be that when it maps, the surface will probably already be the correct size so the transaction would have one less view to wait on. This isn't a huge positive considering it still has to wait for the siblings to resize.

It would be fairly complicated to determine what size the view should be without actually arranging its siblings. You'd need a function similar to arrange_children_of. And to further complicate it, you don't know what size it should be until you know whether it'll have borders, will be placed on a different workspace due to criteria, or will be floating.

In my opinion the negatives greatly outweigh the positives. It's a lot of complicated code for something that the user is never going to see.

It sounds to me like the recommendation is intended for floating window managers.

@emersion
Copy link
Member Author

My understanding is that the view can create itself but choose not to map immediately.

I don't think so.

@RyanDwyer
Copy link
Member

It must. After we send the configure, the client takes time to create the buffer, then commits or requests to be mapped when it's done. This could take 0ms, or it could take 10 seconds. It would be stupid for the compositor to block the event queue while waiting for the client to map, which means the user might change focus between these operations.

@emersion
Copy link
Member Author

It must. After we send the configure, the client takes time to create the buffer, then commits or requests to be mapped when it's done.

It's no different than a normal resize operation. If we block when resizing, we can block when mapping.

@ddevault
Copy link
Contributor

We can't let clients slowloris the compositor into a standstill.

@ddevault
Copy link
Contributor

Also, letting the client choose its initial size itself lets us stow that away and restore it when we float the client later.

@emersion
Copy link
Member Author

We can't let clients slowloris the compositor into a standstill.

I don't understand.

Also, letting the client choose its initial size itself lets us stow that away and restore it when we float the client later.

The official way to do this is to send a 0x0 size.

@emersion
Copy link
Member Author

To fix this, we should create a transaction when the client sends the initial NULL commit. We'd send a configure event with the right size even if the toplevel is currently unmapped. The transaction completes when the client sends an ack_configure and commits a buffer.

@anarcat
Copy link

anarcat commented Dec 18, 2023

Just wanting to add that gstreamer has a good reproducer for this:

gst-launch-1.0 videotestsrc ! waylandsink  fullscreen=TRUE

That crashes completely here, which is probably suboptimal from gstreamer, but that's another story:

ERROR: from element /GstPipeline:pipeline0/GstWaylandSink:waylandsink0: Window has no size set

Here's the full log:

anarcat@angela:~$ WAYLAND_DEBUG=1 gst-launch-1.0 -v videotestsrc ! waylandsink  fullscreen=TRUE

(gst-launch-1.0:1786375): GStreamer-Wayland-CRITICAL **: 15:57:26.538: gst_wl_window_ensure_fullscreen: assertion 'self' failed
Setting pipeline to PAUSED ...
[4283479.379]  -> [email protected]_registry(new id wl_registry@2)
[4283479.396]  -> [email protected](new id wl_callback@3)
[4283479.609] [email protected]_id(3)
[4283479.620] [email protected](1, "wl_shm", 1)
[4283479.631]  -> [email protected](1, "wl_shm", 1, new id [unknown]@4)
[4283479.640] [email protected](2, "wl_drm", 2)
[4283479.648] [email protected](3, "zwp_linux_dmabuf_v1", 4)
[4283479.655]  -> [email protected](3, "zwp_linux_dmabuf_v1", 1, new id [unknown]@5)
[4283479.663] [email protected](4, "wl_compositor", 5)
[4283479.669]  -> [email protected](4, "wl_compositor", 4, new id [unknown]@6)
[4283479.676] [email protected](5, "wl_subcompositor", 1)
[4283479.682]  -> [email protected](5, "wl_subcompositor", 1, new id [unknown]@7)
[4283479.688] [email protected](6, "wl_data_device_manager", 3)
[4283479.693] [email protected](7, "zwlr_gamma_control_manager_v1", 1)
[4283479.704] [email protected](8, "zxdg_output_manager_v1", 3)
[4283479.713] [email protected](9, "org_kde_kwin_idle", 1)
[4283479.718] [email protected](10, "ext_idle_notifier_v1", 1)
[4283479.727] [email protected](11, "zwp_idle_inhibit_manager_v1", 1)
[4283479.736] [email protected](12, "zwlr_layer_shell_v1", 4)
[4283479.745] [email protected](13, "xdg_wm_base", 2)
[4283479.754]  -> [email protected](13, "xdg_wm_base", 1, new id [unknown]@8)
[4283479.764] [email protected](14, "zwp_tablet_manager_v2", 1)
[4283479.771] [email protected](15, "org_kde_kwin_server_decoration_manager", 1)
[4283479.780] [email protected](16, "zxdg_decoration_manager_v1", 1)
[4283479.789] [email protected](17, "zwp_relative_pointer_manager_v1", 1)
[4283479.796] [email protected](18, "zwp_pointer_constraints_v1", 1)
[4283479.802] [email protected](19, "wp_presentation", 1)
[4283479.810] [email protected](20, "zwlr_output_manager_v1", 4)
[4283479.818] [email protected](21, "zwlr_output_power_manager_v1", 1)
[4283479.827] [email protected](22, "zwp_input_method_manager_v2", 1)
[4283479.836] [email protected](23, "zwp_text_input_manager_v3", 1)
[4283479.844] [email protected](24, "zwlr_foreign_toplevel_manager_v1", 3)
[4283479.853] [email protected](25, "ext_session_lock_manager_v1", 1)
[4283479.860] [email protected](26, "wp_drm_lease_device_v1", 1)
[4283479.868] [email protected](27, "zwlr_export_dmabuf_manager_v1", 1)
[4283479.878] [email protected](28, "zwlr_screencopy_manager_v1", 3)
[4283479.887] [email protected](29, "zwlr_data_control_manager_v1", 2)
[4283479.893] [email protected](30, "zwp_primary_selection_device_manager_v1", 1)
[4283479.902] [email protected](31, "wp_viewporter", 1)
[4283479.910]  -> [email protected](31, "wp_viewporter", 1, new id [unknown]@9)
[4283479.923] [email protected](32, "wp_single_pixel_buffer_manager_v1", 1)
[4283479.934] [email protected](33, "zxdg_exporter_v1", 1)
[4283479.943] [email protected](34, "zxdg_importer_v1", 1)
[4283479.949] [email protected](35, "zxdg_exporter_v2", 1)
[4283479.958] [email protected](36, "zxdg_importer_v2", 1)
[4283479.966] [email protected](37, "xdg_activation_v1", 1)
[4283479.974] [email protected](38, "zwp_virtual_keyboard_manager_v1", 1)
[4283479.983] [email protected](39, "zwlr_virtual_pointer_manager_v1", 2)
[4283479.992] [email protected](40, "zwlr_input_inhibit_manager_v1", 1)
[4283479.998] [email protected](41, "zwp_keyboard_shortcuts_inhibit_manager_v1", 1)
[4283480.007] [email protected](42, "wl_seat", 8)
[4283480.018] [email protected](43, "zwp_pointer_gestures_v1", 3)
[4283480.027] [email protected](44, "wl_output", 4)
[4283480.036] [email protected](86791)
[4283480.046]  -> [email protected](new id wl_callback@3)
[4283480.301] [email protected]_id(3)
[4283480.311] [email protected](0)
[4283480.318] [email protected](1)
[4283480.326] [email protected](875709016)
[4283480.333] [email protected](875708993)
[4283480.341] [email protected](875710274)
[4283480.349] [email protected](842094674)
[4283480.355] [email protected](842088786)
[4283480.362] [email protected](892426322)
[4283480.370] [email protected](892420434)
[4283480.377] [email protected](909199186)
[4283480.384] [email protected](808665688)
[4283480.391] [email protected](808665665)
[4283480.398] [email protected](1211384408)
[4283480.405] [email protected](1211384385)
[4283480.412] [email protected](942948952)
[4283480.419] [email protected](942948929)
[4283480.426] [email protected](1211384385)
[4283480.434] [email protected](1211384408)
[4283480.449] [email protected](942948929)
[4283480.455] [email protected](942948952)
[4283480.462] [email protected](808669761)
[4283480.469] [email protected](808669784)
[4283480.476] [email protected](808665665)
[4283480.483] [email protected](875713089)
[4283480.491] [email protected](875708993)
[4283480.499] [email protected](875713112)
[4283480.506] [email protected](875709016)
[4283480.514] [email protected](892424769)
[4283480.521] [email protected](909199186)
[4283480.528] [email protected](538982482)
[4283480.535] [email protected](540422482)
[4283480.542] [email protected](943215175)
[4283480.550] [email protected](842224199)
[4283480.558] [email protected](961959257)
[4283480.565] [email protected](825316697)
[4283480.580] [email protected](842093913)
[4283480.588] [email protected](909202777)
[4283480.596] [email protected](875713881)
[4283480.603] [email protected](961893977)
[4283480.614] [email protected](825316953)
[4283480.622] [email protected](842094169)
[4283480.629] [email protected](909203033)
[4283480.636] [email protected](875714137)
[4283480.643] [email protected](842094158)
[4283480.651] [email protected](808530000)
[4283480.658] [email protected](842084432)
[4283480.665] [email protected](909193296)
[4283480.674] [email protected](909203022)
[4283480.681] [email protected](1448433985)
[4283480.688] [email protected](1448434008)
[4283480.696] [email protected](808531033)
[4283480.702] [email protected](842085465)
[4283480.710] [email protected](909194329)
[4283480.717] [email protected](1448695129)
[4283480.725] [email protected](1498831189)
[4283480.735] [email protected](808530521)
[4283480.743] [email protected](842084953)
[4283480.751] [email protected](909193817)
[4283480.759] [email protected](86791)
Pipeline is PREROLLING ...
/GstPipeline:pipeline0/GstVideoTestSrc:videotestsrc0.GstPad:src: caps = video/x-raw, format=(string)BGRA, width=(int)320, height=(int)240, framerate=(fraction)30/1, multiview-mode=(string)mono, pixel-aspect-ratio=(fraction)1/1, interlace-mode=(string)progressive
/GstPipeline:pipeline0/GstWaylandSink:waylandsink0.GstPad:sink: caps = video/x-raw, format=(string)BGRA, width=(int)320, height=(int)240, framerate=(fraction)30/1, multiview-mode=(string)mono, pixel-aspect-ratio=(fraction)1/1, interlace-mode=(string)progressive
[4283484.795]  -> [email protected]_surface(new id wl_surface@3)
[4283484.826]  -> [email protected]_surface(new id wl_surface@10)
[4283484.832]  -> [email protected]_subsurface(new id wl_subsurface@11, wl_surface@10, wl_surface@3)
[4283484.838]  -> [email protected]_desync()
[4283484.843]  -> [email protected]_viewport(new id wp_viewport@12, wl_surface@3)
[4283484.849]  -> [email protected]_viewport(new id wp_viewport@13, wl_surface@10)
[4283484.854]  -> [email protected]_region(new id wl_region@14)
[4283484.858]  -> [email protected]_input_region(wl_region@14)
[4283484.863]  -> [email protected]()
[4283484.868]  -> [email protected]_xdg_surface(new id xdg_surface@15, wl_surface@3)
[4283484.874]  -> [email protected]_toplevel(new id xdg_toplevel@16)
[4283484.882]  -> [email protected]_fullscreen(nil)
[4283484.887]  -> [email protected]()
[4283491.140] [email protected]_id(14)
[4283491.186] [email protected](86793)
[4283491.193]  -> [email protected](86793)
[4283491.204] [email protected](0, 0, array[0])
[4283491.212] [email protected](86792)
[4283491.217]  -> [email protected]_configure(86792)
[4283491.382]  -> [email protected]()
ERROR: from element /GstPipeline:pipeline0/GstWaylandSink:waylandsink0: Window has no size set
Additional debug info:
../ext/wayland/gstwaylandsink.c(926): gst_wayland_sink_show_frame (): /GstPipeline:pipeline0/GstWaylandSink:waylandsink0:
Make sure you set the size after calling set_window_handle
ERROR: pipeline doesn't want to preroll.
ERROR: from element /GstPipeline:pipeline0/GstVideoTestSrc:videotestsrc0: Internal data stream error.
Additional debug info:
../libs/gst/base/gstbasesrc.c(3132): gst_base_src_loop (): /GstPipeline:pipeline0/GstVideoTestSrc:videotestsrc0:
streaming stopped, reason error (-5)
ERROR: pipeline doesn't want to preroll.
Setting pipeline to NULL ...
[4283491.926]  -> [email protected]()
[4283491.950]  -> [email protected]()
[4283491.956]  -> [email protected]()
[4283491.959]  -> [email protected]()
[4283491.963]  -> [email protected]()
[4283491.967]  -> [email protected]()
[4283491.971]  -> [email protected]()
[4283492.196]  -> [email protected]()
[4283492.218]  -> [email protected]()
[4283492.223]  -> [email protected]()
[4283492.228]  -> [email protected]()
Freeing pipeline ...

kennylevinsen added a commit to kennylevinsen/sway that referenced this issue Aug 24, 2024
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.

Introduce a view_premap which goes through part of the mapping dance,
calculating what the container dimensions would be at this time and
sending it as a configure. This is only an estimate and inherently
racey if the user very quickly changes focused workspace between surface
role commit and first attached buffer, but when it works the client has
a chance of getting the first buffer right, when it doesn't it is no
worse than it would have been otherwise.

Fixes: swaywm#2176
kennylevinsen added a commit to kennylevinsen/sway that referenced this issue Aug 25, 2024
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: swaywm#2176
@kennylevinsen kennylevinsen linked a pull request Aug 25, 2024 that will close this issue
kennylevinsen added a commit to kennylevinsen/sway that referenced this issue Aug 25, 2024
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: swaywm#2176
kennylevinsen added a commit to kennylevinsen/sway that referenced this issue Aug 25, 2024
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: swaywm#2176
kennylevinsen added a commit to kennylevinsen/sway that referenced this issue Aug 27, 2024
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: swaywm#2176
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Not working as intended
Development

Successfully merging a pull request may close this issue.

4 participants