Atomic surface state updates (Re: [PATCH 0/8] Thread safety patches and 1.0 plan)

Wed Oct 10 19:39:39 PDT 2012

On Wed, Oct 10, 2012 at 12:46:18PM +0300, Pekka Paalanen wrote:
> On Tue,  9 Oct 2012 22:37:57 -0400
> Kristian Høgsberg <krh at bitplanet.net> wrote:
> 
> > Hi all,
> > 
> > Wayland 1.0 is just around the corner and we have a couple of blockers for
> > that release: thread safety of the client side API and the surface commit
> > feature.  Pekka has been working on the commit part and these eight patches
> > rework the client library to be thread safe.  I've been meaning to release 1.0
> > this week, but we may have to take a few more days to let these changes
> > settle down.  We ended up changing the API and protocol a little more and
> > a little closer to 1.0 than I was hoping we'd need to.
> ...
> > With these and Pekkas changes, we'll have to update the clients once more.
> > Unfortunately Mesa 9.0 was just released, so we'll have to get these changes
> > into a 9.0.1 release.  Toolkits will need a change as well.
> > 
> > Anyway, I would much appreciate feedback on these patches, but I do also
> > expect that we'll merge them (and Pekkas patches) within a couple of days,
> > update clients and toolkits and then cut the 1.0 release of wayland next week.
> 
> Hi,
> 
> I will reply with my current patch sets to this email. My patch sets
> are not rebased on top of Kristian's work yet.
> 
> First, the Wayland protocol changes for introducing atomic surface
> state updates by making surface state double-buffered. This means, that
> wl_surface.attach, damage, set_input_region etc. do not take effect
> immediately when the server receives the requests. You need a commit
> request, that will kick it all in atomically. See the commit message of
> the first patch for details.
> 
> 
> The following changes since commit 0371668dcca5aa29c0fc4b59b974e40bcf073563:
> 
>   Ensure cursor_data.c is included in distribution tarballs (2012-10-09 23:42:52 -0400)
> 
> are available in the git repository at:
>   git://git.collabora.co.uk/git/user/pq/wayland.git latched-2
> 
> Pekka Paalanen (4):
>       protocol: double-buffered state for wl_surface
>       protocol: clarify input region on drags and pointers
>       protocol: wl_surface.frame needs wl_surface.commit
>       protocol: elaborate on wl_buffer
> 
>  protocol/wayland.xml |  169 +++++++++++++++++++++++++++++++++++++++++--------
>  1 files changed, 141 insertions(+), 28 deletions(-)
> 
> 
> Then the implementation of the above for Weston compositor and demo
> clients:
> 
> The following changes since commit c37b32dac29b3e9989ed54f9582707be467f7a68:
> 
>   compositor-wayland: Use some nicer border sizes. (2012-10-09 23:17:46 -0400)
> 
> are available in the git repository at:
>   git://git.collabora.co.uk/git/user/pq/wayland-demos.git latched-2
> 
> Pekka Paalanen (12):
>       compositor: make wl_surface.attach double-buffered
>       clients: use wl_surface_commit
>       compositor, clients: make damage double-buffered
>       shell: do not double-init a region
>       compositor, clients: double-buffer opaque region
>       compositor, clients: double-buffer input region
>       shell: do not double-init input region
>       compositor: fix surface pointer cast
>       compositor, clients: apply wl_surface.frame on commit
>       compositor: fix event-test
>       compositor: add weston_surface:commit_signal
>       compositor: allow override of wl_surface.commit
> 
>  clients/dnd.c            |    2 +
>  clients/simple-egl.c     |    4 +-
>  clients/simple-shm.c     |    1 +
>  clients/simple-touch.c   |    5 +
>  clients/smoke.c          |    1 +
>  clients/window.c         |   46 ++++++-----
>  src/compositor-wayland.c |    4 +-
>  src/compositor.c         |  207 ++++++++++++++++++++++++++++------------------
>  src/compositor.h         |   40 ++++++++-
>  src/shell.c              |    2 +
>  10 files changed, 202 insertions(+), 110 deletions(-)
> 
> That work is still missing some things:
> - simple-egl will hang, this is caused by a nested wl_display_iterate()
>   call from Mesa. Kristian's thread safety patches should fix this.
> - nested Weston will hang: I did not investigate this yet, but could be
>   the same cause as for simple-egl
> - xwayland will need fixing
> - wl_buffer.release handling in Weston is buggy, and none of the
>   clients use it at all; needs fixing
> I will fix the above.
> 
> Also the last two patches are more for reference for sub-surface
> support in the future, since they add details that are still unused:
>       compositor: add weston_surface:commit_signal
>       compositor: allow override of wl_surface.commit
> 
> Then of course all the clients and toolkits out in the wild need fixing.
> 
> Finally, there is a two-line Mesa patch to send wl_surface.commit from
> eglSwapBuffers(), that lets at least GL/GLESv2 toytoolkit clients work.
> 
> simple-egl is special, because it calls eglSwapBuffers() from a
> protocol event handler, and therefore ends up in nesting
> wl_display_iterate() through Mesa, and exploding.
> 
> If you find other breakages, or problems in the protocol changes or
> language, let me know. The protocol should be self-documented well
> enough to not need further explanations.

I've integrated the thread safety patches and your commit patches and
pushed it all out here:

  http://cgit.freedesktop.org/~krh/wayland/log/?h=next
  http://cgit.freedesktop.org/~krh/weston/log/?h=next
  http://cgit.freedesktop.org/~krh/mesa/log/?h=next (based on mesa master)

As expected, simple-egl no longer explodes and it all seems to be
working fine.  I've done an initial pass through all your patches and
they look good, but I'll do another pass tomorrow.

Kristian