Sub-surface protocol

Kristian Høgsberg hoegsberg at gmail.com
Thu Dec 13 09:33:33 PST 2012


On Thu, Dec 13, 2012 at 04:07:44PM +0100, John Kåre Alsaker wrote:
> I added buffered commit_surface in wl_surface_group, which allows
> clients to atomically update the surfaces of their choice. This way we
> don't have to change wl_surface.commit. We can also update the main or
> parent surface and subsurfaces independently.
> 
> <interface name="wl_surface_group_factory" version="1">
> 	<request name="create_surface_group">
> 		<description summary="remove a surface from the group">
> 			This creates a new wl_surface_group with 'main_surface' as the main surface.
> 			Moving the main surface will move all member surfaces too.
> 			Only the main surface can have an associated wl_shell_surface or
> other shell interfaces.
> 		</description>
> 		<arg name="new_surface_group" type="new_id" interface="wl_surface_group"/>
> 		<arg name="main_surface" type="object" interface="wl_surface"/>
> 	</request>
> </interface>
> 
> <interface name="wl_surface_group" version="1">
> 	<request name="destroy" type="destructor">
> 	</request>
> 
> 	<request name="remove">
> 		<description summary="remove a surface from the group">
> 			This removes a surface from the group. The main surface cannot be removed.
> 			
> 			This won't take effect until wl_surface_group.commit is called.
> 		</description>
> 		<arg name="surface" type="object" interface="wl_surface"/>
> 	</request>
> 
> 	<request name="set_position">
> 		<description summary="remove a surface from the group">
> 			This sets the position of a member surface relative to the main surface.
> 			
> 			This won't take effect until wl_surface_group.commit is called.
> 		</description>
> 		<arg name="surface" type="object" interface="wl_surface"/>
> 		<arg name="x" type="int"/>
> 		<arg name="y" type="int"/>
> 	</request>
> 
> 	<request name="place_above">
> 		<description summary="place a surface above another">
> 			This places 'surface' right above 'relative_to_surface'.
> 			If 'surface' isn't a member of this group, it will become a member.
> 			It is an error to add a surface belonging to another wl_surface_group.
> 			
> 			This won't take effect until wl_surface_group.commit is called.
> 		</description>
> 		<arg name="surface" type="object" interface="wl_surface"/>
> 		<arg name="relative_to_surface" type="object" interface="wl_surface"/>
> 	</request>
> 
> 	<request name="place_below">
> 		<description summary="place a surface below another">
> 			This places 'surface' right below 'relative_to_surface'.
> 			If 'surface' isn't a member of this group, it will become a member.
> 			It is an error to add a surface belonging to another wl_surface_group.
> 			
> 			This won't take effect until wl_surface_group.commit is called.
> 		</description>
> 		<arg name="surface" type="object" interface="wl_surface"/>
> 		<arg name="relative_to_surface" type="object" interface="wl_surface"/>
> 	</request>
> 	
> 	<request name="commit_surface">
> 		<description summary="commit a surface">
> 			This calls wl_surface.commit on the surface.
> 			
> 			This won't take effect until wl_surface_group.commit is called.
> 		</description>
> 		<arg name="surface" type="object" interface="wl_surface"/>
> 	</request>
> 	
> 	<request name="commit">
> 		<description summary="execute all pending requests">
> 			This executes all pending remove, place_above, place_below,
> set_position and commit_surface requests atomically.
> 		</description>
> 	</request>
> </interface>
> 
> On Thu, Dec 13, 2012 at 3:30 PM, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> > Hi John,
> >
> >
> > On Thu, 13 Dec 2012 14:51:17 +0100
> > John Kåre Alsaker <john.kare.alsaker at gmail.com> wrote:
> >
> >> Here is my "subsurface" proposal. I don't like video sinks (or other
> >> things) running on an independent framerate. I don't want to maintain
> >> more state in the compositor side for them or have increased
> >> complexity in the protocol. They are inefficient and can be solved by
> >> a number of other ways. In those cases you can change the API, clients
> >> can buffer the video sink state for themselves (which you should be
> >> able to do if you're porting to Wayland), or the worst case, render
> >> them off-screen and let the client draw them as they please.
> >
> > Ok, that is the reason you chose that sub-surface commits are no-op,
> > right?
> >
> >> My proposal uses a wl_surface_group which is a group of wl_surfaces
> >> with a defined Z-order and positions relative to the main surface. It
> >> has a concept of a main surface which is the surface which acts as the
> >> wl_surface_group for other APIs like wl_shell_surface. Member surfaces
> >> cannot have a wl_shell_surface. main_surface is created in the
> >> create_surface_group request so it can have a different implementation
> >> (which probably won't happen in Weston).
> >
> > My thoughts exactly, just slightly different objects in the protocol.
> > I'm hooking the sub-surface role into the weston_surface::configure
> > pointer, which automatically excludes all other roles for a surface.
> >
> >> <interface name="wl_surface_group_factory" version="1">
> >>       <request name="create_surface_group">
> >>               <description summary="remove a surface from the group">
> >>                       This creates a new wl_surface_group with 'main_surface' as the main surface.
> >>                       The commit for the main surface commits all pending place_above,
> >> place_below, set_position,
> >>                       remove requests and pending state for all member surfaces. Doing
> >> commit on member surface will be a no-op.
> >>                       Moving the main surface will move all member surfaces too.
> >>                       Only the main surface can have an associated wl_shell_surface or
> >> other shell interfaces.
> >>               </description>
> >>               <arg name="new_surface_group" type="new_id" interface="wl_surface_group"/>
> >>               <arg name="main_surface" type="new_id" interface="wl_surface"/>
> >>       </request>
> >> </interface>
> >>
> >> <interface name="wl_surface_group" version="1">
> >>       <request name="destroy" type="destructor">
> >>       </request>
> >>
> >>       <request name="remove">
> >>               <description summary="remove a surface from the group">
> >>                       This removes a surface from the group. The main surface cannot be removed.
> >>               </description>
> >>               <arg name="surface" type="object" interface="wl_surface"/>
> >>       </request>
> >>
> >>       <request name="set_position">
> >>               <description summary="remove a surface from the group">
> >>                       This sets the position of a member surface relative to the main surface.
> >>               </description>
> >>               <arg name="surface" type="object" interface="wl_surface"/>
> >>               <arg name="x" type="int"/>
> >>               <arg name="y" type="int"/>
> >>       </request>
> >>
> >>       <request name="place_above">
> >>               <description summary="place a surface above another">
> >>                       This places 'surface' right above 'relative_to_surface'.
> >>                       If 'surface' isn't a member of this group, it will become a member.
> >>                       It is an error to add a surface belonging to another wl_surface_group.
> >>               </description>
> >>               <arg name="surface" type="object" interface="wl_surface"/>
> >>               <arg name="relative_to_surface" type="object" interface="wl_surface"/>
> >>       </request>
> >>
> >>       <request name="place_below">
> >>               <description summary="place a surface below another">
> >>                       This places 'surface' right below 'relative_to_surface'.
> >>                       If 'surface' isn't a member of this group, it will become a member.
> >>                       It is an error to add a surface belonging to another wl_surface_group.
> >>               </description>
> >>               <arg name="surface" type="object" interface="wl_surface"/>
> >>               <arg name="relative_to_surface" type="object" interface="wl_surface"/>
> >>       </request>
> >> </interface>
> >
> > This looks very much like what I am working with, with the exceptions:
> > - instead of wl_surface_group, each sub-surface has a wl_subsurface
> >   object, whose creation defines also the parent.
> > - the requests you have in wl_surface_group, I have in wl_subsurface,
> >   practically identical otherwise
> > - what you call main surface, I call parent
> I prefer the group terminology and it also avoids having a
> wl_subsurface for each subsurface.

The wl_subsurface (as well as wl_shell_surface) is a deliberate design
choice.  It's easy to avoid, but the key is that creating a new
protocol object for this role, will give the server side
implementation the per-surface sub-surface data directly in the
incoming args.  So instead of having to attach user data to wl_surface
and then look it up from the generic wl_surface, the protocol request
handler will receive a wl_subsurface object with whatever subsurface
specific state we need.  In other words, we save a lookup (which would
be a linear loop through the wl_surface destroy listeners) and let the
object ID mapping code do it (which is a constant time array lookup).

>From a more conceptual point of view, you can also argue that
set_position, place_above etc are operations on the subsurface, and
the remove request just becomes subsurface.destroy.

Kristian

> >
> > The description in your create_surface_group is pretty much the same
> > semantics I am implementing, with the difference in sub-surface commit
> > behaviour.
> >
> > I have my version in a WIP branch:
> > http://cgit.collabora.com/git/user/pq/wayland.git/tree/protocol/wayland.xml?h=subsurface-wip#n1358
> > There are still some semantics to be explained more. It's a simple
> > addition to protocol, but the semantics and implementation are
> > non-trivial the least.
> >
> >> I see that in weston the shell is very glad in inspecting geometry. We
> >> also have to alpha property per-surface instead of per
> >> wl_shell_surface. For fading windows we may want to compose the
> >> wl_surface_group into a single image.
> >
> > I don't think we want to do intermediate composites, unless we can put
> > it into a plane. Otherwise it's just extra data movement. It seems to
> > be very easy to hook a compound window into the input and repaint
> > procedures by just adding all the wl_surfaces into the big surface
> > list created during weston_output_repaint(). I don't have to revamp
> > either input nor repaint code. I actually got that part working right
> > now.
> >
> > For alpha, we can easily enough change the surface alpha for all...
> > wait, that would not really work right. We'd have to manipulate the
> > repaint regions, too. Maybe that's not even enough, oh well.
> We have to compose it or we'll see through parts that's supposed to be
> opaque (I think Cinnamon does that for it's menu, it looks odd).

Yeah, that's a good point.

Kristian


More information about the wayland-devel mailing list