Commits to unmapped layer surfaces error after role object has been destroyed

[wmww]

Originally from wmww/gtk4-layer-shell#119. GTK4 Layer Shell (a library I maintain) does some weird but technically legal things with the layer shell protocol due to how it interacts with GTK. In this case it calls:

1. zwlr_layer_surface_v1.destroy()
2. wl_surface.attach(NULL, 0, 0)
3. wl_surface.commit()

Which I believe should be allowed, however Smithay errors the client with "width 0 requested without setting left and right anchors".

Side note: this only happens on the native Rust Wayland stack, the libwayland once silently drops the error (I think because the error is being sent on a dead Wayland object).

Repro

1. Install ghostty (it uses GTK4 Layer Shell)
2. Put keybind = global:cmd+backquote=toggle_quick_terminal in ~/.config/ghostty/config
3. Run cargo run -p anvil --no-default-features --features winit -- --winit
4. Run ghostty inside anvil
5. Press Super+` to bring up the quick terminal (uses layer shell)
6. Press Super+` again
7. Note the the client is killed and main window appears, instead of just the quick terminal closing

Root Cause

I had my Claude investigate and she thinks the the problem is pre_commit_hook getting run after the layer surface object is destroyed. Indeed wmww@6a18fc4 appears to fix the bug, but I'm unconvinced this is the correct solution. Just adding a check seems like a hack, and this approach doesn't seem to apply as cleanly to XDG surface types, which also appear to have the same problem (untested).

[Drakulix]

Which I believe should be allowed, however Smithay errors the client with "width 0 requested without setting left and right anchors".

I'd argue, that this is debatable.

The documentation for wl_surface states:

A surface can have only one role at a time. Initially a wl_surface does not have a role. Once a wl_surface is given a role, it is set permanently for the whole lifetime of the wl_surface object. Giving the current role again is allowed, unless explicitly forbidden by the relevant interface specification.

[...]

Destroying the role object does not remove the role from the wl_surface, but it may stop the wl_surface from "playing the role". For instance, if a wl_subsurface object is destroyed, the wl_surface it was created for will be unmapped and forget its position and z-order. It is allowed to create a wl_subsurface for the same wl_surface again, but it is not allowed to use the wl_surface as a cursor (cursor is a different role than sub-surface, and role switching is not allowed).

So at least it isn't clearly specified, if the surface should still "play the role" of a layer-surface, which probably makes it compositor policy?

If it does still "play the role", the commit is obviously incomplete.

If it doesn't - which admittedly makes more sense - then checking for an active role object inside the pre-commit hook is a somewhat sane solution. Alternatively we could remove the pre-commit hook, when the role object is destroyed and attach a new one, when/if a new role object is created.

I kinda want to raise this issue somewhere, to fix the documentation for this edge-case, but I am not even sure where. I'd say the wl_surface core protocol is as specific as it can be while being generic, so probably at wlr-protocols?

[wmww]

Ok, so two separate questions:

1. is a wl_surface that had a layer surface role still logically considered a layer surface after the zwlr_layer_surface_v1 object is deleted?
2. is attaching and committing a null buffer allowed in this state?

Question 2

I believe the answer to 2 is yes regardless of the answer to 1. The layer shell protocol says "Attaching a null buffer to a layer surface unmaps it" (implying that's allowed), and I see no reason committing a null null buffer to a roleless wl_surface would not be allowed.

The exact error ("width 0 requested without setting left and right anchors") sounds like what should be sent when .set_size is called without 0 without the right anchors being set, but in the sequence of events GTK4 Layer Shell never calls .set_size. This indicates to me something is screwy with Smithay's layer surface state.

Question 1

I think a surface should clearly be (and is in all known implementations) unmapped when its layer surface object is destroyed. If a new layer surface object is created for it that surface should have the same rules as a freshly created layer surface (eg require an initial configure). This indicates to me a surface in this state is not "playing the role", and thus does not have layer surface-specific rules. Conceptually, the state of "which edges is the layer surface anchored to" died with the layer surface object, so it doesn't make sense to send client errors dependent on that state.