layer-shell: introduce ack for new outputs

[ifreund]

This ack sequence eliminates the race between a client creating a new layer surface and the compositor rendering the first frame of a new output.

This race is especially important for screenlockers using layer-shell such as swaylock swaywm/swaylock#16

[emersion]

If wonder if it would be desirable to change this to a ping/pong mechanism similar to xdg-shell.

[ifreund]

Clarified that all clients must ack and rebased onto #87 to avoid conflicts.

If wonder if it would be desirable to change this to a ping/pong mechanism similar to xdg-shell.

If I understand correctly, this would just make the ability to use this to achieve atomicity of new outputs implicit instead of explicit. The logic would stay the same from the server's point of view:

1. post new output global
2. send ping to all layer-shells
3. if a new layer surface is requested on the new output before receiving a pong from that layer_shell, wait for a committed buffer before enabling/drawing the first frame of the new output.

This seems like it would work just as well and could be more generally useful than the new output specific language.

[ifreund]

The disadvantage to solving this race implicitly through a ping/pong is that clients which for whatever reason make the pong request before get_layer_surface will still be racy. We could certainly explain how compositors could use ping/pong to solve this in the protocol and encourage clients to respond to the events in order though.

[ifreund]

Opened up #90 to explore the ping/pong alternative approach.

[emersion]

If I understand correctly, this would just make the ability to use this to achieve atomicity of new outputs implicit instead of explicit.

Yeah. That would work similarly to wl_display_roundtrip: the ping-pong roundtrip guarantees that the client has processed all events until the ping request.

This also has more use-cases, like detecting unresponsive layer-shell clients (like xdg-shell).

[ifreund]

Tweaked the descriptions to clarify that the compositor may create several new outputs at once before sending this event. Also bumped the protocol version to 4 as there was a wlroots release with version 3 since this PR was opened.

[ammen99]

Sorry if this is a stupid question, but: what does this last sentence refer to? Aren't next commits taking into account all acked outputs?

[ifreund]

The idea with this paragraph was to parallel the configure/ack behavior of the xdg-shell protocol. However, I think it may be over-complicating things. There are two paths we could take here:

1. State that new layer surfaces created on a new output before acking that new output are associated only with that change of state.
2. In the case that a second new output is created before the first commit on a new layer surface which was created in response to a previous new_output event and the client acks the second new output before committing the layer surface, the layer surface then becomes part of the ack sequence for the second new output.

In it's current state this paragraph is, I believe, trying to explain option 2. I'd suggest that we switch this over to option 1 though as it's simpler and I believe more ideal behavior. Creation of a second new output wouldn't prolong the waiting period for the first frame on a prior new output.

Hopefully that all makes sense, it's rather late here and I don't feel like I've done a terribly good job of putting it into words.

[ammen99]

IMHO 1. makes more sense

[ifreund]

I switched this to option 1, let me know if the wording is unclear.

[ammen99]

To make sure I understand correctly:

1. Server sends new_output
2. Client creates layer-surface but does not commit
3. Client sends ack_output
4. Server waits for client to "map" its surface(s) and then starts rendering

Is this the intended sequence? Also, another question. Why do we choose this design over the following:

1. Server sends new_output
2. Client creates layer-surface
3. Client commits all of its surfaces for the new output
4. Client sends ack_output, at this point compositor just starts operating as usual

It seems to me this second variant will be easier to implement because compositor doesn't have to guess when a client is ready, although I may be missing something.

BTW, wayfire does something similar with a private protocol: https://github.com/WayfireWM/wayfire/blob/master/proto/wayfire-shell-unstable-v2.xml#L47-L64, which is used for ex. for fade-in animation when an output is plugged in or at startup. (Note that this mechanism in wayfire-shell-unstable is racy, I'd switch to the ack_output once it is implemented).

[ifreund]

Is this the intended sequence? Also, another question. Why do we choose this design over the following:

Yes, that's the intended sequence. This was chosen mostly for consistency with other established ack sequences (e.g. in xdg_shell) where the ack sent in response to a configure indicates that the next commit takes the change in state into account.

It seems to me this second variant will be easier to implement because compositor doesn't have to guess when a client is ready, although I may be missing something.

There's no guessing here unless I'm missing something. Only new surfaces created on the new output(s) before the client sends the ack_new_output request are waited on. Each surface is "ready" exactly when it has attached and committed its first buffer, which will then be part of the first frame rendered on the new output.

[ammen99]

Is this the intended sequence? Also, another question. Why do we choose this design over the following:

Yes, that's the intended sequence. This was chosen mostly for consistency with other established ack sequences (e.g. in xdg_shell) where the ack sent in response to a configure indicates that the next commit takes the change in state into account.

It seems to me this second variant will be easier to implement because compositor doesn't have to guess when a client is ready, although I may be missing something.

There's no guessing here unless I'm missing something. Only new surfaces created on the new output(s) before the client sends the ack_new_output request are waited on. Each surface is "ready" exactly when it has attached and committed its first buffer, which will then be part of the first frame rendered on the new output.

Ok, thanks for the explanation.

[emersion]

Overall looks good. Would be nice to see a client & compositor implementation!

[emersion]

LGTM

[emersion]

wlr-protocols has migrated to gitlab.freedesktop.org. This pull request has been moved to:

https://gitlab.freedesktop.org/wlroots/wlr-protocols/-/merge_requests/86