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/clients/.gitignore b/clients/.gitignore > > index dcd4564..16088e8 100644 > > --- a/clients/.gitignore > > +++ b/clients/.gitignore > > @@ -20,6 +20,8 @@ simple-egl > > simple-shm > > simple-touch > > smoke > > +subsurface-client-protocol.h > > +subsurface-protocol.c > > tablet-shell-client-protocol.h > > tablet-shell-protocol.c > > text-client-protocol.h > > diff --git a/clients/Makefile.am b/clients/Makefile.am > > index 8c9bcd4..5f83acd 100644 > > --- a/clients/Makefile.am > > +++ b/clients/Makefile.am > > @@ -81,6 +81,8 @@ libtoytoolkit_la_SOURCES = \ > > window.h \ > > text-cursor-position-protocol.c \ > > text-cursor-position-client-protocol.h \ > > + subsurface-protocol.c \ > > + subsurface-client-protocol.h \ > > workspaces-protocol.c \ > > workspaces-client-protocol.h > > > > @@ -185,6 +187,8 @@ BUILT_SOURCES = \ > > desktop-shell-protocol.c \ > > tablet-shell-client-protocol.h \ > > tablet-shell-protocol.c \ > > + subsurface-client-protocol.h \ > > + subsurface-protocol.c \ > > workspaces-client-protocol.h \ > > workspaces-protocol.c > > > > diff --git a/clients/window.h b/clients/window.h > > index c2946d8..815b3f1 100644 > > --- a/clients/window.h > > +++ b/clients/window.h > > @@ -27,6 +27,7 @@ > > #include <wayland-client.h> > > #include <cairo.h> > > #include "../shared/config-parser.h" > > +#include "subsurface-client-protocol.h" > > > > #define ARRAY_LENGTH(a) (sizeof (a) / sizeof (a)[0]) > > > > 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 _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
