[PATCH] xdg-shell: add draw states for xdg_surface

[Pekka Paalanen]

On Sat, 28 May 2016 08:39:59 -0500
Yong Bakos <junk at humanoriented.com> wrote:

> Hi Mike,
> Regarding the combination of type="array" enum="foo"...
> 
> > On May 27, 2016, at 12:30 PM, Mike Blumenkrantz <michael.blumenkrantz at gmail.com> wrote:
> > 
> > I've inlined some replies below.
> > 
> > On Fri, May 27, 2016 at 1:13 PM Yong Bakos <junk at humanoriented.com <mailto:junk at humanoriented.com>> wrote:
> > On May 27, 2016, at 10:29 AM, Mike Blumenkrantz <zmike at osg.samsung.com <mailto:zmike at osg.samsung.com>> wrote:  
> > >
> > > this adds a method for compositors to change various draw attributes
> > > for a surface
> > >
> > > Signed-off-by: Mike Blumenkrantz <zmike at osg.samsung.com <mailto:zmike at osg.samsung.com>>
> > > Signed-off-by: Jonas Ådahl <jadahl at gmail.com <mailto:jadahl at 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

> > > +
> > > +        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?
> 
> (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.)

Hi,

here is some background.

A type="array" argument is really just a binary blob of data. The XML
description, human documentation aside, does not specify anything about
the blob contents. Therefore adding an XML attribute pointing to an
enum definition is half-useless. Generators could use it for creating
automatic links in documentation, but it cannot be used for code
generation, because you don't know the types contained in the blob.

We also do not want to add blob content type definitions to the XML
language, because you might want to have everything C is able to
express, including nested structs. There is also no requirement that
the "array" is really an array - every "element" could be a different
thing. It could be bitstream and whatnot. Only the use of
wl_array_for_each() implies it is an array of similar elements,
wl_array_add() does not.

The big point in adding enum annotations was that language bindings
generators (other than wayland-scanner) could use the annotation for
code generation. I don't think it is possible with the array type.

If we allow enum annotation with the array type, it will only be usable
for doc links, unlike the other enum annotations.

OTOH, we have lots and lots of places in the documentation texts that
refer to some request, event, interface, etc. that would be useful as a
hyperlink in the generated docs. Enums could fall into that as well, so
we would not need the attribute for only documentation.

Auke, Nils, what's your take on this matter?

We do have some documentation about enums in
https://wayland.freedesktop.org/docs/html/ch04.html#sect-Protocol-Basic-Principles



Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160530/6cf26f32/attachment.sig>