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 <ppaala...@gmail.com> wrote: > > Hi John, > > > > > > On Thu, 13 Dec 2012 14:51:17 +0100 > > John Kåre Alsaker <john.kare.alsa...@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 _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel