[PATCH wayland-protocols v3 1/7] xdg-shell: Turn xdg_surface into a generic base interface

Benoit Gschwind gschwind at gnu-log.net
Tue Jun 7 22:37:01 UTC 2016


Hello,

This proposal look good, few optional comments following.

On 26/05/2016 06:32, Jonas Ådahl 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>
> Signed-off-by: Mike Blumenkrantz <zmike at samsung.com>
> ---
> 
> Changes since v2:
>   - Grammar, typos and spelling fixes
>   - Tried to clarify the latched state configure events sequence
>   - Clarified window geometry constraints and semantics
> 
> 
>  unstable/xdg-shell/xdg-shell-unstable-v6.xml | 275 ++++++++++++++++-----------
>  1 file changed, 165 insertions(+), 110 deletions(-)
> 
> diff --git a/unstable/xdg-shell/xdg-shell-unstable-v6.xml b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
> index ce57153..fa838f9 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.
>  

I think the 3 line above are more confusing than useful and may be
replaced by the single sentence:
This creates an xdg_surface for the given surface.

>  	See the documentation of xdg_surface for more details about what an
>  	xdg_surface is and how it is used.
> @@ -67,29 +65,6 @@
>        <arg name="surface" type="object" interface="wl_surface"/>
>      </request>
>  
> -    <request name="get_xdg_popup">
> -      <description summary="create a popup for a surface">
> -	This creates an xdg_popup for the given surface and gives it the
> -	xdg_popup role. A wl_surface can only be given an xdg_popup role
> -	once. If get_xdg_popup is called with a wl_surface that already has
> -	an active xdg_popup associated with it, or if it had any other role,
> -	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="surface" type="object" interface="wl_surface"/>
> -      <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>
> -
>      <event name="ping">
>        <description summary="check if the client is alive">
>  	The ping event asks the client if it's still alive. Pass the
> @@ -117,13 +92,23 @@
>    </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 are split into
> +      xdg_surface roles.
> +
> +      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.,

In order to map an xdg_surface, the client must create a role-specific ...

> +      get_toplevel, get_popup. The wl_surface for any given xdg_surface can
> +      have at most one role, and may not be assigned any role not based on
> +      xdg_surface.
> +
> +      A role must be assigned before any other requests are made to the
> +      xdg_surface object.
>  
>        The client must call wl_surface.commit on the corresponding wl_surface
>        for the xdg_surface state to take effect.
> @@ -133,12 +118,146 @@
>        manipulate a buffer prior to the first xdg_surface.configure call must
>        also be treated as errors.
>  
> -      For a surface to be mapped by the compositor the client must have
> -      committed both an xdg_surface state and a buffer.
> +      For a surface to be mapped by the compositor the client must have assigned
> +      one of the xdg_surface based roles, it must have committed both the
> +      xdg_surface state and the role dependent state, and it must have committed
> +      a buffer.

For a surface to be mapped by the compositor, the following conditions
must be met: (1) the client has assigned a xdg_surface based role to the
surface, (2) the client has set the xdg_surface state and the role
dependent state to the surface and (3) the client has committed a buffer
to the surface.

> +    </description>
> +
> +    <enum name="error">
> +      <entry name="not_constructed" value="1"/>
> +      <entry name="already_constructed" value="2"/>
> +    </enum>
> +
> +    <request name="destroy" type="destructor">
> +      <description summary="destroy the xdg_surface">
> +	Destroy the xdg_surface object. An xdg_surface must 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.
> +
> +	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.
> +
> +	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, 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 is meant for extremely simple clients.

This sentence was from the previous spec. but I would use :

This defaults behavior is meant for simplest clients.

maybe could be done in another patch.


