On May 27, 2016, at 10:29 AM, Mike Blumenkrantz <zm...@osg.samsung.com> wrote: > > this adds a method for compositors to change various draw attributes > for a surface > > Signed-off-by: Mike Blumenkrantz <zm...@osg.samsung.com> > Signed-off-by: Jonas Ådahl <jad...@gmail.com>
Hi Mike & Jonas, A question about communicating default state, and some minor nits you can certainly ignore, inline below. > --- > unstable/xdg-shell/xdg-shell-unstable-v6.xml | 69 ++++++++++++++++++++++++++++ > 1 file changed, 69 insertions(+) > > diff --git a/unstable/xdg-shell/xdg-shell-unstable-v6.xml > b/unstable/xdg-shell/xdg-shell-unstable-v6.xml > index dfd7e84..0fa76d4 100644 > --- a/unstable/xdg-shell/xdg-shell-unstable-v6.xml > +++ b/unstable/xdg-shell/xdg-shell-unstable-v6.xml > @@ -843,6 +843,75 @@ > </description> > </request> > > + <enum name="draw_state"> > + <description summary="draw states for the surface"> > + The draw state enum defines optional states which describe how nit: that describe > + a client should draw a surface. A client must at least support the > + default state, and support for optional draw states is explicitly > + advertised using xdg_toplevel.set_available_draw_states. > + > + The default draw state implies that the client draws the surface > + with complete window decorations. nit: Same paragraph, so remove line break > + This may include, e.g., window frame and drop shadow. > + > + Each draw state defines an alteration to the default. Some draw > + states may be combined, while some are mutually exclusive. See > + each draw state for details. > + > + 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 nit: extension (singular) > + make its way into standardization into this enum. > + > + The current reserved ranges are: > + > + 0x0000 - 0x0FFF: xdg-shell core values, documented below. > + 0x1000 - 0x1FFF: EFL > + </description> should there be a 0 entry in the enum for default? (see my comment re: empty array below) > + <entry name="no_shadow" value="1" summary="no dropshadow"> > + <description summary="the surface without a dropshadow"> > + The "no_shadow" draw state implies that the client must not draw nit: not draw a > + drop shadow around the surface. This may have side effects > + on usability, e.g., the inability to activate client-initiated > + interactive resize. > + </description> > + </entry> > + </enum> > + > + <event name="configure_draw_states"> > + <description summary="set the draw state(s) of the surface"> > + Set the draw state(s) which the client should use to draw a given nit: that the client should > + surface. The absence of this event prior to an xdg_surface.configure > + event indicates that no change has occurred in the draw state since > the > + previous xdg_surface.configure. > + > + Sending an empty array of states with this method resets a surface > to the > + default draw state. Would it not be more explicit for compositors to pass a "default" enum value rather than an empty array? (Assuming there is a default in the draw_state enum, per my comment above.) But, you will definitely know better than I! > + > + This event is not sent by itself but as a latched state sent prior to > + the xdg_surface.configure event. When received, a client should adapt > + the drawing of the surface according to the state and respond to the > + configure event accordingly. See xdg_surface.ack_configure for > + details. > + > + A compositor will only configure a client to draw with optional > states on a > + given surface using the states which were advertised by that surface > using nit: that were advertised > + xdg_toplevel.set_available_draw_states. > + </description> > + <arg name="states" type="array" enum="draw_state"/> > + </event> > + > + <request name="set_available_draw_states"> > + <description summary="advertise optional draw states for the window"> nit: advertise available draw states Seems clearer, as there's no separation between "available" and "optional," since all draw states are optional. (Not being consistent here makes the reader second- guess, "are there available ones and optional ones?") > + Inform the compositor of optional draw states which are available > + for the xdg_toplevel. nit: of available draw states for the xdg_toplevel. (same as previous reason) > + > + Calling this after an xdg_toplevel's first commit will raise a > client error. > + </description> > + <arg name="states" type="array" enum="draw_state"/> Just a sanity check, since I haven't seen it in a protocol spec yet. Does scanner handle this combination of array and enum correctly? > + </request> > + > <event name="configure"> > <description summary="suggest a surface change"> > This configure event asks the client to resize its toplevel surface or > -- > 2.5.5 yong _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel