Hi,

I have some suggestions which I've inlined below:

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

Reply via email to