On May 28, 2016, at 9:51 AM, Mike Blumenkrantz <michael.blumenkra...@gmail.com> wrote: > > On Sat, May 28, 2016 at 9:40 AM Yong Bakos <j...@humanoriented.com > <mailto:j...@humanoriented.com>> wrote: > Hi Mike, > Regarding the combination of type="array" enum="foo"... > >> On May 27, 2016, at 12:30 PM, Mike Blumenkrantz >> <michael.blumenkra...@gmail.com <mailto:michael.blumenkra...@gmail.com>> >> wrote: >> >> I've inlined some replies below. >> >> On Fri, May 27, 2016 at 1:13 PM Yong Bakos <j...@humanoriented.com >> <mailto:j...@humanoriented.com>> wrote: >> On May 27, 2016, at 10:29 AM, Mike Blumenkrantz <zm...@osg.samsung.com >> <mailto: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 >> > <mailto:zm...@osg.samsung.com>> >> > Signed-off-by: Jonas Ådahl <jad...@gmail.com <mailto: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 >> >> "which" is correct and intended in this case. >> >> >> > + 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 >> >> Line break was intentional: documentation will not generate a line break, >> but anyone reading the protocol file will see it. >> >> >> > + 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) >> >> Other state enums begin at 1. >> >> >> > + <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 >> >> Using "a" would imply that the drop shadow is a singular unit instead of a >> combination of multiple shadow regions--a possible case. >> >> >> > + 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 >> >> "which" is correct and intended in this case. >> >> >> > + 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 was discussed, and the resulting decision was that implementations >> would be simplified if the default value never got passed here. >> >> >> > + >> > + 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 >> >> "which" is correct and intended in this case. >> >> >> > + 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?") >> >> It's possible explicit required states may be added in the future; your >> proposed change here would go against the main idea of this patch, i.e., not >> passing required states. >> >> >> > + 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) >> >> Same as above. >> >> >> > + >> > + 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? >> >> Good catch. This also affects the event above it. > > As we discussed via IRC (27 May), the scanner will choke on this. While we > talked about > making a change to the scanner to allow this, perhaps such a change doesn't > make sense. > > Given a type="array", scanner will generate a parameter of type wl_array. > > Perhaps the short story here is to just remove the enum from this arg, and > the similar > arg in the configure_draw_states event above. What do you think? > > Yes, I supposed that my previous reply would indicate my plan to do this, but > it seems I should have been more explicit. This is a trivial change, so I am > not going to create a v2 thread for it. > > > (I wonder if it's the DTD that can change, so the scanner's validation step > will catch the unsupported combination of type="array" enum="foo". My gut > tells me that > DTDs don't support this logic, but I'll dig into this.) > > I think it would be useful to allow the annotation, even if there was no > difference in the scanner output. Anyone who reads the xml directly would > find it useful.
Ah, that's a good point. I'll address the scanner change as planned this week. Thank you, yong > > > yong > > >> >> >> > + </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
