On Fri, 26 Apr 2013 12:02:52 +0300 Pekka Paalanen <[email protected]> wrote:
> On Thu, 25 Apr 2013 16:47:15 +0200 > David Herrmann <[email protected]> wrote: > > > Hi Pekka > > > > On Thu, Apr 25, 2013 at 12:57 PM, Pekka Paalanen <[email protected]> > > 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 <[email protected]> > > > --- > > > 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 _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
