[PATCH weston v3 01/13] protocol: add sub-surfaces
David Herrmann
dh.herrmann at gmail.com
Thu Apr 25 07:47:15 PDT 2013
Hi Pekka
On Thu, Apr 25, 2013 at 12:57 PM, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> Add protocol for sub-surfaces, wl_subcompositor as the global interface,
> and wl_subsurface as the per-surface interface extension.
>
> This patch is meant to be reverted, once sub-surfaces are moved into
> Wayland core.
>
> Changes in v2:
>
> - Rewrite wl_subcompositor.get_subsurface description, and move mapping
> and commit details into wl_subsurface description. Check the wording
> in wl_subsurface.set_position description.
>
> - Add wl_subsurface.set_commit_mode request, and document it, with the
> commit_mode enum. Add bad_value error code for wl_subsurface.
>
> - Moved the protocol into Weston repository so we can land it upstream
> sooner for public exposure. It is to be moved into Wayland core later.
>
> - Add destroy requests to both wl_subcompositor and wl_subsurface, and
> document them. Experience has showed, that interfaces should always
> have a destructor unless there is a good and future-proof reason to not
> have it.
>
> Changes in v3:
>
> - Specify, that wl_subsurface will become inert, if the corresponding
> wl_surface is destroyed, instead of requiring a certain destruction
> order.
>
> - Replaced wl_subsurface.set_commit_mode with wl_subsurface.set_sync and
> wl_subsurface.set_desync. Parent-cached commit mode is now called
> synchronized, and independent mode is desynchronized. Removed
> commit_mode enum, and bad_vBut if we introduce other protocol operations that cause state to be applied, a recursive definition is alue error.
>
> - Added support for nested sub-surfaces.
Nice! \o/
> Signed-off-by: Pekka Paalanen <ppaalanen at gmail.com>
> ---
> clients/.gitignore | 2 +
> clients/Makefile.am | 4 +
> clients/window.h | 1 +
> protocol/subsurface.xml | 236 ++++++++++++++++++++++++++++++++++++++++++++++++
> src/.gitignore | 3 +
> src/Makefile.am | 4 +
> src/compositor.h | 1 +
> tests/.gitignore | 2 +
> tests/Makefile.am | 4 +
> 9 files changed, 257 insertions(+)
> create mode 100644 protocol/subsurface.xml
>
> diff --git a/clients/.gitignore b/clients/.gitignore
> index dcd4564..16088e8 100644
> --- a/clients/.gitignore
> +++ b/clients/.gitignore
> @@ -20,6 +20,8 @@ simple-egl
> simple-shm
> simple-touch
> smoke
> +subsurface-client-protocol.h
> +subsurface-protocol.c
> tablet-shell-client-protocol.h
> tablet-shell-protocol.c
> text-client-protocol.h
> diff --git a/clients/Makefile.am b/clients/Makefile.am
> index 8c9bcd4..5f83acd 100644
> --- a/clients/Makefile.am
> +++ b/clients/Makefile.am
> @@ -81,6 +81,8 @@ libtoytoolkit_la_SOURCES = \
> window.h \
> text-cursor-position-protocol.c \
> text-cursor-position-client-protocol.h \
> + subsurface-protocol.c \
> + subsurface-client-protocol.h \
> workspaces-protocol.c \
> workspaces-client-protocol.h
>
> @@ -185,6 +187,8 @@ BUILT_SOURCES = \
> desktop-shell-protocol.c \
> tablet-shell-client-protocol.h \
> tablet-shell-protocol.c \
> + subsurface-client-protocol.h \
> + subsurface-protocol.c \
> workspaces-client-protocol.h \
> workspaces-protocol.c
>
> diff --git a/clients/window.h b/clients/window.h
> index c2946d8..815b3f1 100644
> --- a/clients/window.h
> +++ b/clients/window.h
> @@ -27,6 +27,7 @@
> #include <wayland-client.h>
> #include <cairo.h>
> #include "../shared/config-parser.h"
> +#include "subsurface-client-protocol.h"
>
> #define ARRAY_LENGTH(a) (sizeof (a) / sizeof (a)[0])
>
> diff --git a/protocol/subsurface.xml b/protocol/subsurface.xml
> new file mode 100644
> index 0000000..60b4002
> --- /dev/null
> +++ b/protocol/subsurface.xml
> @@ -0,0 +1,236 @@
> +<?xml version="1.0" encoding="UTF-8"?>
> +<protocol name="subsurface">
> +
> + <copyright>
> + Copyright © 2012-2013 Collabora, Ltd.
> +
> + Permission to use, copy, modify, distribute, and sell this
> + software and its documentation for any purpose is hereby granted
> + without fee, provided that the above copyright notice appear in
> + all copies and that both that copyright notice and this permission
> + notice appear in supporting documentation, and that the name of
> + the copyright holders not be used in advertising or publicity
> + pertaining to distribution of the software without specific,
> + written prior permission. The copyright holders make no
> + representations about the suitability of this software for any
> + purpose. It is provided "as is" without express or implied
> + warranty.
> +
> + THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
> + SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
> + FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY
> + SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN
> + AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION,
> + ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF
> + THIS SOFTWARE.
> + </copyright>
> +
> + <interface name="wl_subcompositor" version="1">
> + <description summary="sub-surface compositing">
> + The global interface exposing sub-surface compositing capabilities.
> + A wl_surface, that has sub-surfaces associated, is called the
> + parent surface. Sub-surfaces can be arbitrarily nested and create
> + a tree of sub-surfaces.
> +
> + The root surface in a tree of sub-surfaces is the main
> + surface. The main surface cannot be a sub-surface, because
> + sub-surfaces must always have a parent.
> +
> + A main surface with its sub-surfaces forms a (compound) window.
> + For window management purposes, this set of wl_surface objects is
> + to be considered as a single window, and it should also behave as
> + such.
> +
> + The aim of sub-surfaces is to offload some of the compositing work
> + within a window from clients to the compositor. A prime example is
> + a video player with decorations and video in separate wl_surface
> + objects. This should allow the compositor to pass YUV video buffer
> + processing to dedicated overlay hardware when possible.
> + </description>
> +
> + <request name="destroy" type="destructor">
> + <description summary="unbind from the subcompositor interface">
> + Informs the server that the client will not be using this
> + protocol object anymore. This does not affect any other
> + objects, wl_subsurface objects included.
> + </description>
> + </request>
> +
> + <enum name="error">
> + <entry name="bad_surface" value="0"
> + summary="the to-be sub-surface is invalid"/>
> + <entry name="bad_parent" value="1"
> + summary="the given parent is a sub-surface"/>
> + </enum>
> +
> + <request name="get_subsurface">
> + <description summary="give a surface the role sub-surface">
> + Create a sub-surface interface for the given surface, and
> + associate it with the given parent surface. This turns a
> + plain wl_surface into a sub-surface.
> +
> + The to-be sub-surface must not already have a dedicated
> + purpose, like any shell surface type, cursor image, drag icon,
> + or sub-surface. Otherwise a protocol error is raised.
> + </description>
> +
> + <arg name="id" type="new_id" interface="wl_subsurface"
> + summary="the new subsurface object id"/>
> + <arg name="surface" type="object" interface="wl_surface"
> + summary="the surface to be turned into a sub-surface"/>
> + <arg name="parent" type="object" interface="wl_surface"
> + summary="the parent surface"/>
> + </request>
> + </interface>
> +
> + <interface name="wl_subsurface" version="1">
> + <description summary="sub-surface interface to a wl_surface">
> + An additional interface to a wl_surface object, which has been
> + made a sub-surface. A sub-surface has one parent surface.
> +
> + A sub-surface becomes mapped, when a non-NULL wl_buffer is applied
> + and the parent surface is mapped. The order of which one happens
> + first is irrelevant. A sub-surface is hidden if the parent becomes
> + hidden, or if a NULL wl_buffer is applied. These rules apply
> + recursively through the tree of surfaces.
> +
> + The behaviour of wl_surface.commit request on a sub-surface
> + depends on the sub-surface's mode. The possible modes are
> + synchronized and desynchronized, see methods
> + wl_subsurface.set_sync and wl_subsurface.set_desync. Synchronized
> + mode caches wl_surface state to be applied on the next parent
> + surface's commit, and desynchronized mode applies the pending
I had to look at the code to actually understand which implementation
you chose. It is not clear to me (from that description) what happens
in the case that you described earlier:
C.commit
B.commit
C.commit
A.commit
(assuming A<-B<-C stacking)
According to your implementation you apply a sub-surface state if
wl_surface.commit is called on _any_ ancestor. I think you should
mention that explicitly. This description could mean both, but your
description below is definitely misleading.
> + wl_surface state directly. A sub-surface is initially in the
> + synchronized mode.
> +
> + Sub-surfaces have also other kind of state, which is managed by
> + wl_subsurface requests, as opposed to wl_surface requests. This
> + state includes the sub-surface position relative to the parent
> + surface (wl_subsurface.set_position), and the stacking order of
> + the parent and its sub-surfaces (wl_subsurface.place_above and
> + .place_below). This state is applied when the parent surface's
> + wl_surface state is applied, regardless of the sub-surface's mode.
> + As the exception, set_sync and set_desync are effective immediately.
> +
> + The main surface can thought to be always in desynchronized mode,
> + since it does not have a parent in the sub-surfaces sense.
> +
> + Even if a sub-surface is in desynchronized mode, it will behave as
> + in synchronized mode, if its parent surface behaves as in
> + synchronized mode. This rule is applied recursively throughout the
> + tree of surfaces. This means, that one can set a sub-surface into
> + synchronized mode, and then assume that all its child sub-surfaces
> + are synchronized, too, without explicitly setting them.
> +
> + If the wl_surface associated with the wl_subsurface is destroyed, the
> + wl_subsurface object becomes inert. Note, that destroying either object
> + takes effect immediately. If you need to synchronize the removal
> + of a sub-surface to the parent surface update, unmap the sub-surface
> + first by attaching a NULL wl_buffer, update parent, and then destroy
> + the sub-surface.
> +
> + If the parent wl_surface object is destroyed, the sub-surface is
> + unmapped.
> + </description>
> +
> + <request name="destroy" type="destructor">
> + <description summary="remove sub-surface interface">
> + The sub-surface interface is removed from the wl_surface object
> + that was turned into a sub-surface with
> + wl_subcompositor.get_subsurface request. The wl_surface's association
> + to the parent is deleted, and the wl_surface loses its role as
> + a sub-surface. The wl_surface is unmapped.
> + </description>
> + </request>
> +
> + <enum name="error">
> + <entry name="bad_surface" value="0"
> + summary="wl_surface is not a sibling or the parent"/>
> + </enum>
> +
> + <request name="set_position">
> + <description summary="reposition the sub-surface">
> + This schedules a sub-surface position change.
> + The sub-surface will be moved so, that its origin (top-left
> + corner pixel) will be at the location x, y of the parent surface.
> +
> + The next wl_surface.commit on the parent surface will reset
> + the sub-surface's position to the scheduled coordinates.
> +
> + The initial position is 0, 0.
Your patch doesn't mention what happens if the parent doesn't fully
include the sub-surface. I think it should be clear whether the child
is clipped or not.
I know you postponed the rotation/clipping extension, but we should
still define the behavior for raw sub-surfaces. I guess "no clipping"
is the best option. Or is that implied by not mentioning it?
> + </description>
> +
> + <arg name="x" type="int" summary="coordinate in the parent surface"/>
> + <arg name="y" type="int" summary="coordinate in the parent surface"/>
> + </request>
> +
> + <request name="place_above">
> + <description summary="restack the sub-surface">
> + This sub-surface is taken from the stack, and put back just
> + above the reference surface, changing the z-order of the sub-surfaces.
> + The reference surface must be one of the sibling surfaces, or the
> + parent surface. Using any other surface, including this sub-surface,
> + will cause a protocol error.
> +
> + The z-order is double-buffered state, and will be applied on the
> + next commit of the parent surface.
> + See wl_surface.commit and wl_subcompositor.get_subsurface.
> + </description>
> +
Maybe I missed it, but what's the initial z-order for new
sub-surfaces? I guess it's "top-most" but I think it should be
mentioned either here or in the wl_subsurface description.
> + <arg name="sibling" type="object" interface="wl_surface"
> + summary="the reference surface"/>
> + </request>
> +
> + <request name="place_below">
> + <description summary="restack the sub-surface">
> + The sub-surface is placed just below of the reference surface.
> + See wl_subsurface.place_above.
> + </description>
> +
> + <arg name="sibling" type="object" interface="wl_surface"
> + summary="the reference surface"/>
> + </request>
> +
> + <request name="set_sync">
> + <description summary="set sub-surface to synchronized mode">
> + Change the commit behaviour of the sub-surface to synchronized
> + mode, also described as the parent dependant mode.
> +
> + In synchronized mode, wl_surface.commit on a sub-surface will
> + accumulate the committed state in a cache, but the state will
> + not be applied and hence will not change the compositor output.
> + The cached state is applied to the sub-surface when
> + wl_surface.commit is called on the parent surface, after the
This is the description that I was talking about. It's misleading. A
sub-surface state is _not_ necessarily applied _only_ when
wl_surface.commit is called on the parent. Instead it is applied if
the parent's state is applied.
On the first sight this seems to be identical scenarios, but it's not:
wl_surface.commit causes a state and all synchronized sub-surface
states to be applied. That means, a state of a surface is applied when
wl_surface.commit is called on _any_ ancestor (direct or indirect
parent) not only the direct parent.
With the current implementation this is identical to:
wl_surface.commit is called on the top-most synchronized parent. All
other commits in between are invisible to the client.
So whether "wl_surface.commit is called" or whether a "surface's state
is applied" are no longer identical. The first implies the second, but
not vice versa.
Other than these few documentation details, the extension looks really
nice. I doubt that we need the multi-layer-cache that we were talking
about with v2 so I like this proposal a lot!
Regards
David
More information about the wayland-devel
mailing list