[PATCH wayland-protocols v2 1/7] xdg-shell: Turn xdg_surface into a generic base interface
Jonas Ådahl
jadahl at gmail.com
Wed May 18 02:48:24 UTC 2016
On Tue, May 17, 2016 at 01:35:43PM -0500, Yong Bakos wrote:
> On May 11, 2016, at 12:49 AM, 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>
> > Signed-off-by: Mike Blumenkrantz <zmike at samsung.com>
>
> Hi Jonas and Mike,
> Some feedback inline below. Forgive my ignorance, but is the goal
> of the xdg-shell protocol extension to extract/remove the wl_shell
> and wl_shell_surface stuff from the core Wayland protocol?
wl_shell/wl_shell_surface should be considered deprecated, except we
don't have anything stable to replace it with. It was just a proof of
concept of how to actually get something on the screen.
xdg_shell aims to be that replacement.
I'm not sure if we can actually remove wl_shell and friends later, but
we might at least be able to mark it as deprecated (if we add such a
meaning to the XML).
>
> I realize my confusions noted below may be due to my lack of
> experience here.
>
> Thank you,
> yong
>
>
> > ---
> >
> > Changes since v1:
> >
> > - moved xdg_surface based role semantics into xdg_surface
> > - reworded xdg_toplevel description a bit
> > - various minor doc changes
> >
> >
> > Jonas
> >
> >
> > unstable/xdg-shell/xdg-shell-unstable-v6.xml | 271 ++++++++++++++++-----------
> > 1 file changed, 161 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..4080119 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.
> > @@ -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.,
> > + 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,142 @@
> > 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.
> > </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">
> > + <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
>
> Missing closing period.
>
>
> > + </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 once, it is not
>
> is set,
>
>
> > + 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.
>
> Perhaps "unset geometry" instead of "unset mode."
Maybe we should just drop that sentence. It doesn't add anything more
than an opinion.
>
>
> > +
> > + The arguments are given in the surface local coordinate space of
>
> surface-local
>
>
> > + the wl_surface associated with this xdg_surface.
> > +
> > + The width and height must be greater than zero.
>
> Perhaps it's worth noting whether or not negative values are ignored or
> raise a protocol error.
Good point. FWIW, negative x/y values are allowed, but we should state
that they are either clamped to the subsurface tree extents or raises
error if it goes outside.
>
>
> > + </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.
>
> This is a little confusing, since the serial is used to relate the configure
> event with the ack. Or am I misunderstanding that the serial here is of some
> "user event" and not the serial of the configure event itself?
What is it exactly you find confusing? FWIW, it has nothing to do with
user events.
What the last sentence here is about is to say that the state double
buffered, i.e. that on wl_surface.commit, the last
xdg_surface.ack_configure is the one that will be used by the
compositor. For example the compositor may send N number of configure
events before the client decides to draw the next frame; when the client
decides to draw, it'll take the last received configure event, and use
that to determine how to draw, and use the serial number from that event
in the ack_configure request.
>
>
> > + </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, 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"/>
>
> Makes sense but a summary for the serial number here would help clarify (per my
> confusion above).
Not sure exactly what would be a description of the serial, except of
how it should be passed around. Maybe replacing the "Clients should ..."
paragraph with this?
The serial number configure represents a specific configuration
notification from the compositor. Clients should arrange their
surface given the configuration notification, and reply with an
ack_configure request with the serial number sent in the
configure event before committing the new surface state.
>
>
> > + </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_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 +270,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 +463,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.
> >
> > The width and height arguments specify a hint to the window
> > about how its surface should be resized in window geometry
> > @@ -364,81 +481,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. See
>
> in response to this event.
>
>
> > + 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"/>
>
> Doesn't the serial arg need to be here in the protocol, despite this configure
> event 'extending' the xdg_surface.configure event? I may be overlooking how
> this event precedes the final xdg_surface.configure event, which does
> have the serial param and gets ack'd by the client. But then, the description here,
> "Clients must send an ack_configure in response to this," wouldn't be right, would it?
> Seems clients just ack the final xdg_surface.configure event and not each individual
> configure event?
No. The configure events of the sub-xdg-surface interfaces
(xdg_toplevel, xdg_popup) "extend" the xdg_surface.configure event,
meaning they will never ever be sent alone. When a client receives for
example an xdg_toplevel.configure event, it should store the received
state in a "pending" data structure, and then act on it when the
xdg_surface.configure event is received.
In effect, a client must send an ack_configure response to this request,
as semantically, should be combined with xdg_surface.configer considered
one "configuration notification". Given this, I suppose only it is
enough to document this in xdg_surface.configure; this part here was
just in order to make it clear that it needs a response.
Maybe it can be changed to make that more clear.. maybe something like:
This is part of a configuration notification by the compositor
that clients needs to respond to using ack_configure. See
xdg_surface.configure and xdg_surface.ack_configure for details.
?
Thanks a lot for your review, it is much appreciated. The parts I didn't
reply to I will fix locally.
Jonas
>
>
> > </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 +498,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 +539,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 +682,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 +704,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.
> > --
> > 2.5.5
> >
> > _______________________________________________
> > wayland-devel mailing list
> > wayland-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
More information about the wayland-devel
mailing list