[PATCH wayland 1/4] protocol: double-buffered state for wl_surface

[Pekka Paalanen]

On Thu, 11 Oct 2012 13:00:40 +0200
David Herrmann <dh.herrmann at googlemail.com> wrote:

> Hi Pekka
> 
> On Wed, Oct 10, 2012 at 11:47 AM, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> > This change breaks the protocol.
> >
> > The current protocol is racy in that updates to surface content and
> > surface state (e.g. damage, input and opaque regions) are not guaranteed
> > to happen at the same time. Due to protocol buffering and handling
> > practices, the issues are very hard to trigger.
> >
> > Committing damage to a surface at arbitrary times makes it hard to
> > track when the wl_buffer is being read by the server, and when it is
> > safe to overwrite (the case of wl_shm with a single buffer reused
> > constantly).
> >
> > This protocol change introduces the concept of double-buffered state.
> > Such state is accumulated and cached in the server, unused, until the
> > final commit request. The surface will receive its new content and apply
> > its new state atomically.
> >
> > A wl_surface.commit request is added to the protocol. This is thought to
> > be more clear, than having wl_surface.attach committing implicitly, and
> > then having another request to commit without attaching, as would be
> > required for a GL app that wants to change e.g. input region without
> > redrawing.
> >
> > When these changes are implemented, clients do not have to worry about
> > ordering damage vs. input region vs. attach vs. ... anymore. Clients set
> > the state in any order they want, and kick it all in with a commit.
> >
> > The interactions between wl_surface.attach, (wl_surface.commit,)
> > wl_buffer.release, and wl_buffer.destroy have been undocumented. Only
> > careful inspection of the compositor code has told when a wl_buffer is
> > free for re-use, especially for wl_shm and wrt. wl_surface.damage.
> > Try to clarify how it all should work, and what happens if the wl_buffer
> > gets destroyed.
> >
> > An additional minor fix: allow NULL argument to
> > wl_surface.set_opaque_region. The wording in the documentation already
> > implied that a nil region is allowed.
> >
> > Signed-off-by: Pekka Paalanen <ppaalanen at gmail.com>
> > ---
> >  protocol/wayland.xml |  121 ++++++++++++++++++++++++++++++++++++++++---------
> >  1 files changed, 98 insertions(+), 23 deletions(-)
> >
> > diff --git a/protocol/wayland.xml b/protocol/wayland.xml
> > index efd71dd..c45a404 100644
> > --- a/protocol/wayland.xml
> > +++ b/protocol/wayland.xml
> > @@ -630,9 +630,37 @@
> >
> >      <request name="attach">
> >        <description summary="set the surface contents">
> > -       Copy the contents of a buffer into this surface. The x and y
> > -       arguments specify the location of the new buffers upper left
> > -       corner, relative to the old buffers upper left corner.
> > +       Set the contents of a buffer into this surface. The x and y
> > +       arguments specify the location of the new pending buffer's upper
> > +       left corner, relative to the current buffer's upper left corner. In
> > +       other words, the x and y, and the width and height of the wl_buffer
> > +       together define in which directions the surface's size changes.
> > +
> > +       Surface contents are double-buffered state, see wl_surface.commit.
> > +
> > +       The initial surface contents are void; there is no content.
> > +       wl_surface.attach assigns the given wl_buffer as the pending wl_buffer.
> > +       wl_surface.commit applies the pending wl_buffer as the new
> > +       surface contents, and the size of the surface becomes the size of
> > +       the wl_buffer. The wl_buffer is also kept as pending, until
> > +       changed by wl_surface.attach or the wl_buffer is destroyed.
> > +
> > +       Committing a pending wl_buffer allows the compositor to read the
> > +       pixels in the wl_buffer. The compositor may access the pixels at any
> > +       time after the wl_surface.commit request. When the compositor will
> > +       not access the pixels anymore, it will send the wl_buffer.release
> > +       event. Only after receiving wl_buffer.release, the client may re-use
> > +       the wl_buffer.
> > +
> > +       Destroying the wl_buffer after wl_buffer.release does not change the
> > +       surface contents, even if the wl_buffer is still pending for the
> > +       next commit. In such case, the next commit does not change the
> > +       surface contents. However, if the client destroys the wl_buffer
> > +       before receiving wl_buffer.release, the surface contents become
> > +       undefined immediately.
> > +
> > +       Only if wl_surface.attach is sent with a nil wl_buffer, the
> > +       following wl_surface.commit will remove the surface content.
> 
> If I do multiple wl_surface.attach()s on different buffers and after
> that a wl_surface.commit(), do I get wl_buffer.release() events for
> the buffers that I attached first and aren't affected by the commit?
> I guess not but I just wanted to go sure.

You do not, since those buffers were never actually in use by the
compositor. I can clarify this in the spec.


Thanks,
pq