[PATCH weston v3 01/13] protocol: add sub-surfaces
Pekka Paalanen
ppaalanen at gmail.com
Fri Apr 26 02:02:52 PDT 2013
On Thu, 25 Apr 2013 16:47:15 +0200
David Herrmann <dh.herrmann at gmail.com> wrote:
> 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)
That really depends on the commit modes of each surface, but if we
assume A is main, and B and C are synchronized (IIRC that was the
example), then:
C.commit(buffer C1)
- C1 becomes cached in C
B.commit(buffer B1)
- B1 becomes cached in B
C.commit(buffer C2)
- C2 becomes cached in C, and C1 is released
A.commit(buffer A1)
- A1 becomes current in A, B1 becomes current in B, C2 becomes current
in C
Actually, C's commit mode is irrelevant. As long as B is synchronized,
C will behave as synchronized.
> 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.
Hmm, no. I only apply the new state in children, if the children are
effectively in synchronized mode.
A sub-surface has a commit mode, which is either desynchronized or
synchronized. However, the effective commit mode is synchronized, if
any of sub-surface's ancestors are in synchronized mode.
Maybe I should explain the difference between the commit mode and the
effective commit mode here?
I'm uncertain how to clarify what you are asking. Can you propose
something here?
> > + 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?
I thought not mentioning anything implies no clipping, and that is how
it is implemented. Sub-surface area is in no way restricted by the
parent or ancestors.
No clipping is essential for stitching window decorations from
sub-surfaces, while eliminating surface overlap, for instance.
I'll add a note here.
I never suggested rotation. It is the clipping and scaling extension
that will follow separately, and is applicable to any wl_surface.
> > + </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.
Yeah, it is top-most of the particular parent's children. The logical
place to document it is in wl_subsurface.place_above, right?
I'll add it.
> > + <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.
Right, I need to fix that. Left-overs from before nesting support.
> 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.
Yeah.
> 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.
Actually, neither implies the other. Btw. is it clear enough, that
commit and apply are separate steps now?
> 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!
Oh yeah, the multi-layer cache would be madness on resource usage.
Thank you, David!
- pq
More information about the wayland-devel
mailing list