> +
> +	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. Setting an invalid size
> +	will raise an error. When applied, the effective window geometry will be
> +	the set window geometry clamped to the bounding rectangle of the
> +	combined geometry of the surface of the xdg_surface and the associated
> +	subsurfaces.
> +      </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.
> +
> +	A client may send multiple ack_configure requests before committing, but
> +	only the last request sent before a commit indicates which configure
> +	event the client really 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 marks the end of a configure sequence. A configure
> +	sequence is a set of zero or more events configuring the drawing of the
> +	surface, finalized by this event.

A configure sequence is a set of one or more events configuring the
state of the xdg_surface, including the final xdg_surface.configure event.

> +
> +	Where applicable, xdg_surface surface roles will during a configure
> +	sequence 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.

I tried to figure out what the paragraph above explain. Here is my
rewording, If I'm wrong please correct myself and feel free to pick my
wording to improve this paragraph.

The client must freeze the state of a window until a configure sequence
is completed. During the configure sequence the client accumulate state
changes. When and only when the client receive a xdg_surface.configure
event, the client can apply accumulated changes of the surface state.
During the configure sequence, a newer events override all previous
event of the same type.

> +
> +	Clients should arrange their surface for the new states, and then send
> +	an 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" summary="serial of the configure event"/>
> +    </event>
> +  </interface>
> +
> +  <interface name="zxdg_toplevel_v6" version="1">
> +    <description summary="toplevel surface">
> +      This interface defines an xdg_surface role which allows a surface to,
> +      among other things, set window-like properties such as maximize,
> +      fullscreen, and minimize, set application-specific metadata like title and
> +      id, and well as trigger user interactive operations such as interactive
> +      resize and move.
>      </description>
>  
>      <request name="destroy" type="destructor">
> -      <description summary="Destroy the xdg_surface">
> +      <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.
> @@ -155,7 +274,7 @@
>  	"auxiliary" surfaces, so that the parent is raised when the dialog
>  	is raised.
>        </description>
> -      <arg name="parent" type="object" interface="zxdg_surface_v6" allow-null="true"/>
> +      <arg name="parent" type="object" interface="zxdg_toplevel_v6" allow-null="true"/>
>      </request>
>  
>      <request name="set_title">
> @@ -348,8 +467,10 @@
>  
>      <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 but as a latched state
> +	sent prior to the xdg_surface.configure event. See xdg_surface.configure
> +	for details.

This configure event asks the client to resize or to change states of
the client surface. This event is not applied immediately, see
xdg_surface.configure for details.

>  
>  	The width and height arguments specify a hint to the window
>  	about how its surface should be resized in window geometry
> @@ -364,81 +485,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 must send an ack_configure in response to this event. 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_max_size">
>        <description summary="set the maximum size">
>  	Set a maximum size for the window.
> @@ -447,7 +502,7 @@
>  	not try to configure the window beyond this size.
>  
>  	The width and height arguments are in window geometry coordinates.
> -	See set_window_geometry.
> +	See xdg_surface.set_window_geometry.
>  
>  	Values set in this way are double-buffered. They will get applied
>  	on the next commit.
> @@ -488,7 +543,7 @@
>  	not try to configure the window below this size.
>  
>  	The width and height arguments are in window geometry coordinates.
> -	See set_window_geometry.
> +	See xdg_surface.set_window_geometry.
>  
>  	Values set in this way are double-buffered. They will get applied
>  	on the next commit.
> @@ -631,7 +686,7 @@
>        their own is clicked should dismiss the popup using the destroy
>        request.
>  
> -      The parent surface must have either an xdg_surface 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
> @@ -653,7 +708,7 @@
>        The x and y arguments passed when creating the popup object specify
>        where the top left of the popup should be placed, relative to the
>        local surface coordinates of the parent surface. See
> -      xdg_shell.get_xdg_popup.
> +      xdg_surface.get_popup.
>  
>        The client must call wl_surface.commit on the corresponding wl_surface
>        for the xdg_popup state to take effect.
> 

Best regards
--
Benoit Gschwind



More information about the wayland-devel mailing list