<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Apr 26, 2013 at 9:14 AM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">pq, <div class="gmail_extra"> <div class="gmail_quote"><div><div class="h5">On Fri, Apr 26, 2013 at 7:09 AM, David Herrmann <<a href="mailto:dh.herrmann@gmail.com" target="_blank">dh.herrmann@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Pekka <div><div> On Fri, Apr 26, 2013 at 1:12 PM, Pekka Paalanen <<a href="mailto:ppaalanen@gmail.com" target="_blank">ppaalanen@gmail.com</a>> 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: </div></div>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. <div> >> 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. </div>>>ehen schloß feschtle gefeiert? July 15, 2012 at 10:59pm · Like <div><div>>> > 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? </div></div>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. <div> >> > 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? > <a href="http://cgit.collabora.com/git/user/pq/weston.git/commit/?h=subsurface-wip2&id=0dce7236606e0a433390c970560c3050247c4e60&context=6" target="_blank">http://cgit.collabora.com/git/user/pq/weston.git/commit/?h=subsurface-wip2&id=0dce7236606e0a433390c970560c3050247c4e60&context=6</a> </div>Perfect, that solves all my concerns. </blockquote><div> </div></div></div><div>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! </div><div>--Jason Ekstrand </div></div></div></div></blockquote><div> </div><div>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). </div><div>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. </div><div>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. </div><div>Hope that's more clear, </div><div>--Jason Ekstrand </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"> <div class="gmail_quote"><div></div><div class="im"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Thanks David <div><div>_______________________________________________ wayland-devel mailing list <a href="mailto:wayland-devel@lists.freedesktop.org" target="_blank">wayland-devel@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/wayland-devel" target="_blank">http://lists.freedesktop.org/mailman/listinfo/wayland-devel</a> </div></div></blockquote></div></div> </div></div> </blockquote></div> </div></div>