On Wed, Jun 08, 2016 at 04:45:28PM +0200, Benoit Gschwind wrote: > Hello Jonas, > > On 08/06/2016 03:18, Jonas Ådahl wrote: > > On Wed, Jun 08, 2016 at 12:37:01AM +0200, Benoit Gschwind wrote: > >> 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 <jad...@gmail.com> > >>> Signed-off-by: Mike Blumenkrantz <zm...@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. > > > > I think it helps describing the expectations here. > > Then something like that: > > xdg_surface are used as basis to define a role to a given surface, such > as xdg_toplevel or xdg_popup. It's also the basis of the management of > xdg's surfaces states.
Sure, that sounds reasonable. Jonas > > > > >> > >>> 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 ... > > > > Sure. > > > >> > >>> + 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. > > > > Sure, that makes it a bit clearer. > > > >> > >>> + </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. > > > > Yea, lets that would be better as a follow up. > > > >> > >> > >>> + > >>> + 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. > > > > Yes, that sounds better. > > > >> > >>> + > >>> + 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. > > > > Yes, except I would assume a client wouldn't "freeze" anything, but > > would rather accumulate the changes in a "pending state" container > > similar to how the compositor handles surface state changes before > > wl_surface.commit. > > > >> > >>> + > >>> + 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. > > > > Maybe "The configured state should not be 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 > > > > Thanks for taking a look > > > > > > Jonas > > > >> -- > >> Benoit Gschwind > >> > > Best regards > -- > Benoit Gschwind _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel