wlr-protocols icon indicating copy to clipboard operation
wlr-protocols copied to clipboard

layer-shell: introduce ack for new outputs

Open ifreund opened this issue 5 years ago • 11 comments

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 https://github.com/swaywm/swaylock/issues/16

ifreund avatar Jun 23 '20 17:06 ifreund

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

emersion avatar Jun 30 '20 10:06 emersion

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 avatar Jun 30 '20 11:06 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 avatar Jun 30 '20 11:06 ifreund

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

ifreund avatar Jul 01 '20 18:07 ifreund

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).

emersion avatar Jul 02 '20 10:07 emersion

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.

ifreund avatar Aug 20 '20 19:08 ifreund

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).

ammen99 avatar Aug 21 '20 09:08 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.

ifreund avatar Aug 21 '20 09:08 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.

Ok, thanks for the explanation.

ammen99 avatar Aug 21 '20 10:08 ammen99

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

emersion avatar Aug 25 '20 12:08 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

emersion avatar Nov 01 '21 10:11 emersion