[PATCH weston v3 01/13] protocol: add sub-surfaces
Jason Ekstrand
jason at jlekstrand.net
Fri Apr 26 07:14:26 PDT 2013
pq,
On Fri, Apr 26, 2013 at 7:09 AM, David Herrmann <dh.herrmann at gmail.com>wrote:
> 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.
>
My thoughts were basically the same as David's and those changes solve my
concerns as well. I'm still not sure I'm 100% happy with how it requires
you to build strange trees in certain cases. That said, I think it looks
pretty good and certainly good enough to go into weston so we can all start
playing with it. I'm hoping I can get sub-surfaces implemented in my java
compositor soonish. Looking good!
--Jason Ekstrand
Thanks
> David
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20130426/d4bdb474/attachment-0001.html>
More information about the wayland-devel
mailing list