<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">On May 28, 2016, at 9:51 AM, Mike Blumenkrantz <<a href="mailto:michael.blumenkrantz@gmail.com" class="">michael.blumenkrantz@gmail.com</a>> wrote:<br class=""><div><blockquote type="cite" class=""><br class=""><div class=""><div dir="ltr" class=""><div class="gmail_quote"><div dir="ltr" class="">On Sat, May 28, 2016 at 9:40 AM Yong Bakos <<a href="mailto:junk@humanoriented.com" class="">junk@humanoriented.com</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class="">Hi Mike,<div class="">Regarding the combination of type="array" enum="foo"...</div><div class=""><br class=""><div class=""></div></div></div><div style="word-wrap:break-word" class=""><div class=""><div class=""><blockquote type="cite" class=""><div class="">On May 27, 2016, at 12:30 PM, Mike Blumenkrantz <<a href="mailto:michael.blumenkrantz@gmail.com" target="_blank" class="">michael.blumenkrantz@gmail.com</a>> wrote:</div><br class=""><div class=""><div dir="ltr" class="">I've inlined some replies below.<br class=""><br class=""><div class="gmail_quote"><div dir="ltr" class="">On Fri, May 27, 2016 at 1:13 PM Yong Bakos <<a href="mailto:junk@humanoriented.com" target="_blank" class="">junk@humanoriented.com</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On May 27, 2016, at 10:29 AM, Mike Blumenkrantz <<a href="mailto:zmike@osg.samsung.com" target="_blank" class="">zmike@osg.samsung.com</a>> wrote:<br class=""> ><br class=""> > this adds a method for compositors to change various draw attributes<br class=""> > for a surface<br class=""> ><br class=""> > Signed-off-by: Mike Blumenkrantz <<a href="mailto:zmike@osg.samsung.com" target="_blank" class="">zmike@osg.samsung.com</a>><br class=""> > Signed-off-by: Jonas Ådahl <<a href="mailto:jadahl@gmail.com" target="_blank" class="">jadahl@gmail.com</a>><br class=""> <br class=""> Hi Mike & Jonas,<br class=""> A question about communicating default state, and some<br class=""> minor nits you can certainly ignore, inline below.<br class=""> <br class=""> <br class=""> > ---<br class=""> > unstable/xdg-shell/xdg-shell-unstable-v6.xml | 69 ++++++++++++++++++++++++++++<br class=""> > 1 file changed, 69 insertions(+)<br class=""> ><br class=""> > diff --git a/unstable/xdg-shell/xdg-shell-unstable-v6.xml b/unstable/xdg-shell/xdg-shell-unstable-v6.xml<br class=""> > index dfd7e84..0fa76d4 100644<br class=""> > --- a/unstable/xdg-shell/xdg-shell-unstable-v6.xml<br class=""> > +++ b/unstable/xdg-shell/xdg-shell-unstable-v6.xml<br class=""> > @@ -843,6 +843,75 @@<br class=""> > </description><br class=""> > </request><br class=""> ><br class=""> > + <enum name="draw_state"><br class=""> > + <description summary="draw states for the surface"><br class=""> > + The draw state enum defines optional states which describe how<br class=""> <br class=""> nit: that describe<br class=""></blockquote><div class=""><br class=""></div><div class="">"which" is correct and intended in this case.</div><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <br class=""> > + a client should draw a surface. A client must at least support the<br class=""> > + default state, and support for optional draw states is explicitly<br class=""> > + advertised using xdg_toplevel.set_available_draw_states.<br class=""> > +<br class=""> > + The default draw state implies that the client draws the surface<br class=""> > + with complete window decorations.<br class=""> <br class=""> nit: Same paragraph, so remove line break<br class=""></blockquote><div class=""><br class=""></div><div class="">Line break was intentional: documentation will not generate a line break, but anyone reading the protocol file will see it.</div><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <br class=""> > + This may include, e.g., window frame and drop shadow.<br class=""> > +<br class=""> > + Each draw state defines an alteration to the default. Some draw<br class=""> > + states may be combined, while some are mutually exclusive. See<br class=""> > + each draw state for details.<br class=""> > +<br class=""> > + Desktop environments may extend this enum by taking up a range of<br class=""> > + values and documenting the range they chose in this description.<br class=""> > + They are not required to document the values for the range that they<br class=""> > + chose. Ideally, any good extensions from a desktop environment should<br class=""> <br class=""> nit: extension (singular)<span style="line-height:1.5" class=""> </span></blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <br class=""> > + make its way into standardization into this enum.<br class=""> > +<br class=""> > + The current reserved ranges are:<br class=""> > +<br class=""> > + 0x0000 - 0x0FFF: xdg-shell core values, documented below.<br class=""> > + 0x1000 - 0x1FFF: EFL<br class=""> > + </description><br class=""> <br class=""> should there be a 0 entry in the enum for default?<br class=""> (see my comment re: empty array below)<br class=""></blockquote><div class=""><br class=""></div><div class="">Other state enums begin at 1.</div><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <br class=""> > + <entry name="no_shadow" value="1" summary="no dropshadow"><br class=""> > + <description summary="the surface without a dropshadow"><br class=""> > + The "no_shadow" draw state implies that the client must not draw<br class=""> <br class=""> nit: not draw a<br class=""></blockquote><div class=""><br class=""></div><div class="">Using "a" would imply that the drop shadow is a singular unit instead of a combination of multiple shadow regions--a possible case.</div><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <br class=""> > + drop shadow around the surface. This may have side effects<br class=""> > + on usability, e.g., the inability to activate client-initiated<br class=""> > + interactive resize.<br class=""> > + </description><br class=""> > + </entry><br class=""> > + </enum><br class=""> > +<br class=""> > + <event name="configure_draw_states"><br class=""> > + <description summary="set the draw state(s) of the surface"><br class=""> > + Set the draw state(s) which the client should use to draw a given<br class=""> <br class=""> nit: that the client should<br class=""></blockquote><div class=""><br class=""></div><div class="">"which" is correct and intended in this case.<br class=""></div><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <br class=""> > + surface. The absence of this event prior to an xdg_surface.configure<br class=""> > + event indicates that no change has occurred in the draw state since the<br class=""> > + previous xdg_surface.configure.<br class=""> > +<br class=""> > + Sending an empty array of states with this method resets a surface to the<br class=""> > + default draw state.<br class=""> <br class=""> Would it not be more explicit for compositors to pass a "default" enum value rather<br class=""> than an empty array? (Assuming there is a default in the draw_state enum, per my<br class=""> comment above.)<br class=""> But, you will definitely know better than I!<br class=""></blockquote><div class=""><br class=""></div><div class="">This was discussed, and the resulting decision was that implementations would be simplified if the default value never got passed here.</div><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <br class=""> > +<br class=""> > + This event is not sent by itself but as a latched state sent prior to<br class=""> > + the xdg_surface.configure event. When received, a client should adapt<br class=""> > + the drawing of the surface according to the state and respond to the<br class=""> > + configure event accordingly. See xdg_surface.ack_configure for<br class=""> > + details.<br class=""> > +<br class=""> > + A compositor will only configure a client to draw with optional states on a<br class=""> > + given surface using the states which were advertised by that surface using<br class=""> <br class=""> nit: that were advertised<br class=""></blockquote><div class=""><br class=""></div><div class="">"which" is correct and intended in this case.<br class=""></div><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <br class=""> > + xdg_toplevel.set_available_draw_states.<br class=""> > + </description><br class=""> > + <arg name="states" type="array" enum="draw_state"/><br class=""> > + </event><br class=""> > +<br class=""> > + <request name="set_available_draw_states"><br class=""> > + <description summary="advertise optional draw states for the window"><br class=""> <br class=""> nit: advertise available draw states<br class=""> Seems clearer, as there's no separation between "available" and "optional," since<br class=""> all draw states are optional. (Not being consistent here makes the reader second-<br class=""> guess, "are there available ones and optional ones?")<br class=""></blockquote><div class=""><br class=""></div><div class="">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.</div><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <br class=""> > + Inform the compositor of optional draw states which are available<br class=""> > + for the xdg_toplevel.<br class=""> <br class=""> nit: of available draw states for the xdg_toplevel.<br class=""> (same as previous reason)<br class=""></blockquote><div class=""><br class=""></div><div class="">Same as above.</div><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <br class=""> > +<br class=""> > + Calling this after an xdg_toplevel's first commit will raise a client error.<br class=""> > + </description><br class=""> > + <arg name="states" type="array" enum="draw_state"/><br class=""> <br class=""> Just a sanity check, since I haven't seen it in a protocol spec yet. Does scanner handle<br class=""> this combination of array and enum correctly?<br class=""></blockquote><div class=""><br class=""></div><div class="">Good catch. This also affects the event above it.</div></div></div></div></blockquote><div class=""><br class=""></div></div></div></div><div style="word-wrap:break-word" class=""><div class=""><div class=""><div class="">As we discussed via IRC (27 May), the scanner will choke on this. While we talked about</div><div class="">making a change to the scanner to allow this, perhaps such a change doesn't make sense.</div><div class=""><br class=""></div><div class="">Given a type="array", scanner will generate a parameter of type wl_array.</div><div class=""><br class=""></div><div class="">Perhaps the short story here is to just remove the enum from this arg, and the similar</div><div class="">arg in the configure_draw_states event above. What do you think?</div></div></div></div></blockquote><div class=""><br class=""></div><div class="">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.</div><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class=""><div class=""><div class=""><div class=""><br class=""></div><div class="">(I wonder if it's the DTD that can change, so the scanner's validation step</div><div class="">will catch the unsupported combination of type="array" enum="foo". My gut tells me that</div><div class="">DTDs don't support this logic, but I'll dig into this.)</div></div></div></div></blockquote><div class=""><br class=""></div><div class="">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.</div></div></div></div></blockquote><div><br class=""></div><div>Ah, that's a good point. I'll address the scanner change as planned this week.</div><div><br class=""></div><div>Thank you,</div><div>yong</div><div><br class=""></div><div><br class=""></div><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_quote"><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class=""><div class=""><div class=""><div class=""><br class=""></div><div class="">yong</div></div></div></div><div style="word-wrap:break-word" class=""><div class=""><div class=""><div class=""><br class=""></div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_quote"><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <br class=""> > + </request><br class=""> > +<br class=""> > <event name="configure"><br class=""> > <description summary="suggest a surface change"><br class=""> > This configure event asks the client to resize its toplevel surface or<br class=""> > --<br class=""> > 2.5.5<br class=""> <br class=""> yong<br class=""></blockquote></div></div></div></blockquote></div></div></div></blockquote></div></div> </div></blockquote></div><br class=""></body></html>