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

[Jonas Ådahl]

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 at emersion.fr>
> Reviewed-by: Olivier Fourdan <ofourdan at 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
> 
>