[PATCH weston 09/10] xdg-shell: Rewrite documentation
Pekka Paalanen
ppaalanen at gmail.com
Fri Feb 27 11:37:06 UTC 2015
On Fri, 13 Feb 2015 14:02:01 +0800
Jonas Ådahl <jadahl at gmail.com> wrote:
> From: "Jasper St. Pierre" <jstpierre at mecheye.net>
>
> This rewrites basically all of the text inside xdg-shell to be up to
> date, clearer, and rid of wl_shell and X11 terminology.
>
> [jadahl: Added paragraph about popup surface mapping order.]
> ---
> protocol/xdg-shell.xml | 262 +++++++++++++++++++++++++++++--------------------
> 1 file changed, 157 insertions(+), 105 deletions(-)
Hi,
since this is said to be a complete documentation rewrite, I am not
reviewing this as a patch. Instead, I review the final xdg-shell.xml
file as it will be after this whole patch series.
I did check this patch does not change any interface signatures.
> <?xml version="1.0" encoding="UTF-8"?>
> <protocol name="xdg_shell">
>
> <copyright>
> Copyright © 2008-2013 Kristian Høgsberg
> Copyright © 2013 Rafael Antognolli
> Copyright © 2013 Jasper St. Pierre
> Copyright © 2010-2013 Intel Corporation
>
> Permission to use, copy, modify, distribute, and sell this
> software and its documentation for any purpose is hereby granted
> without fee, provided that the above copyright notice appear in
> all copies and that both that copyright notice and this permission
> notice appear in supporting documentation, and that the name of
> the copyright holders not be used in advertising or publicity
> pertaining to distribution of the software without specific,
> written prior permission. The copyright holders make no
> representations about the suitability of this software for any
> purpose. It is provided "as is" without express or implied
> warranty.
>
> THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
> SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
> FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY
> SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN
> AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION,
> ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF
> THIS SOFTWARE.
> </copyright>
>
> <interface name="xdg_shell" version="1">
> <description summary="create desktop-style surfaces">
> xdg_shell allows clients to turn a wl_surface into a "real window"
> which can be dragged, resized, stacked, and moved around by the
> user. Everything about this interface is suited towards traditional
> desktop environments.
> </description>
>
> <enum name="version">
> <description summary="latest protocol version">
> The 'current' member of this enum gives the version of the
> protocol. Implementations can compare this to the version
> they implement using static_assert to ensure the protocol and
> implementation versions match.
Could add, that this enum will go away once the xdg-shell protocol
becomes stable.
> </description>
> <entry name="current" value="5" summary="Always the latest version"/>
> </enum>
>
> <enum name="error">
> <entry name="role" value="0" summary="given wl_surface has another role"/>
> </enum>
>
> <request name="destroy" type="destructor">
> <description summary="destroy xdg_shell">
> Destroy this xdg_shell object.
>
> Destroying a bound xdg_shell object while there are surfaces
> still alive with roles from this interface is illegal and will
> result in a protocol error. Make sure to destroy all surfaces
> before destroying this object.
What about multiple binds to xdg_shell global? If a client has
different components sharing the Wayland connection but using separate
wl_registries, they may have separate xdg_shell objects.
- Is it forbidden to destroy any xdg_shell objects while any xdg_*
roles are in use?
- Is it forbidden to destroy the last xdg_shell object while any xdg_*
roles are in use?
- Or, is the server supposed to track which xdg_* roles were created
with which xdg_shell object, and allow destruction of an xdg_shell
object only when the roles created from that specific object are all
gone?
The question about tracking extends also to the ping/pong messaging.
> </description>
> </request>
>
> <request name="use_unstable_version">
> <description summary="enable use of this unstable version">
> Negotiate the unstable version of the interface. This
> mechanism is in place to ensure client and server agree on the
> unstable versions of the protocol that they speak or exit
> cleanly if they don't agree. This request will go away once
> the xdg-shell protocol is stable.
> </description>
> <arg name="version" type="int"/>
> </request>
>
> <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. See the documentation of xdg_surface for more details.
> </description>
> <arg name="id" type="new_id" interface="xdg_surface"/>
> <arg name="surface" type="object" interface="wl_surface"/>
Missing specification of error conditions and codes. So far there is
only the role error. (The xdg_surface interface does not seem to define
errors either.)
> </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. See the documentation of xdg_popup for more details.
>
> This request must be used in response to some sort of user action
> like a button press, key press, or touch down event.
> </description>
> <arg name="id" type="new_id" interface="xdg_popup"/>
> <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"/>
Missing specification of error conditions and codes.
However, I see you have error codes defined in the xdg_popup interface.
IIRC, that is a novel convention, where you need to create the new
wl_resource before sending a protocol error on *that* rather than on
the object whose request caused the error.
I would prefer to have the errors defined in the interface whose
request raises them, but I suppose this could work too.
> </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
> serial specified in the event back to the compositor by sending
> a "pong" request back with the specified serial.
>
> Compositors can use this to determine if the client is still
> alive. It's unspecified what will happen if the client doesn't
> respond to the ping request, or in what timeframe. Clients should
> try to respond in a reasonable amount of time.
> </description>
> <arg name="serial" type="uint" summary="pass this to the pong request"/>
Should servers ping all xdg_shell objects of a client or just one?
> </event>
>
> <request name="pong">
> <description summary="respond to a ping event">
> A client must respond to a ping event with a pong request or
> the client may be deemed unresponsive.
> </description>
> <arg name="serial" type="uint" summary="serial of the ping event"/>
> </request>
> </interface>
>
> <interface name="xdg_surface" version="1">
> <description summary="A desktop window">
> 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.
Is it explained somewhere how the window will become visible, i.e. mapped?
Is it required to have a commit on the wl_surface after giving the
xdg_surface role?
This also relates to the whole window state initialization sequence
before mapping it, which has been discussed several times wrt. tiling
WMs and whatnot.
> </description>
>
> <request name="destroy" type="destructor">
> <description summary="Destroy the xdg_surface">
> 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.
> </description>
> </request>
>
> <request name="set_parent">
> <description summary="set the parent of this surface">
> Set the "parent" of this surface. This window should be stacked
> above a parent. The parent surface must be mapped as long as this
> surface is mapped.
>
> Parent windows should be set on dialogs, toolboxes, or other
> "auxilliary" surfaces, so that the parent is raised when the dialog
> is raised.
> </description>
> <arg name="parent" type="object" interface="xdg_surface" allow-null="true"/>
> </request>
>
> <request name="set_title">
> <description summary="set surface title">
> Set a short title for the surface.
>
> This string may be used to identify the surface in a task bar,
> window list, or other user interface elements provided by the
> compositor.
>
> The string must be encoded in UTF-8.
> </description>
> <arg name="title" type="string"/>
> </request>
>
> <request name="set_app_id">
> <description summary="set application ID">
> Set an application identifier for the surface.
>
> The app ID identifies the general class of applications to which
> the surface belongs. The compositor can use this to group multiple
> applications together, or to determine how to launch a new
> application.
>
> See the desktop-entry specification [0] for more details on
> application identifiers and how they relate to well-known DBus
> names and .desktop files.
>
> [0] http://standards.freedesktop.org/desktop-entry-spec/
> </description>
> <arg name="app_id" type="string"/>
Okay, but what actually is the app_id?
Is it the same as the DESKTOP_STARTUP_ID in desktop-entry-spec?
Is it the "desktop file id"?
I went through the desktop-entry-spec, and I couldn't see any other
ids, but I have a vague recollection this might actually be a DBus
err... address? Like org.gnome.gedit or something.
An example from e.g. org.freedesktop namespace would be nice, and
mentioning if it indeed is a DBus something for apps that support DBus.
> </request>
>
> <request name="show_window_menu">
> <description summary="show the window menu">
> Clients implementing client-side decorations might want to show
> a context menu when right-clicking on the decorations, giving the
> user a menu that they can use to maximize or minimize the window.
>
> This request asks the compositor to pop up such a window menu at
> the given position, relative to the local surface coordinates of
> the parent surface. There are no guarantees as to what menu items
> the window menu contains.
>
> This request must be used in response to some sort of user action
> like a button press, key press, or touch down event.
> </description>
>
> <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" summary="the x position to pop up the window menu at"/>
> <arg name="y" type="int" summary="the y position to pop up the window menu at"/>
> </request>
>
> <request name="move">
> <description summary="start an interactive move">
> Start an interactive, user-driven move of the surface.
>
> This request must be used in response to some sort of user action
> like a button press, key press, or touch down event.
>
> The server may ignore move requests depending on the state of
> the surface (e.g. fullscreen or maximized).
> </description>
> <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"/>
Okay, is the serial used to detect which input device (type) will be
used for the interactive move? Is it so that the serial will precisely
determine the device type (pointer, kbd, touch, ...)?
Would the client need to know when the move ends? How?
> </request>
>
> <enum name="resize_edge">
> <description summary="edge values for resizing">
> These values are used to indicate which edge of a surface
> is being dragged in a resize operation. The server may
> use this information to adapt its behavior, e.g. choose
> an appropriate cursor image.
Do you assume the server will control the cursor image while resizing?
What if the application uses custom cursors?
Are there any cases where the server *needs* to control the cursor
image while resizing, why not just let the app update the cursor?
I thought the main reason for this enum is that the server knows which
corner of the window should be the anchor, but you say nothing about
that. Yet, AFAIK it is needed if the client doesn't use attach dx,dy to
control the resizing direction, and I recall no-one liking the dx,dy
method.
> </description>
> <entry name="none" value="0"/>
> <entry name="top" value="1"/>
> <entry name="bottom" value="2"/>
> <entry name="left" value="4"/>
> <entry name="top_left" value="5"/>
> <entry name="bottom_left" value="6"/>
> <entry name="right" value="8"/>
> <entry name="top_right" value="9"/>
> <entry name="bottom_right" value="10"/>
> </enum>
>
> <request name="resize">
> <description summary="start an interactive resize">
> Start a user-driven, interactive resize of the surface.
>
> This request must be used in response to some sort of user action
> like a button press, key press, or touch down event.
>
> The server may ignore resize requests depending on the state of
> the surface (e.g. fullscreen or maximized).
> </description>
> <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="edges" type="uint" summary="which edge or corner is being dragged"/>
The same questions as for "move": how do you pick the input device
type, how do you know it finished?
> </request>
>
> <enum name="state">
> <description summary="types of state on the surface">
> The different state values used on the surface. This is designed for
> state values like maximized, fullscreen. It is paired with the
> configure event to ensure that both the client and the compositor
> setting the state can be synchronized.
>
> States set in this way are double-buffered. They will get applied on
> the next commit.
>
> Desktop environments may extend this enum by taking up a range of
> values and documenting the range they chose in this description.
> They are not required to document the values for the range that they
> chose. Ideally, any good extensions from a desktop environment should
> make its way into standardization into this enum.
>
> The current reserved ranges are:
>
> 0x0000 - 0x0FFF: xdg-shell core values, documented below.
> 0x1000 - 0x1FFF: GNOME
> </description>
> <entry name="maximized" value="1" summary="the surface is maximized">
> The surface is maximized. The window geometry specified in the configure
> event must be obeyed by the client.
> </entry>
> <entry name="fullscreen" value="2" summary="the surface is fullscreen">
> The surface is fullscreen. The window geometry specified in the configure
> event must be obeyed by the client.
If the client disagrees on e.g. aspect ratio, is the client responsible
for the black bars? If it was not, then the compositor might be able to
realize the black bars by adjusting scanout parameters, which would be
better than the compositor or the client compositing the black bars
into the image.
> </entry>
> <entry name="resizing" value="3">
> The surface is being resized. The window geometry specified in the
> configure event is a maximum; the client cannot resize beyond it.
> Clients that have aspect ratio or cell sizing configuration can use
> a smaller size, however.
> </entry>
> <entry name="activated" value="4">
> Client window decorations should be painted as if the window is
> active. Do not assume this means that the window actually has
> keyboard or pointer focus.
> </entry>
> </enum>
>
> <event name="configure">
> <description summary="suggest a surface change">
> The configure event asks the client to resize its surface or to
> change its state.
>
> The width and height arguments specify a hint to the window
> about how its surface should be resized in window geometry
> coordinates.
Add: "See set_window_geometry."
>
> The states listed in the event specify how the width/height
> 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.
> </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 a ack_configure request before the commit request,
> passing along the serial of the configure event.
>
> The compositor might use this information to move a surface
For instance, ...
> 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.
> </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.
>
> 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 width and height must be greater than zero.
Very good. Might want to add that the arguments are given in the
surface-local coordinates of the wl_surface associated with this
xdg_surface specifically.
> </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" />
> <request name="unset_maximized" />
Should these be explained a bit?
>
> <request name="set_fullscreen">
> <description summary="set the window as fullscreen on a monitor">
> Make the surface fullscreen.
>
> You can specify an output that you would prefer to be fullscreen.
> If this value is NULL, it's up to the compositor to choose which
> display will be used to map this surface.
> </description>
> <arg name="output" type="object" interface="wl_output" allow-null="true"/>
wl_shell has a lot more about fullscreening than this. I think we
should still review that stuff before xdg-shell is stabilized to see if
we need anything more here.
> </request>
> <request name="unset_fullscreen" />
>
> <request name="set_minimized">
> <description summary="set the window as minimized">
> Request that the compositor minimize your surface. There is no
> way to know if the surface is currently minimized, nor is there
> any way to unset minimization on this surface.
>
> If you are looking to throttle redrawing when minimized, please
> instead use the wl_surface.frame event for this, as this will
> also work with live previews on windows in Alt-Tab, Expose or
> similar compositor features.
> </description>
> </request>
>
> <event name="close">
> <description summary="surface wants to be closed">
> The close event is sent by the compositor when the user
> wants the surface to be closed. This should be equivalent to
> the user clicking the close button in client-side decorations,
> if your application has any...
>
> This is only a request that the user intends to close your
> window. The client may choose to ignore this request, or show
> a dialog to ask the user to save their data...
> </description>
> </event>
> </interface>
>
> <interface name="xdg_popup" version="1">
> <description summary="short-lived, popup surfaces for menus">
> A popup surface is a short-lived, temporary surface that can be
> used to implement menus. It takes an explicit grab on the surface
> that will be dismissed when the user dismisses the popup. This can
> be done by the user clicking outside the surface, using the keyboard,
> or even locking the screen through closing the lid or a timeout.
>
> When the popup is dismissed, a popup_done event will be sent out,
> and at the same time the surface will be unmapped. The xdg_popup
> object is now inert and cannot be reactivated, so clients should
> destroy it. Explicitly destroying the xdg_popup object will also
> dismiss the popup and unmap the surface.
>
> Clients will receive events for all their surfaces during this
> grab (which is an "owner-events" grab in X11 parlance). This is
> done so that users can navigate through submenus and other
> "nested" popup windows without having to dismiss the topmost
> popup.
>
> Clients that want to dismiss the popup when another surface of
> 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.
>
> Specifying an xdg_popup for the parent means that the popups are
> nested, with this popup now being the topmost popup. Nested
> popups must be destroyed in the reverse order they were created
> in, e.g. the only popup you are allowed to destroy at all times
> is the topmost one.
>
> If there is an existing popup when creating a new popup, the
> parent must be the current topmost popup.
>
> A parent surface must be mapped before the new popup is mapped.
>
> When compositors choose to dismiss a popup, they will likely
> dismiss every nested popup as well.
I suppose compositors must also follow the same dismissing order as
clients.
>
> The x and y arguments specify where the top left of the popup
> should be placed, relative to the local surface coordinates of the
> parent surface.
Add: "See xdg_shell.get_xdg_popup."
> </description>
>
> <enum name="error">
> <description summary="xdg_popup error values">
> These errors can be emitted in response to xdg_popup requests.
> </description>
> <entry name="not_the_topmost_popup" value="0" summary="The client tried to map or destroy a non-toplevel popup"/>
> <entry name="invalid_parent" value="1" summary="The client specified an invalid parent surface"/>
> </enum>
>
> <request name="destroy" type="destructor">
> <description summary="remove xdg_popup interface">
> This destroys the popup. Explicitly destroying the xdg_popup
> object will also dismiss the popup, and unmap the surface.
>
> If this xdg_popup is not the "topmost" popup, a protocol error
> will be sent.
> </description>
> </request>
>
> <event name="popup_done">
> <description summary="popup interaction is done">
> The popup_done event is sent out when a popup is dismissed
> by the compositor.
Add: "The client should destroy the xdg_popup object as it has become
inert." or something.
> </description>
> </event>
>
> </interface>
> </protocol>
Ok, all my comments are mostly just additions, nothing too bad. Also
considering that v5 has already landed elsewhere, there's nothing else
to say for the patch than:
Reviewed-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
These comments should be an inspiration for follow-up patches rather
than revising this patch. Adding KDE developers to CC.
Please, bear in mind that we are trying to lay down the minimal basic
desktop protocol so that apps can work at all. Once we agree on a solid
basis, we can then add more features without breaking everything in
the process. The goal for now should be stabilizing the minimal feature
set, while making sure we can add features later. The extendability of
Wayland ensures most of the latter.
Thanks,
pq
More information about the Plasma-devel
mailing list