[wayland-protocols,v3] xdg-output: deprecate the xdg_output.done event

Submitted by Simon Ser on July 12, 2019, 3:42 p.m.

Details

Message ID UoXcT5dHB0RYmCXA4ArpqnS0W2RT4r764n4yNs42xyGi-qF5XyOghk7vnxr2wAkz0wpPxRaPwh6b59_opXppEVhQ12IAY3XJ1PYcRNZj5DM=@emersion.fr
State Accepted
Headers show
Series "xdg-output: deprecate the xdg_output.done event" ( rev: 3 ) in Wayland

Not browsing as part of any series.

Commit Message

Simon Ser July 12, 2019, 3:42 p.m.
This commit makes it so a wl_output.done event is guaranteed to be sent with a
xdg_output.done event.

This protocol change has been discussed in a recent xorg-devel discussions [1].

First let's recap why a change is needed: Xwayland listens to both wl_output and
xdg_output changes. When an output's properties change, Xwayland expects to
receive both a wl_output.done event and an xdg_output.done event. If that's not
the case, Xwayland doesn't update its state (so old state is still exposed to
X11 clients).

Most of the time, both objects will be updated at the same time (e.g. the
current mode is changed, so both wl_output.mode and xdg_output.logical_size are
sent) so this won't be an issue. However in some situations only one of
wl_output or xdg_output changes. For instance:

- The mode is changed at the same time as the scale, resulting in the same
  logical_size.
- The compositor doesn't expose the outputs' position via wl_output, so whenever
  the position changes only xdg_output is updated.

Both KDE [2] and wlroots [3] have experienced this issue.

For this reason, I'd like to update the xdg-output protocol to make it mandatory
to always send a wl_output.done event after xdg_output changes. This effectively
makes wl_output.done atomically apply all output state (including the state of
add-on objects like xdg_output). This approach is pretty similar to
wl_surface.commit: this request will atomically apply surface state including
the state of e.g. the xdg_surface object tied to the wl_surface.

To update the protocol to reflect this new requirement we can either:

- **Bump xdg_output version**. The current protocol doesn't specify that
  wl_output.done must be sent, adding this new requirement would be a breaking
  change. We need to fix Xwayland for the current xdg_output version (maybe make
  it non-atomic for the current version, atomic for the new one?). Should we
  deprecate xdg_output.done in the new version?
- **Don't bump xdg_output version**. This clarifies what is expected in practice
  by Xwayland, a major xdg_output consumer, and what is currently implemented by
  all compositors.

There's one issue with the "don't bump" approach: indeed in practice compositors
always send wl_output.done and xdg_output.done in pairs, however the ordering
between those two events is not guaranteed. This means some compositors might
send this sequence:

    wl_output.geometry(…)
    wl_output.done()
    xdg_output.logical_position(…)
    xdg_output.done()

In this case the wl_output.done event fails to atomically apply the xdg_output
state.

For this reason, I think bumping the version is a better approach.

This commit also deprecates xdg_output.done, which doesn't have any purpose
anymore.

[1]: https://lists.x.org/archives/xorg-devel/2019-April/058148.html
[2]: https://phabricator.kde.org/D19253
[3]: https://github.com/swaywm/sway/issues/4064

Signed-off-by: Simon Ser <contact@emersion.fr>
Reviewed-by: Olivier Fourdan <ofourdan@redhat.com>
---

Changes from v2 to v3: in xdg_output's description, mention wl_output.done is
guaranteed to be sent only from version 3.

 unstable/xdg-output/xdg-output-unstable-v1.xml | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

--
2.22.0

Patch hide | download patch | download mbox

diff --git a/unstable/xdg-output/xdg-output-unstable-v1.xml b/unstable/xdg-output/xdg-output-unstable-v1.xml
index ccbfe1c9a955..9efb7a40417b 100644
--- a/unstable/xdg-output/xdg-output-unstable-v1.xml
+++ b/unstable/xdg-output/xdg-output-unstable-v1.xml
@@ -54,7 +54,7 @@ 
     reset.
   </description>

-  <interface name="zxdg_output_manager_v1" version="2">
+  <interface name="zxdg_output_manager_v1" version="3">
     <description summary="manage xdg_output objects">
       A global factory interface for xdg_output objects.
     </description>
@@ -77,12 +77,17 @@ 
     </request>
   </interface>

-  <interface name="zxdg_output_v1" version="2">
+  <interface name="zxdg_output_v1" version="3">
     <description summary="compositor logical output region">
       An xdg_output describes part of the compositor geometry.

       This typically corresponds to a monitor that displays part of the
       compositor space.
+
+      For objects version 3 onwards, after all xdg_output properties have been
+      sent (when the object is created and when properties are updated), a
+      wl_output.done event is sent. This allows changes to the output
+      properties to be seen as atomic, even if they happen via multiple events.
     </description>

     <request name="destroy" type="destructor">
@@ -157,6 +162,10 @@ 

 	This allows changes to the xdg_output properties to be seen as
 	atomic, even if they happen via multiple events.
+
+	For objects version 3 onwards, this event is deprecated. Compositors
+	are not required to send it anymore and must send wl_output.done
+	instead.
       </description>
     </event>


