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

Fri Feb 17 11:56:41 UTC 2017

On Thu, 19 Jan 2017 12:28:02 +0200
Pekka Paalanen <ppaalanen at gmail.com> wrote:

> On Wed, 18 Jan 2017 10:42:22 -0500 (EST)
> Olivier Fourdan <ofourdan at redhat.com> wrote:
> 
> > Hi Pekka,
> > 
> > ----- Original Message -----  
> > > 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?    
> > 
> > I meant to reply your previous email but didn't quite finish it,
> > sorry...
> > 
> > What I meant to say there is NET_WM_SYNC and _XWAYLAND_ALLOW_COMMITS
> > are two different things and I don't think you can reach the same
> > goals using NET_WM_SYNC_REQUEST.
> > 
> > The goal of NET_WM_SYNC is to make sure the window manager is not
> > flooding the client with configure requests while the user resizes
> > the window. Without NET_WM_SYNC, you can easily see the client
> > window/repaint lagging behind when resizing even with an X11
> > compositor - That's quite different from what _XWAYLAND_ALLOW_COMMITS
> > is meant for.
> > 
> > Not all apps use NET_WM_SYNC, actually few do (mostly gtk and qt
> > based apps) when considering the large number of X11 apps available,
> > so you cannot rely on NET_WM_SYNC being available, whereas having
> > _XWAYLAND_ALLOW_COMMITS in Xwayland make it available for all X11
> > clients running on Wayland with a compositor taking advantage of it.
> > 
> > So, *IMHO* _XWAYLAND_ALLOW_COMMITS should not depend on weston
> > implementing NET_WM_SYNC_REQUEST. Sorry if I caused some confusion.   
> 
> Hi Olivier,
> 
> I'm glad we agree. :-)
> 
> In my current Weston implementation, ALLOW_COMMITS is only used for the
> initial map. I suppose it could also be used to guarantee flicker-free
> decorations: just before sending Configure disable commits, and
> enable commits after the decorations have been "drawn" in the new size.
> Do you think that might be a good idea to implement in the future?
> 
> Then there is the question of whether NET_WM_SYNC should automatically
> disable/enable commits in Xwayland or do we just implement that in XWM
> via _XWAYLAND_ALLOW_COMMITS, but that's completely orthogonal.

Hi Adam,

you asked for a branch and here it is, but note: do NOT pull this directly.

It contains a patch to fix the build that I needed to verify the
branch, but probably should not be merged as is. More of that topic
here:
https://lists.x.org/archives/xorg-devel/2017-February/052770.html

I can trivially do a re-spin after the build issues are fixed upstream,
if you wish.

The following changes since commit 461aa95591fef046d3b9e8ab9182fd750a6f1de2:

  tests: fixes for non-enable-xorg build (2017-02-17 12:02:53 +0200)

are available in the git repository at:

  git://git.collabora.co.uk/git/user/pq/xserver.git allow_commits-v3-rebased

for you to fetch changes up to d90c0ca4132bef2376ffafb4f1a2269be6278c70:

  xwayland: use _XWAYLAND_ALLOW_COMMITS property (2017-02-17 12:21:17 +0200)

----------------------------------------------------------------
Adam Jackson (2):
      dix: Pass the whole property into deliverPropertyNotifyEvent
      dix: Add a callback chain for window property state change

Pekka Paalanen (3):
      xwayland: refactor into xwl_window_post_damage()
      xwayland: fix 'buffer' may be used uninitialized warning
      xwayland: use _XWAYLAND_ALLOW_COMMITS property

 dix/property.c         |  23 +++++++----
 hw/xwayland/xwayland.c | 177 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------
 hw/xwayland/xwayland.h |   3 ++
 include/property.h     |   8 ++++
 4 files changed, 181 insertions(+), 30 deletions(-)

The last patch is still missing anyone's review.

This branch is the same as the v3 submission, just rebased on top of
current master:
https://patchwork.freedesktop.org/series/16610/

The last time we were still wondering if this is was a good enough idea
or needed more use first. You haven't commented on the latest my and
Olivier's discussion or Daniel's reply.

I would warmly welcome more acks for the last patch.

The corresponding Weston patches are still here:
https://lists.freedesktop.org/archives/wayland-devel/2017-January/032712.html
https://patchwork.freedesktop.org/series/18180/

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/20170217/c4f11a6c/attachment.sig>