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

Submitted by Simon Ser on July 2, 2019, 3:33 p.m.

Details

Message ID c4IrN7zJa1pgsdPD9AmnPAVO_L6revU7C2SqHoVACNuG-LIFHimlKfQvBjjqHfkBDKqLdl7cRaH5u6uRwtFWyPC46bCXc86654wDJkEJ9i0=@emersion.fr
State Superseded
Headers show
Series "xdg-output: deprecate the xdg_output.done event" ( rev: 2 ) in Wayland

Not browsing as part of any series.

Commit Message

Simon Ser July 2, 2019, 3:33 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 v1 to v2: state in xdg_output's description that wl_output.done
is sent after properties have been sent (Jonas)

 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..e3ed58766d17 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.
+
+      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

Hi Jonas,

Did you get the chance to have a look at this new version?

Thanks!

Simon

On Tuesday, July 2, 2019 6:33 PM, Simon Ser <contact@emersion.fr> 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>
> ---
>
> Changes from v1 to v2: state in xdg_output's description that wl_output.done
> is sent after properties have been sent (Jonas)
>
>  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..e3ed58766d17 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.
> +
> +      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
On Tue, Jul 02, 2019 at 03:33:00PM +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>
> ---
> 
> Changes from v1 to v2: state in xdg_output's description that wl_output.done
> is sent after properties have been sent (Jonas)
> 
>  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..e3ed58766d17 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.
> +
> +      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.

This is a good description, but probably want to add that it's only
after version 3 this is guaranteed.


Jonas

>      </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
> 
>