[PATCH xserver v2 5/5] xwayland: use _XWAYLAND_ALLOW_COMMITS property

[Pekka Paalanen]

On Thu, 12 Jan 2017 16:27:10 +0200
Pekka Paalanen <ppaalanen at gmail.com> wrote:

> On Tue, 3 Jan 2017 04:31:39 -0500 (EST)
> Olivier Fourdan <ofourdan at redhat.com> wrote:
> 
> > Hi,
> > 
> > ----- Original Message -----  
> > > On Fri, 2016-12-09 at 14:24 +0200, Pekka Paalanen wrote:    
> > > > From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> > > > 
> > > > The X11 window manager (XWM) of a Wayland compositor can use the
> > > > _XWAYLAND_ALLOW_COMMITS property to control when Xwayland sends
> > > > wl_surface.commit requests. If the property is not set, the behaviour
> > > > remains what it was.    
> > > 
> > > I'm still thinking over whether I like this or whether I'd rather have
> > > this keyed off the netwm sync props. (Did we think that was a thing
> > > that could work? Forgive me, I'm freshly back from holidays.) [...]    
> > 
> > Maybe it's two different goals. If I understand correctly, this patch
> > was initially intended for the initial map of the window.  
> 
> Hi,
> 
> yes, that's correct.
> 
> Btw. I would love to write a documentation patch explaining what
> _XWAYLAND_ALLOW_COMMITS is meant for, how it works, and how it should
> be used. Where would such documentation live? I might suggest putting
> it in the Wayland docbook, but that would be a bit odd, since it is
> documenting X.org's Xwayland implementation. But I bet it would make
> reviewing this patch easier.
> 
> Originally I was working on letting -geometry option of X11 programs to
> allow setting the initial position of the window. I got it implemented
> in Weston, but with the regression that all decorated windows would
> appear to jump once when mapped (without -geometry, even). The root
> cause for that was the racing between Xwayland and the XWM/compositor.
> 
> Xwayland did the first commit before the decorations were drawn, that
> caused the compositor to position and show the window, then the
> decorations appeared and the window needed to move to account for the
> added decorations. Except that is not the whole truth, because if it
> happened like that, I would see the decorations appear around the
> window, but instead I see the decorated window jump to another
> position, so there is probably also a drawing/compositing race going
> on. Plus I am not quite sure if Xwayland is still single-buffered to
> towards the compositor which would be incorrect behaviour of a Wayland
> client (but was originally written so, because maintaining multiple
> buffers would have cost significantly more and X11 drawing does not
> have a concept of "completed frames" anyway...).
> 
> Anyway, the initial mapping is a mess if Xwayland commits before XWM is
> ready, and it would be quite difficult for XWM to internally postpone
> the mapping in the compositor. I hope we agreed at least that much
> already.
> 
> This patch is particularly for the initial mapping, especially for apps
> and compositors that do not use NET_WM_SYNC (yet).
> 
> Adding support for NET_WM_SYNC protocol would be the next step (in
> Weston), but I am not sure when I would get there. I suppose the XWM
> could drive it via _XWAYLAND_ALLOW_COMMITS or Xwayland could hook up to
> the NET_WM_SYNC protocol messages itself like Adam and Daniel discussed.

Hi all,

here is an update on the Weston side:
https://lists.freedesktop.org/archives/wayland-devel/2017-January/032712.html

The related Weston patches series has shrunk from 24 to 3 patches as
lots of them have been merged. The stuff about _XWAYLAND_ALLOW_COMMITS
is still pending.

Given the development on Weston side, would you demand implementing
NET_WM_SYNC_REQUEST support in Weston before deciding whether to merge
_XWAYLAND_ALLOW_COMMITS support in Xwayland, or is the rationale in the
remaining Weston patches enough to justify it already?

The window jumping I talked about no longer happens, due to reordering
the patch series, but I believe that is just the race being won rather
than lost most of the time and that the race which
_XWAYLAND_ALLOW_COMMITS will remove still exists.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 801 bytes
Desc: OpenPGP digital signature
URL: <https://lists.x.org/archives/xorg-devel/attachments/20170118/51b01692/attachment.sig>