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

[David Herrmann]

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.

Regards
David