Comments

On Fri, Jul 12, 2019 at 03:42:52PM +0000, Simon Ser wrote:
> This commit makes it so a wl_output.done event is guaranteed to be sent with a
> xdg_output.done event.
> 
> This protocol change has been discussed in a recent xorg-devel discussions [1].
> 
> First let's recap why a change is needed: Xwayland listens to both wl_output and
> xdg_output changes. When an output's properties change, Xwayland expects to
> receive both a wl_output.done event and an xdg_output.done event. If that's not
> the case, Xwayland doesn't update its state (so old state is still exposed to
> X11 clients).
> 
> Most of the time, both objects will be updated at the same time (e.g. the
> current mode is changed, so both wl_output.mode and xdg_output.logical_size are
> sent) so this won't be an issue. However in some situations only one of
> wl_output or xdg_output changes. For instance:
> 
> - The mode is changed at the same time as the scale, resulting in the same
>   logical_size.
> - The compositor doesn't expose the outputs' position via wl_output, so whenever
>   the position changes only xdg_output is updated.
> 
> Both KDE [2] and wlroots [3] have experienced this issue.
> 
> For this reason, I'd like to update the xdg-output protocol to make it mandatory
> to always send a wl_output.done event after xdg_output changes. This effectively
> makes wl_output.done atomically apply all output state (including the state of
> add-on objects like xdg_output). This approach is pretty similar to
> wl_surface.commit: this request will atomically apply surface state including
> the state of e.g. the xdg_surface object tied to the wl_surface.
> 
> To update the protocol to reflect this new requirement we can either:
> 
> - **Bump xdg_output version**. The current protocol doesn't specify that
>   wl_output.done must be sent, adding this new requirement would be a breaking
>   change. We need to fix Xwayland for the current xdg_output version (maybe make
>   it non-atomic for the current version, atomic for the new one?). Should we
>   deprecate xdg_output.done in the new version?
> - **Don't bump xdg_output version**. This clarifies what is expected in practice
>   by Xwayland, a major xdg_output consumer, and what is currently implemented by
>   all compositors.
> 
> There's one issue with the "don't bump" approach: indeed in practice compositors
> always send wl_output.done and xdg_output.done in pairs, however the ordering
> between those two events is not guaranteed. This means some compositors might
> send this sequence:
> 
>     wl_output.geometry(…)
>     wl_output.done()
>     xdg_output.logical_position(…)
>     xdg_output.done()
> 
> In this case the wl_output.done event fails to atomically apply the xdg_output
> state.
> 
> For this reason, I think bumping the version is a better approach.
> 
> This commit also deprecates xdg_output.done, which doesn't have any purpose
> anymore.
> 
> [1]: https://lists.x.org/archives/xorg-devel/2019-April/058148.html
> [2]: https://phabricator.kde.org/D19253
> [3]: https://github.com/swaywm/sway/issues/4064
> 
> Signed-off-by: Simon Ser <contact@emersion.fr>
> Reviewed-by: Olivier Fourdan <ofourdan@redhat.com>

Lets go with this!

Reviewed-by: Jonas Ådahl <jadahl@gmail.com>


Jonas

> ---
> 
> Changes from v2 to v3: in xdg_output's description, mention wl_output.done is
> guaranteed to be sent only from version 3.
> 
>  unstable/xdg-output/xdg-output-unstable-v1.xml | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/unstable/xdg-output/xdg-output-unstable-v1.xml b/unstable/xdg-output/xdg-output-unstable-v1.xml
> index ccbfe1c9a955..9efb7a40417b 100644
> --- a/unstable/xdg-output/xdg-output-unstable-v1.xml
> +++ b/unstable/xdg-output/xdg-output-unstable-v1.xml
> @@ -54,7 +54,7 @@
>      reset.
>    </description>
> 
> -  <interface name="zxdg_output_manager_v1" version="2">
> +  <interface name="zxdg_output_manager_v1" version="3">
>      <description summary="manage xdg_output objects">
>        A global factory interface for xdg_output objects.
>      </description>
> @@ -77,12 +77,17 @@
>      </request>
>    </interface>
> 
> -  <interface name="zxdg_output_v1" version="2">
> +  <interface name="zxdg_output_v1" version="3">
>      <description summary="compositor logical output region">
>        An xdg_output describes part of the compositor geometry.
> 
>        This typically corresponds to a monitor that displays part of the
>        compositor space.
> +
> +      For objects version 3 onwards, after all xdg_output properties have been
> +      sent (when the object is created and when properties are updated), a
> +      wl_output.done event is sent. This allows changes to the output
> +      properties to be seen as atomic, even if they happen via multiple events.
>      </description>
> 
>      <request name="destroy" type="destructor">
> @@ -157,6 +162,10 @@
> 
>  	This allows changes to the xdg_output properties to be seen as
>  	atomic, even if they happen via multiple events.
> +
> +	For objects version 3 onwards, this event is deprecated. Compositors
> +	are not required to send it anymore and must send wl_output.done
> +	instead.
>        </description>
>      </event>
> 
> --
> 2.22.0
> 
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel