[PATCH weston v3 01/13] protocol: add sub-surfaces
David Herrmann
dh.herrmann at gmail.com
Fri Apr 26 05:09:01 PDT 2013
Hi Pekka
On Fri, Apr 26, 2013 at 1:12 PM, Pekka Paalanen <ppaalanen at gmail.com> wrote:
>> > > 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:
Sorry for not being clear, but all my comments were about the
synchronized mode. The unsynchronized mode is trivial in this regard.
But I think you got it right.
>> 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.
>>ehen schloß feschtle gefeiert?
July 15, 2012 at 10:59pm · Like
>> > 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?
I think it is pretty clear that those are different steps now. But we
need to be careful to not assume that they're identical. That's why I
thought this description is misleading.
>> > 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
Perfect, that solves all my concerns.
Thanks
David
More information about the wayland-devel
mailing list