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

Sun Apr 28 09:04:37 UTC 2019

Hi,

On Sat, Apr 27, 2019 at 9:44 AM Simon Ser <contact at emersion.fr> wrote:

> This commit makes it so a wl_output.done event is guaranteed to be sent
> with a
> xdg_output.done event.
>

Humm, I am not sure I like changing xdg-output to rely on another protocol
event, it's looks like a weird mixup to me...

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

If the mode or scale changes, wl_output events should be sent. If the
resulting xdg-output remains the same, that's fine, no xdg-output event
would be sent and Xwayland should be fixed for that case.

Same for rotation actually, I remember seing displays (Barco, iirc) used by
ATM that were square (both physically and in resolution), applying a
rotation to such an output device would not change the resulting xdg-output
definition.

> - The compositor doesn't expose the outputs' position via wl_output, so
> whenever
>   the position changes only xdg_output is updated.
>

That sounds like a bug in the compositor, wl_output is core protocol and
the geometry event should be sent “[...] whenever any of the properties
change.”

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

To me, that feels like changing the protocol definitions to please specific
implementations  or work around bugs.

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

I'd rather we try harder to fix the implementations. In the worse case,
Xwayland would send two XRandr events instead of one, which IMHO is no big
deal.

Cheers,
Olivier

> 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>
> ---
>  unstable/xdg-output/xdg-output-unstable-v1.xml | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/unstable/xdg-output/xdg-output-unstable-v1.xml
> b/unstable/xdg-output/xdg-output-unstable-v1.xml
> index ccbfe1c..01ac7a7 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,7 +77,7 @@
>      </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.
>
> @@ -157,6 +157,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.21.0
>
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20190428/b7c60094/attachment.html>