[PATCH weston v3 01/13] protocol: add sub-surfaces
Pekka Paalanen
ppaalanen at gmail.com
Fri Apr 26 04:12:16 PDT 2013
On Fri, 26 Apr 2013 12:02:52 +0300
Pekka Paalanen <ppaalanen at gmail.com> wrote:
> 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/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
How's this?
http://cgit.collabora.com/git/user/pq/weston.git/commit/?h=subsurface-wip2&id=0dce7236606e0a433390c970560c3050247c4e60&context=6
Thanks,
pq
More information about the wayland-devel
mailing list