On Fri, Jan 15, 2016 at 09:02:57PM -0500, Mike Blumenkrantz wrote: > Hi, > > I have some suggestions which I've inlined below:
All fine points (except the sneaky whitespace in the bottom, that was actually intentional since a sneaky whitespace is in the top as well :P). And sorry for missing adding your Signed-off-by (as this patch was a result of shedcraft session a while back). Will send a new version with wording improvements. Jonas > > On Tue, 12 Jan 2016 16:16:47 +0800 > Jonas Ådahl <jad...@gmail.com> wrote: > > > Split out toplevel window like requests and events into a new interface > > called xdg_toplevel, and turn xdg_surface into a generic base interface > > which others extends. > > > > xdg_popup is changed to extend the xdg_surface. > > > > The configure event in xdg_surface was split up making > > xdg_surface.configure an event only carrying the serial number, while a > > new xdg_toplevel.configure event carries the other data previously sent > > via xdg_surface.configure. xdg_toplevel.configure is made to extend, > > via the latch-state mechanism, xdg_surface.configure and depends on > > that event to synchronize state. > > > > Other future xdg_surface based extensions are meant to also extend > > xdg_surface.configure for relevant window type dependend state > > synchronization. > > > > Signed-off-by: Jonas Ådahl <jad...@gmail.com> > > --- > > unstable/xdg-shell/xdg-shell-unstable-v6.xml | 229 > > +++++++++++++++++---------- > > 1 file changed, 148 insertions(+), 81 deletions(-) > > > > diff --git a/unstable/xdg-shell/xdg-shell-unstable-v6.xml > > b/unstable/xdg-shell/xdg-shell-unstable-v6.xml > > index 2b028c0..d1c315e 100644 > > --- a/unstable/xdg-shell/xdg-shell-unstable-v6.xml > > +++ b/unstable/xdg-shell/xdg-shell-unstable-v6.xml > > @@ -54,11 +54,9 @@ > > > > <request name="get_xdg_surface"> > > <description summary="create a shell surface from a surface"> > > - This creates an xdg_surface for the given surface and gives it the > > - xdg_surface role. A wl_surface can only be given an xdg_surface role > > - once. If get_xdg_surface is called with a wl_surface that already has > > - an active xdg_surface associated with it, or if it had any other role, > > - an error is raised. > > + This creates an xdg_surface for the given surface. While xdg_surface > > + itself is not a role, the corresponding surface may only be assigned > > + a role extending xdg_surface, such as xdg_toplevel or xdg_popup. > > > > See the documentation of xdg_surface for more details about what an > > xdg_surface is and how it is used. > > @@ -117,13 +115,18 @@ > > </interface> > > > > <interface name="zxdg_surface_v6" version="1"> > > - <description summary="A desktop window"> > > + <description summary="desktop user interface surface base interface"> > > An interface that may be implemented by a wl_surface, for > > implementations that provide a desktop-style user interface. > > > > - It provides requests to treat surfaces like windows, allowing to set > > - properties like maximized, fullscreen, minimized, and to move and > > resize > > - them, and associate metadata like title and app id. > > + It provides a base set of functionality required to construct user > > + interface elements requiring management by the compositor, such as > > + toplevel windows, menus, etc. The types of functionality is split > > into xdg > > + surface roles. > > + > > + Initially, an xdg_surface cannot be mapped, as the underlying > > wl_surface > > + has not been given any role. To give it a role, use one of the > > functions > > + creating xdg surface role specific objects: get_toplevel, get_popup. > > I think this could be rephrased as something like > > "Creating an xdg_surface does not set the role for a wl_surface. In order to > map > an xdg_surface, create a role-specific object using, e.g., get_toplevel, > get_popup. > The wl_surface for any given xdg_surface can have at most one role." > > > > > The client must call wl_surface.commit on the corresponding > > wl_surface > > for the xdg_surface state to take effect. > > @@ -139,6 +142,134 @@ > > > > <request name="destroy" type="destructor"> > > <description summary="Destroy the xdg_surface"> > > + Destroy the xdg_surface object. Before destroying the xdg_surface, any > > + xdg surface role object must have been previously destroyed. > > Perhaps "An xdg_surface can only be destroyed after its role object has been > destroyed" ? > > > + </description> > > + </request> > > + > > + <request name="get_toplevel"> > > + <description summary="assign the xdg_toplevel surface role"> > > + This creates an xdg_toplevel object for the given xdg_surface and gives > > + the associated wl_surface the xdg_toplevel role. A wl_surface can only > > + have one xdg_toplevel role. If the wl_surface is given the xdg_toplevel > > + role while it already has an active xdg_toplevel role, or if it has been > > + given any other role before, an error is raised. > > Stating the restriction on number of roles for an xdg_surface in the > xdg_surface part > would make this implicit for all of the roles. I think this would make future > additions > simpler. > > > + > > + See the documentation of xdg_toplevel for more details about what an > > + xdg_toplevel is and how it is used. > > + </description> > > + <arg name="id" type="new_id" interface="zxdg_toplevel_v6"/> > > + </request> > > + > > + <request name="get_popup"> > > + <description summary="assign the xdg_popup surface role"> > > + This creates an xdg_popup object for the given xdg_surface and gives the > > + associated wl_surface the xdg_popup role. A wl_surface can only have one > > + xdg_popup role. If the wl_surface is given the xdg_popup role while it > > + already has an active xdg_popup role, or if it has been given any other > > + role before, an error is raised. > > + > > + This request must be used in response to some sort of user action like a > > + button press, key press, or touch down event. > > + > > + See the documentation of xdg_popup for more details about what an > > + xdg_popup is and how it is used. > > + </description> > > + <arg name="id" type="new_id" interface="zxdg_popup_v6"/> > > + <arg name="parent" type="object" interface="wl_surface"/> > > + <arg name="seat" type="object" interface="wl_seat" summary="the > > wl_seat of the user event"/> > > + <arg name="serial" type="uint" summary="the serial of the user > > event"/> > > + <arg name="x" type="int"/> > > + <arg name="y" type="int"/> > > + </request> > > + > > + <request name="set_window_geometry"> > > + <description summary="set the new window geometry"> > > + The window geometry of a surface is its "visible bounds" from the > > + user's perspective. Client-side decorations often have invisible > > + portions like drop-shadows which should be ignored for the > > + purposes of aligning, placing and constraining windows. > > + > > + The window geometry is double buffered, and will be applied at the > > + time wl_surface.commit of the corresponding wl_surface is called. > > + > > + Once the window geometry of the surface is set once, it is not > > + possible to unset it, and it will remain the same until > > + set_window_geometry is called again, even if a new subsurface or > > + buffer is attached. > > + > > + If never set, the value is the full bounds of the surface, > > + including any subsurfaces. This updates dynamically on every > > + commit. This unset mode is meant for extremely simple clients. > > + > > + The arguments are given in the surface local coordinate space of > > + the wl_surface associated with this xdg_surface. > > + > > + The width and height must be greater than zero. > > + </description> > > + <arg name="x" type="int"/> > > + <arg name="y" type="int"/> > > + <arg name="width" type="int"/> > > + <arg name="height" type="int"/> > > + </request> > > + > > + <request name="ack_configure"> > > + <description summary="ack a configure event"> > > + When a configure event is received, if a client commits the > > + surface in response to the configure event, then the client > > + must make an ack_configure request sometime before the commit > > + request, passing along the serial of the configure event. > > + > > + For instance, for toplevel surfaces the compositor might use this > > + information to move a surface to the top left only when the client has > > + drawn itself for the maximized or fullscreen state. > > + > > + If the client receives multiple configure events before it > > + can respond to one, it only has to ack the last configure event. > > + > > + A client is not required to commit immediately after sending > > + an ack_configure request - it may even ack_configure several times > > + before its next surface commit. > > + > > + The compositor expects that the most recently received > > + ack_configure request at the time of a commit indicates which > > + configure event the client is responding to. > > + </description> > > + <arg name="serial" type="uint" summary="the serial from the > > configure event"/> > > + </request> > > + > > + <event name="configure"> > > + <description summary="suggest a surface change"> > > + The configure event asks the client to configure the drawing of the > > + surface according to a given state. > > + > > + Where applicable, xdg surface surface roles will extend this event as a > > + latched state sent as events before the xdg_surface.configure event. > > + Such events should be considered to make up a set of atomically applied > > + configuration states, where the xdg_surface.configure commits the > > + accumulated state. > > + > > + Clients should arrange their surface for the new states, states, and > > + then send a ack_configure request with the serial sent in this configure > > + event at some point before committing the new surface. > > + > > + If the client receives multiple configure events before it can respond > > + to one, it is free to discard all but the last event it received. > > + </description> > > + <arg name="serial" type="uint"/> > > + </event> > > + </interface> > > + > > + <interface name="zxdg_toplevel_v6"> > > + <description summary="toplevel surface"> > > + This interface defines an xdg_surface role that provides requests to > > treat > > + surfaces like windows, allowing to set properties like maximized, > > + fullscreen, minimized, and to move and resize them, and associate > > metadata > > + like title and app id. > > "This interface defines an xdg_surface role which allows a surface to set > window-like > properties such as maximize, fullscreen, and minimize as well as > application-specific > metadata like title and id." ? > > > + </description> > > + > > + <request name="destroy" type="destructor"> > > + <description summary="destroy the xdg_toplevel"> > > Unmap and destroy the window. The window will be effectively > > hidden from the user's point of view, and all state like > > maximization, fullscreen, and so on, will be lost. > > @@ -348,8 +479,9 @@ > > > > <event name="configure"> > > <description summary="suggest a surface change"> > > - The configure event asks the client to resize its surface or to > > - change its state. > > + This configure event asks the client to resize its toplevel surface or > > + to change its state. It is not sent by itself, and should be consider to > > + be a latched state to the xdg_surface.configure event. > > "It is not sent by itself and is in a latched state with the > xdg_surface.configure event" ? > > > > > The width and height arguments specify a hint to the window > > about how its surface should be resized in window geometry > > @@ -364,81 +496,15 @@ > > arguments should be interpreted, and possibly how it should be > > drawn. > > > > - Clients should arrange their surface for the new size and > > - states, and then send a ack_configure request with the serial > > - sent in this configure event at some point before committing > > - the new surface. > > - > > - If the client receives multiple configure events before it > > - can respond to one, it is free to discard all but the last > > - event it received. > > + Clients need to send an ack_configure in response to this. See > > + xdg_surface.configure and xdg_surface.ack_configure for detalis. > > "Client must send an ack_configure in response to this. > See xdg_surface.configure and xdg_surface.ack_configure for details." > > > </description> > > > > <arg name="width" type="int"/> > > <arg name="height" type="int"/> > > <arg name="states" type="array"/> > > - <arg name="serial" type="uint"/> > > </event> > > > > - <request name="ack_configure"> > > - <description summary="ack a configure event"> > > - When a configure event is received, if a client commits the > > - surface in response to the configure event, then the client > > - must make an ack_configure request sometime before the commit > > - request, passing along the serial of the configure event. > > - > > - For instance, the compositor might use this information to move > > - a surface to the top left only when the client has drawn itself > > - for the maximized or fullscreen state. > > - > > - If the client receives multiple configure events before it > > - can respond to one, it only has to ack the last configure event. > > - > > - A client is not required to commit immediately after sending > > - an ack_configure request - it may even ack_configure several times > > - before its next surface commit. > > - > > - The compositor expects that the most recently received > > - ack_configure request at the time of a commit indicates which > > - configure event the client is responding to. > > - </description> > > - <arg name="serial" type="uint" summary="the serial from the > > configure event"/> > > - </request> > > - > > - <request name="set_window_geometry"> > > - <description summary="set the new window geometry"> > > - The window geometry of a window is its "visible bounds" from the > > - user's perspective. Client-side decorations often have invisible > > - portions like drop-shadows which should be ignored for the > > - purposes of aligning, placing and constraining windows. > > - > > - The window geometry is double buffered, and will be applied at the > > - time wl_surface.commit of the corresponding wl_surface is called. > > - > > - Once the window geometry of the surface is set once, it is not > > - possible to unset it, and it will remain the same until > > - set_window_geometry is called again, even if a new subsurface or > > - buffer is attached. > > - > > - If never set, the value is the full bounds of the surface, > > - including any subsurfaces. This updates dynamically on every > > - commit. This unset mode is meant for extremely simple clients. > > - > > - If responding to a configure event, the window geometry in here > > - must respect the sizing negotiations specified by the states in > > - the configure event. > > - > > - The arguments are given in the surface local coordinate space of > > - the wl_surface associated with this xdg_surface. > > - > > - The width and height must be greater than zero. > > - </description> > > - <arg name="x" type="int"/> > > - <arg name="y" type="int"/> > > - <arg name="width" type="int"/> > > - <arg name="height" type="int"/> > > - </request> > > - > > <request name="set_maximized"> > > <description summary="maximize the window"> > > Maximize the surface. > > @@ -549,8 +615,8 @@ > > their own is clicked should dismiss the popup using the destroy > > request. > > > > - The parent surface must have either an xdg_surface or xdg_popup > > - role. > > + The parent surface must have either have the surface role > > xdg_toplevel or > > + xdg_popup. > > "The parent surface must have either the xdg_toplevel or xdg_popup surface > role." > > > > > Specifying an xdg_popup for the parent means that the popups are > > nested, with this popup now being the topmost popup. Nested > > @@ -599,4 +665,5 @@ > > </event> > > > > </interface> > > + > > Trying to sneak in some trailing whitespace, I see! > > > </protocol> > _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel