[PATCH wayland-protocols 1/3] xdg-shell: Turn xdg_surface into a generic base interface
Jonas Ådahl
jadahl at gmail.com
Fri Jan 15 18:20:13 PST 2016
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 <jadahl at 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 <jadahl at 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>
>
More information about the wayland-devel
mailing list