[PATCH weston v3 01/13] protocol: add sub-surfaces
Jason Ekstrand
jason at jlekstrand.net
Sat Apr 27 13:18:29 PDT 2013
Sorry to spam the list, but I had another idea kicking around my head this
weekend that I thought was worth sharing. Please note that I don't think
this is a stopper. I just think it's worth throwing out there and seeing
if others think it's useful.
I don't think there are any major holes in commit behavior in the current
protocol. That said, there seems to be some confusion on commit modes and
how they interact with the tree of subalgebras that I think is justified.
I think this could be (somewhat) solved by the following simplification. We
could replace the explicit commit modes that then impose commit modes on
the children with a single cache_child_commits request which would begin
caching the data for child surfaces until commit is called on the same
surface. In terms of the current mechanism, cache_child_commits would set
synced and commit would automatically unset synced.
The advantage I see to this approach is that it makes it more clear that it
affects the entire tree and how it does so. Also, it requires the client
to explicitly put any synced commits between a cached_child_commits and
commit which I personally think is cleaner. The disadvantage is that it is
a little less flexible in that you can't cache single children. However,
if you can have "dummy nodes" (see previous ML posts), this shouldn't be a
problem in most cases.
Again, I don't think this is a show-stopper and I think the protocol as-is
is ok. I just thought it might be worth mentioning.
--Jason Ekstrand
On Fri, Apr 26, 2013 at 11:38 AM, Jason Ekstrand <jason at jlekstrand.net>wrote:
> On Fri, Apr 26, 2013 at 9:14 AM, Jason Ekstrand <jason at jlekstrand.net>wrote:
>
>> 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
>>
>
> Pekka asked me to send an e-mail with a little clarification as to what I
> meant by "strange trees". Consider an EGL video surface with SHM CSD (see
> attachment). To make things more complicated, let's say that the video
> surface is controlled by an external library or (worse yet) is a foreign
> surface controlled by a different process. Due to current commit behavior
> and the way commits cascade, the EGL surface (which really is the primary
> surface in this case) can't be used as the primary surface. Instead, you
> would have to use one of the decoration surfaces as the parent surface
> (again, see the picture).
>
> One solution to this would be to allow some sort of surface that contains
> no actual pixels but gets mapped anyway. Right now you can hack this with
> a 1x1 transparent buffer or something similar. However, I would rather not
> see such hacks common place. One way to achieve this would be to have some
> sort of empty buffer that has a size but no data. Then you could attach
> one of these empty buffers to a surface to get it mapped.
>
> The other advantage of an "empty buffer" is that, if it's used as the main
> window, could be the full size of the window instead of being 1x1. This
> simplifies the problem of handling input across subsurfaces as you can have
> the parent surface handle them. It would also allow us (if we wanted) to
> clip subsurfaces to the parent surface without causing any major problems.
> Not that we necessarily should, but it would leave open the option.
>
> Hope that's more clear,
> --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/20130427/ce0a441b/attachment-0001.html>
More information about the wayland-devel
mailing list