<div dir="ltr"><div dir="ltr"><div>Hi,</div><div><br></div><div><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sat, Apr 27, 2019 at 9:44 AM Simon Ser <<a href="mailto:contact@emersion.fr" target="_blank">contact@emersion.fr</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">This commit makes it so a wl_output.done event is guaranteed to be sent with a<br>
xdg_output.done event.<br></blockquote><div><br></div><div>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...</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">

This protocol change has been discussed in a recent xorg-devel discussions [1].<br>
<br>
First let's recap why a change is needed: Xwayland listens to both wl_output and<br>
xdg_output changes. When an output's properties change, Xwayland expects to<br>
receive both a wl_output.done event and an xdg_output.done event. If that's not<br>
the case, Xwayland doesn't update its state (so old state is still exposed to<br>
X11 clients).<br>
<br>
Most of the time, both objects will be updated at the same time (e.g. the<br>
current mode is changed, so both wl_output.mode and xdg_output.logical_size are<br>
sent) so this won't be an issue. However in some situations only one of<br>
wl_output or xdg_output changes. For instance:<br>
<br>
- The mode is changed at the same time as the scale, resulting in the same<br>
  logical_size.<br></blockquote><div><br></div><div>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.</div><div><br></div><div>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.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
- The compositor doesn't expose the outputs' position via wl_output, so whenever<br>
  the position changes only xdg_output is updated.<br></blockquote><div><br></div><div>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.”</div><div></div><div></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">

Both KDE [2] and wlroots [3] have experienced this issue.<br></blockquote><div><br></div><div>To me, that feels like changing the protocol definitions to please specific implementations  or work around bugs.<br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">

For this reason, I'd like to update the xdg-output protocol to make it mandatory<br>
to always send a wl_output.done event after xdg_output changes. This effectively<br>
makes wl_output.done atomically apply all output state (including the state of<br>
add-on objects like xdg_output). This approach is pretty similar to<br>
wl_surface.commit: this request will atomically apply surface state including<br>
the state of e.g. the xdg_surface object tied to the wl_surface.<br>
<br>
To update the protocol to reflect this new requirement we can either:<br>
<br>
- **Bump xdg_output version**. The current protocol doesn't specify that<br>
  wl_output.done must be sent, adding this new requirement would be a breaking<br>
  change. We need to fix Xwayland for the current xdg_output version (maybe make<br>
  it non-atomic for the current version, atomic for the new one?). Should we<br>
  deprecate xdg_output.done in the new version?<br>
- **Don't bump xdg_output version**. This clarifies what is expected in practice<br>
  by Xwayland, a major xdg_output consumer, and what is currently implemented by<br>
  all compositors.<br>
<br>
There's one issue with the "don't bump" approach: indeed in practice compositors<br>
always send wl_output.done and xdg_output.done in pairs, however the ordering<br>
between those two events is not guaranteed. This means some compositors might<br>
send this sequence:<br>
<br>
    wl_output.geometry(…)<br>
    wl_output.done()<br>
    xdg_output.logical_position(…)<br>
    xdg_output.done()<br>
<br>
In this case the wl_output.done event fails to atomically apply the xdg_output<br>
state.<br>
<br>
For this reason, I think bumping the version is a better approach.<br></blockquote><div><br></div><div>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.<br></div><div><br></div><div> Cheers,</div><div>Olivier</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
This commit also deprecates xdg_output.done, which doesn't have any purpose<br>
anymore.<br>
<br>
[1]: <a href="https://lists.x.org/archives/xorg-devel/2019-April/058148.html" rel="noreferrer" target="_blank">https://lists.x.org/archives/xorg-devel/2019-April/058148.html</a><br>
[2]: <a href="https://phabricator.kde.org/D19253" rel="noreferrer" target="_blank">https://phabricator.kde.org/D19253</a><br>
[3]: <a href="https://github.com/swaywm/sway/issues/4064" rel="noreferrer" target="_blank">https://github.com/swaywm/sway/issues/4064</a><br>
<br>
Signed-off-by: Simon Ser <<a href="mailto:contact@emersion.fr" target="_blank">contact@emersion.fr</a>><br>
---<br>
 unstable/xdg-output/xdg-output-unstable-v1.xml | 8 ++++++--<br>
 1 file changed, 6 insertions(+), 2 deletions(-)<br>
<br>
diff --git a/unstable/xdg-output/xdg-output-unstable-v1.xml b/unstable/xdg-output/xdg-output-unstable-v1.xml<br>
index ccbfe1c..01ac7a7 100644<br>
--- a/unstable/xdg-output/xdg-output-unstable-v1.xml<br>
+++ b/unstable/xdg-output/xdg-output-unstable-v1.xml<br>
@@ -54,7 +54,7 @@<br>
     reset.<br>
   </description><br>
<br>
-  <interface name="zxdg_output_manager_v1" version="2"><br>
+  <interface name="zxdg_output_manager_v1" version="3"><br>
     <description summary="manage xdg_output objects"><br>
       A global factory interface for xdg_output objects.<br>
     </description><br>
@@ -77,7 +77,7 @@<br>
     </request><br>
   </interface><br>
<br>
-  <interface name="zxdg_output_v1" version="2"><br>
+  <interface name="zxdg_output_v1" version="3"><br>
     <description summary="compositor logical output region"><br>
       An xdg_output describes part of the compositor geometry.<br>
<br>
@@ -157,6 +157,10 @@<br>
<br>
        This allows changes to the xdg_output properties to be seen as<br>
        atomic, even if they happen via multiple events.<br>
+<br>
+       For objects version 3 onwards, this event is deprecated. Compositors<br>
+       are not required to send it anymore and must send wl_output.done<br>
+       instead.<br>
       </description><br>
     </event><br>
<br>
--<br>
2.21.0<br>
<br>
<br>
_______________________________________________<br>
wayland-devel mailing list<br>
<a href="mailto:wayland-devel@lists.freedesktop.org" target="_blank">wayland-devel@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/wayland-devel" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/wayland-devel</a></blockquote></div></div></div></div>