Re: Referencing enums in other protocols

[Auke Booij]

On Mon, Apr 23, 2018 at 9:08 AM, Pekka Paalanen  wrote:
> On Mon, 23 Apr 2018 03:42:31 -0400
> Simon Ser  wrote:
>
>> Hi all,
>>
>> While writing a protocol, I've been trying to reference symbols from another
>> protocol. While it seems to work fine for interfaces, it fails for enums:
>>
>> error: could not find enumeration wl_output.transform
>>
>> Is this by design? It seems odd to me that interfaces work but enums don't.
>
> Hi,
>
> I think that's an oversight. It should indeed be possible to reference
> enums just like interfaces from other protocol files.

From my point of view, we did not add this when we were working on
cross-referencing enums, in order to to keep the change as simple as
possible and easy to understand. Now that (apparently) there is a use
case, a solution would be welcome. But there are some problems to
solve. How do you specify the right version of an enum in another
file? What do you do if you want to refer to an enum in a protocol
file that is not available? At what point DO we check that the
cross-referenced enum exists? (Leaving it up to the protocol file
writer to do correctly sounds prone to error.) Etcetera.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: Skylane - implementation of Wayland in Rust

[Auke Booij]

On 5 May 2017 at 09:53, Pekka Paalanen  wrote:
> What could you do then, if you don't want to wrap the C implementation
> of libwayland...
>
> Borrowing the idea from Daniel Stone from whom I heard it first, the
> only other option is to reimplement libwayland *including* its C ABI in
> Rust. You need to completely throw away the C implementation of
> libwayland and replace that with your own libwayland-*.so built with
> Rust. Then you will get libEGL calling into your implementation and it
> will be the sole implementation, which should let things work.

This is exactly the approach I am taking with sudbury [1] (written in
Haskell, see also my 1st of April RFC). The client side is up and
running, and while I am aware of the further complications on the
server side, I think these are solvable.

[1] https://github.com/abooij/sudbury
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

[RFC wayland] move away from C

[Auke Booij]

The C programming language was developed in the early '70s. It was
always considered a proof of concept, as it was clear to everyone
involved that any serious programming lanugage should have a garbage
collector. Indeed, Dennis Ritchie is widely known to have said:

"What I would really like is to extend this prototype into a garbage
collected, multi-paradigm, modern programming language, supported by a
major player in the industry."

Of course, the language thus envisioned would later be implemented as
C pound (not to be confused with the cloud programming language #C).

Let's catch up to 1990 and translate major parts of wayland to the
language of the people - haskell.

Some early code can be found here:
https://github.com/abooij/sudbury
Existing wayland clients run without recompilation. Compositors are
not supported yet.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: array and enum attributes (Was: [PATCH] xdg-shell: add draw states for xdg_surface)

[Auke Booij]

On 1 June 2016 at 20:16, Yong Bakos  wrote:
> On May 30, 2016, at 3:54 AM, Pekka Paalanen  wrote:
>>
>> On Sat, 28 May 2016 08:39:59 -0500
>> Yong Bakos  wrote:
>>
>>> Hi Mike,
>>> Regarding the combination of type="array" enum="foo"...
>>>
 On May 27, 2016, at 12:30 PM, Mike Blumenkrantz 
  wrote:

 I've inlined some replies below.

 On Fri, May 27, 2016 at 1:13 PM Yong Bakos > wrote:
 On May 27, 2016, at 10:29 AM, Mike Blumenkrantz > wrote:
>
> this adds a method for compositors to change various draw attributes
> for a surface
>
> Signed-off-by: Mike Blumenkrantz  >
> Signed-off-by: Jonas Ådahl >

 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.
> +  
> +  

 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
>
> Pekka,
> Thank you for the info. Just so I understand your points correctly, let
> me assert that /just/ making a minor change to scanner to not error on
> the presence of both array and enum together does not have any major
> drawbacks.
>
> Correct?

Technically, it might be the case that nothing will necessarily break
*now*. But such a change would move us from very the currently
well-defined semantics of the enum attribute, into weird
documentation-only or half-defined specifications. This will be
confusing more often than it will be helpful.

If you want to refer to a specific enum, because your binary data
*could* be interpreted 

Re: [PATCH wayland v3] protocol: add support for cross-interface enum attributes

[Auke Booij]

On 3 May 2016 at 13:04, Pekka Paalanen <ppaala...@gmail.com> wrote:
> On Sat,  5 Dec 2015 12:39:12 +0000
> Auke Booij <a...@tulcod.com> wrote:
>
>> The enum attribute, for which scanner support was introduced in
>> 1771299, can be used to link message arguments to s. However,
>> some arguments refer to s in a different .
>>
>> This adds scanner support for referring to an  in a different
>>  using dot notation. It also sets the attributes in this
>> style in the wayland XML protocol (wl_shm_pool::create_buffer::format
>> to wl_shm::format, and wl_surface::set_buffer_transform::transform to
>> wl_output::transform), and updates the documentation XSL so that this
>> new style is supported.
>>
>> Changes since v2:
>>  - add object:: prefix for all enumerations in the documentation
>>  - fix whitespace in scanner.c
>>  - minor code fixup to return early and avoid casts in scanner.c
>>
>> Changes since v1:
>>  - several implementation bugs fixed
>>
>> Signed-off-by: Auke Booij <a...@tulcod.com>
>> ---
>>  doc/publican/protocol-to-docbook.xsl | 19 +--
>>  protocol/wayland.xml |  4 +--
>>  src/scanner.c| 63 
>> +++-
>>  3 files changed, 65 insertions(+), 21 deletions(-)
>>
>> diff --git a/doc/publican/protocol-to-docbook.xsl 
>> b/doc/publican/protocol-to-docbook.xsl
>> index fad207a..7e892c3 100644
>> --- a/doc/publican/protocol-to-docbook.xsl
>> +++ b/doc/publican/protocol-to-docbook.xsl
>> @@ -103,9 +103,22 @@
>>  
>>  
>>
>> -
>> -  
>> -
>> +
>> +  
>> +
>> +  
>> +  ::
>> +  
>> +
>> +  
>> +  
>> +
>> +  
>> +  ::
>> +  
>> +
>> +  
>> +
>>
>>
>>
>
> Hi,
>
> thanks for this.
>
> This XSL snippet does not work when the enum is defined in a different
> XML file, does it? Just making sure I know what the expected behaviour
> is.

Correct; I don't think this is worth implementing at this stage.

> Ok, there would be quite some things to polish, but since this patch
> has waited for so long so I just rebased the xsl hunk myself, made some
> whitespace fixes and pushed it:
>e21aeb5..7ccf35d  master -> master
>
> Please have a look at the merged patch in case I missed something.
>
> If you want to do more clean-ups, those can land after the alpha release.
>
>
> Thanks,
> pq

The patch you pushed looks correct; thanks!
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: Recursive dispatch (Re: [PATCH v2] server: Calculate remaining data size after a closure is processed)

[Auke Booij]

On 3 February 2016 at 09:34, Jonas Ådahl  wrote:
> On Wed, Feb 03, 2016 at 11:14:54AM +0200, Pekka Paalanen wrote:
>> On Wed, 3 Feb 2016 09:56:20 +0900
>> "Jaeyoon Jung"  wrote:
>>
>> > > -Original Message-
>> > > From: Derek Foreman [mailto:der...@osg.samsung.com]
>> > > Sent: Wednesday, February 03, 2016 6:56 AM
>> > > To: Pekka Paalanen; Jaeyoon Jung
>> > > Cc: 'Jonas Ådahl'; wayland-devel@lists.freedesktop.org
>> > > Subject: Re: Recursive dispatch (Re: [PATCH v2] server: Calculate 
>> > > remaining
>> > > data size after a closure is processed)
>> > >
>> > > On 02/02/16 06:57 AM, Pekka Paalanen wrote:
>> > > > On Tue, 12 Jan 2016 13:03:19 +0900 "Jaeyoon Jung"
>> > > >  wrote:
>> > > >
>> > > >>> -Original Message- From: wayland-devel
>> > > >>> [mailto:wayland-devel-boun...@lists.freedesktop.org] On Behalf
>> > > >>> Of Jonas Adahl Sent: Tuesday, January 12, 2016 12:27 PM To:
>> > > >>> Jaeyoon Jung Cc: wayland-devel@lists.freedesktop.org Subject:
>> > > >>> Re: [PATCH v2] server: Calculate remaining data size after a
>> > > >>> closure is processed
>> > > >>>
>> > > >>> On Tue, Jan 12, 2016 at 08:22:56AM +0900, Jaeyoon Jung wrote:
>> > > >>>
>> > >  When processing a closure, data in the connection can be
>> > >  consumed again if the closure itself invokes extra event
>> > >  dispatch. In that case the remaining data size is also
>> > >  altered, so the variable len should be updated after the
>> > >  closure is processed.
>> > > 
>> > >  Signed-off-by: Jaeyoon Jung 
>> > > >>>
>> > > >>> I don't like the name wl_connection_size
>> > > >>> (wl_connection_pending_input() or something would be more
>> > > >>> descriptive).
>> > > >>
>> > > >> I just wanted to have a shorter name.
>> > > >> wl_connection_pending_input() looks better to me as well, and
>> > > >> I'll update the patch accordingly. Thanks.
>> > > >>
>> > > >>
>> > > >>> A good follow up would be a test case where this would
>> > > >>> otherwise cause issues. I assume it would be triggered by a
>> > > >>> wl_event_loop_dispatch() in a interface request handler?
>> > > >>
>> > > >> Yes, exactly.
>> > > >
>> > > > Hi,
>> > > >
>> > > > could you humor me and explain in what sort of situations
>> > > > dispatching from within a request handler is useful?
>> >
>> > Suppose a server has a single event queue for all kind of events
>> > including wayland events. If the server wants to process more events
>> > from the queue in the middle of the request handler, and there is
>> > another wayland request arrived in the queue, it induces a recursive
>> > dispatch. I'm not saying that this is a good practice though.
>>
>> Right, this is a tautology: "the server wants to process more
>> events in the middle of a request handler" is the very definition of
>> recursive dispatch, not a justification for it.
>>
>>
>> > > > I thought recursive dispatch is a programming pattern that can
>> > > > only lead to painful and erratic problems later. Don't we consider
>> > > > this to be bad practice also in the client API?
>> > >
>> > > Is that actually documented somewhere?
>>
>> I don't think it is. It would be worth to make an official statement
>> about it in the docs.
>>
>> To me it is common sense (or expert knowledge) about event loops in
>> general.
>>
>> > > > If you do it in the server, that is like: this message causes more
>> > > > messages to be dispatched - how does that make sense? A server must
>> > > > not block waiting for any client message.
>> > >
>> > > I agree that a server blocking on a client message is a great way to
>> > > create a terrible user experience - but I'm not certain it's
>> > > libwayland's mandate to prevent it?
>>
>> Libwayland could prevent recursive dispatch because it has not been
>> intended to work. This patch proves that is actually was broken.
>>
>> Preventing recursive dispatch does not prevent writing a server that
>> blocks waiting on a client message. You can very well do that without
>> recursive dispatch, but it's just a little harder to write.
>>
>> > > > Also, inspecting future messages before finishing the processing of
>> > > > the current message is racy: the following messages may or may not
>> > > > be in the receive buffer and nothing guarantees either way, you
>> > > > just get lucky a lot of times. Wayland IPC does not guarantee more
>> > > > than one message to be delivered at a time, this is why we have
>> > > > things like wl_surface.commit request.
>> > >
>> > > This is a really good point. :)
>> > >
>> > > > If someone starts hardening libwayland against programmer errors,
>> > > > I'd expect one thing to be to abort() on recursive dispatch. Can
>> > > > you tell me why we should support recursive dispatch?
>> > >
>> > > IMHO we should either do that immediately (before the 1.10 release) or
>> > > revert this patch now.  Not that anything is wrong with the patch - it
>> 

Re: [PATCH wayland v2] client: Fully flush during blocking dispatch

[Auke Booij]

On 12 January 2016 at 04:31, Jonas Ådahl  wrote:
> wl_display_flush() may fail with EAGAIN which means that not all data
> waiting in the buffer has been flushed. We later block until there

+ is

> data to read, which could mean that we block on input from the
> compositor without having sent out all data from the client. Avoid this
> by fully flushing the socket before starting to wait.
>
> Signed-off-by: Jonas Ådahl 
> ---
>  src/wayland-client.c | 39 +--
>  1 file changed, 29 insertions(+), 10 deletions(-)
>
> diff --git a/src/wayland-client.c b/src/wayland-client.c
> index 8bf6124..f47e395 100644
> --- a/src/wayland-client.c
> +++ b/src/wayland-client.c
> @@ -1442,6 +1442,21 @@ wl_display_cancel_read(struct wl_display *display)
> pthread_mutex_unlock(>mutex);
>  }
>
> +static int
> +wl_display_poll(struct wl_display *display, short int events)
> +{
> +   int ret;
> +   struct pollfd pfd[2];

why 2? why an array at all? why not just

struct pollfd pfd;
pfd.fd = display->fd;
pfd.events = events;
do {
  poll(, 1, -1);
} while (blabla);

(i understand this was in the original as well and you're simply
moving code around)

> +
> +   pfd[0].fd = display->fd;
> +   pfd[0].events = events;
> +   do {
> +   ret = poll(pfd, 1, -1);
> +   } while (ret == -1 && errno == EINTR);
> +
> +   return ret;
> +}
> +
>  /** Dispatch events in an event queue
>   *
>   * \param display The display context object
> @@ -1485,27 +1500,31 @@ WL_EXPORT int
>  wl_display_dispatch_queue(struct wl_display *display,
>   struct wl_event_queue *queue)
>  {
> -   struct pollfd pfd[2];
> int ret;
>
> if (wl_display_prepare_read_queue(display, queue) == -1)
> return wl_display_dispatch_queue_pending(display, queue);
>
> +   while (true) {
> +   ret = wl_display_flush(display);
> +   if (ret == -1 && errno == EAGAIN) {
> +   if (wl_display_poll(display, POLLOUT) == -1) {
> +   wl_display_cancel_read(display);
> +   return -1;
> +   }
> +   } else {
> +   break;
> +   }
> +   }
> +
> /* Don't stop if flushing hits an EPIPE; continue so we can read any
>  * protocol error that may have triggered it. */
> -   ret = wl_display_flush(display);
> -   if (ret < 0 && errno != EAGAIN && errno != EPIPE) {
> +   if (ret < 0 && errno != EPIPE) {
> wl_display_cancel_read(display);
> return -1;
> }
>
> -   pfd[0].fd = display->fd;
> -   pfd[0].events = POLLIN;
> -   do {
> -   ret = poll(pfd, 1, -1);
> -   } while (ret == -1 && errno == EINTR);
> -
> -   if (ret == -1) {
> +   if (wl_display_poll(display, POLLIN) == -1) {
> wl_display_cancel_read(display);
> return -1;
> }
> --
> 2.4.3
>
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland v3] protocol: add support for cross-interface enum attributes

[Auke Booij]

Bill, any chance you could review this? Or would you prefer it if this
were based on your patch (which I do still support)?

On 5 December 2015 at 13:39, Auke Booij <a...@tulcod.com> wrote:
> The enum attribute, for which scanner support was introduced in
> 1771299, can be used to link message arguments to s. However,
> some arguments refer to s in a different .
>
> This adds scanner support for referring to an  in a different
>  using dot notation. It also sets the attributes in this
> style in the wayland XML protocol (wl_shm_pool::create_buffer::format
> to wl_shm::format, and wl_surface::set_buffer_transform::transform to
> wl_output::transform), and updates the documentation XSL so that this
> new style is supported.
>
> Changes since v2:
>  - add object:: prefix for all enumerations in the documentation
>  - fix whitespace in scanner.c
>  - minor code fixup to return early and avoid casts in scanner.c
>
> Changes since v1:
>  - several implementation bugs fixed
>
> Signed-off-by: Auke Booij <a...@tulcod.com>
> ---
>  doc/publican/protocol-to-docbook.xsl | 19 +--
>  protocol/wayland.xml |  4 +--
>  src/scanner.c| 63 
> +++-
>  3 files changed, 65 insertions(+), 21 deletions(-)
>
> diff --git a/doc/publican/protocol-to-docbook.xsl 
> b/doc/publican/protocol-to-docbook.xsl
> index fad207a..7e892c3 100644
> --- a/doc/publican/protocol-to-docbook.xsl
> +++ b/doc/publican/protocol-to-docbook.xsl
> @@ -103,9 +103,22 @@
>  
>  
>
> -
> -  
> -
> +
> +  
> +
> +  
> +  ::
> +  
> +
> +  
> +  
> +
> +  
> +  ::
> +  
> +
> +  
> +
>  
>
>
> diff --git a/protocol/wayland.xml b/protocol/wayland.xml
> index f9e6d76..0873553 100644
> --- a/protocol/wayland.xml
> +++ b/protocol/wayland.xml
> @@ -229,7 +229,7 @@
>
>
>
> -  
> +  
>  
>
>  
> @@ -1292,7 +1292,7 @@
> wl_output.transform enum the invalid_transform protocol error
> is raised.
>
> -  
> +  
>  
>
>  
> diff --git a/src/scanner.c b/src/scanner.c
> index 406519f..85aeea7 100644
> --- a/src/scanner.c
> +++ b/src/scanner.c
> @@ -747,8 +747,8 @@ start_element(void *data, const char *element_name, const 
> char **atts)
> enumeration->bitfield = true;
> else
> fail(>loc,
> - "invalid value (%s) for bitfield attribute 
> (only true/false are accepted)",
> - bitfield);
> +"invalid value (%s) for bitfield attribute (only 
> true/false are accepted)",
> +bitfield);
>
> wl_list_insert(ctx->interface->enumeration_list.prev,
>>link);
> @@ -785,32 +785,62 @@ start_element(void *data, const char *element_name, 
> const char **atts)
> }
>  }
>
> +static struct enumeration *
> +find_enumeration(struct protocol *protocol, struct interface *interface, 
> char *enum_attribute)
> +{
> +   struct interface *i;
> +   struct enumeration *e;
> +   char *enum_name;
> +   uint idx = 0, j;
> +
> +   for (j = 0; j + 1 < strlen(enum_attribute); j++) {
> +   if (enum_attribute[j] == '.') {
> +   idx = j;
> +   }
> +   }
> +
> +   if (idx > 0) {
> +   enum_name = enum_attribute + idx + 1;
> +
> +   wl_list_for_each(i, >interface_list, link)
> +   if (strncmp(i->name, enum_attribute, idx) == 0)
> +   wl_list_for_each(e, >enumeration_list, 
> link)
> +   if(strcmp(e->name, enum_name) == 0)
> +   return e;
> +   } else if (interface) {
> +   enum_name = enum_attribute;
> +
> +   wl_list_for_each(e, >enumeration_list, link)
> +   if(strcmp(e->name, enum_name) == 0)
> +   return e;
> +   }
> +
> +   return NULL;
> +}
> +
>  static void
> -verify_arguments(st

Re: RFC: idle protocol

[Auke Booij]

On 9 December 2015 at 07:43, Peter Hutterer  wrote:
> On Wed, Dec 09, 2015 at 08:17:40AM +0100, Martin Graesslin wrote:
>> On Wednesday, December 9, 2015 4:19:12 PM CET Peter Hutterer wrote:
>> > On Tue, Dec 08, 2015 at 02:12:01PM +0100, Martin Graesslin wrote:
>> > > Hi Wayland-developers,
>> > >
>> > > at KDE we developed a protocol for our framework kidletime [1]. The idea
>> > > is to notify Wayland clients when a wl_seat has been idle for a specified
>> > > time. We use this for example for power management, screen locking etc.
>> > > But a common use case is also setting a user as away in a chat
>> > > application.
>> > >
>> > > We think that this protocol can be in general useful for all Wayland 
>> > > based
>> > > systems. Our current protocol is attached to this mail. Of course for
>> > > integration into wayland protocols the namespace needs adjustments (I'm
>> > > open for suggestions).
>> > >
>> > > The reference implementation of the protocol can be found in the KWayland
>> > > repository (client at [2], server at [3]).
>> > >
>> > > Best Regards
>> > > Martin Gräßlin
>> > >
>> > > [1] http://inqlude.org/libraries/kidletime.html
>> > > [2] git://anongit.kde.org/kwayland (path src/client/idle.h and 
>> > > src/client/
>> > > idle.cpp)
>> > > [3] git://anongit.kde.org/kwayland (path src/server/idle_interface.h and
>> > > src/ server/idle_interface.cpp)
>> > >
>> > > 
>> > > 
>> > >
>> > >   
>> > >   
>> > >
>> > >   
>> > >
>> > > This interface allows to monitor user idle time on a given seat.
>> > > The interface allows to register timers which trigger after no
>> > > user activity was registered on the seat for a given interval. It
>> > > notifies when user activity resumes.
>> > you need to describe what classifies as "user activity".
>>
>> something like "after no input events with a new timestamp"? I'm always bad 
>> at
>> writing documentation ;-)
>
> yeah, but you'll need to figure out what you want too. does this include
> input events as seen on the protocol, as handled by the compositor, does it
> include input events not otherwise handled as input (e.g. lid switch), etc.
> it's a lot easier to bike-shed when these things are already filled in :)

(apologies for bike-shedding ahead of time)

I don't think you'll want to specify it in any of these terms. With
our increasingly sensitive devices, it is not unthinkable that future
devices will have a sensor to detect user activity: e.g. whether a
person is looking at the screen. This is not futuristic: for example,
some cars already have eye trackers that detect driver fatigue and
distraction (which could be one way to define "user activity").
Similarly, some smartphones have login by face recognition - so surely
they can also detect a reasonable notion of "user activity" with the
same mechanism.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH wayland v3] protocol: add support for cross-interface enum attributes

[Auke Booij]

The enum attribute, for which scanner support was introduced in
1771299, can be used to link message arguments to s. However,
some arguments refer to s in a different .

This adds scanner support for referring to an  in a different
 using dot notation. It also sets the attributes in this
style in the wayland XML protocol (wl_shm_pool::create_buffer::format
to wl_shm::format, and wl_surface::set_buffer_transform::transform to
wl_output::transform), and updates the documentation XSL so that this
new style is supported.

Changes since v2:
 - add object:: prefix for all enumerations in the documentation
 - fix whitespace in scanner.c
 - minor code fixup to return early and avoid casts in scanner.c

Changes since v1:
 - several implementation bugs fixed

Signed-off-by: Auke Booij <a...@tulcod.com>
---
 doc/publican/protocol-to-docbook.xsl | 19 +--
 protocol/wayland.xml |  4 +--
 src/scanner.c| 63 +++-
 3 files changed, 65 insertions(+), 21 deletions(-)

diff --git a/doc/publican/protocol-to-docbook.xsl 
b/doc/publican/protocol-to-docbook.xsl
index fad207a..7e892c3 100644
--- a/doc/publican/protocol-to-docbook.xsl
+++ b/doc/publican/protocol-to-docbook.xsl
@@ -103,9 +103,22 @@
 
 
   
-
-  
-
+
+  
+
+  
+  ::
+  
+
+  
+  
+
+  
+  ::
+  
+
+  
+
 
   
   
diff --git a/protocol/wayland.xml b/protocol/wayland.xml
index f9e6d76..0873553 100644
--- a/protocol/wayland.xml
+++ b/protocol/wayland.xml
@@ -229,7 +229,7 @@
   
   
   
-  
+  
 
 
 
@@ -1292,7 +1292,7 @@
wl_output.transform enum the invalid_transform protocol error
is raised.
   
-  
+  
 
 
 
diff --git a/src/scanner.c b/src/scanner.c
index 406519f..85aeea7 100644
--- a/src/scanner.c
+++ b/src/scanner.c
@@ -747,8 +747,8 @@ start_element(void *data, const char *element_name, const 
char **atts)
enumeration->bitfield = true;
else
fail(>loc,
- "invalid value (%s) for bitfield attribute (only 
true/false are accepted)",
- bitfield);
+"invalid value (%s) for bitfield attribute (only 
true/false are accepted)",
+bitfield);
 
wl_list_insert(ctx->interface->enumeration_list.prev,
   >link);
@@ -785,32 +785,62 @@ start_element(void *data, const char *element_name, const 
char **atts)
}
 }
 
+static struct enumeration *
+find_enumeration(struct protocol *protocol, struct interface *interface, char 
*enum_attribute)
+{
+   struct interface *i;
+   struct enumeration *e;
+   char *enum_name;
+   uint idx = 0, j;
+
+   for (j = 0; j + 1 < strlen(enum_attribute); j++) {
+   if (enum_attribute[j] == '.') {
+   idx = j;
+   }
+   }
+
+   if (idx > 0) {
+   enum_name = enum_attribute + idx + 1;
+
+   wl_list_for_each(i, >interface_list, link)
+   if (strncmp(i->name, enum_attribute, idx) == 0)
+   wl_list_for_each(e, >enumeration_list, link)
+   if(strcmp(e->name, enum_name) == 0)
+   return e;
+   } else if (interface) {
+   enum_name = enum_attribute;
+
+   wl_list_for_each(e, >enumeration_list, link)
+   if(strcmp(e->name, enum_name) == 0)
+   return e;
+   }
+
+   return NULL;
+}
+
 static void
-verify_arguments(struct parse_context *ctx, struct wl_list *messages, struct 
wl_list *enumerations)
+verify_arguments(struct parse_context *ctx, struct interface *interface, 
struct wl_list *messages, struct wl_list *enumerations)
 {
struct message *m;
wl_list_for_each(m, messages, link) {
struct arg *a;
wl_list_for_each(a, >arg_list, link) {
-   struct enumeration *e, *f;
+   struct enumeration *e;
 
if (!a->enumeration_name)
continue;
 
-   f = NULL;
-   wl_list_for_each(e, enumerations, link) {
-   if(strcmp(e->name, a->enumeration_name) == 0)
-   f = e;
-   }
 
-   if (f == NULL)
+   e = find_enumeration(ctx

Re: [PATCH v2 wayland] protocol: add support for cross-interface enum attributes

[Auke Booij]

I'd be happy to rebase against that, but since that hasn't been merged
yet, I thought I'd take the current tree as a a basis.

If you think having the interface:: prefix everywhere makes sense, I
can change that if you want.

On 2 December 2015 at 03:09, Bill Spitzak <spit...@gmail.com> wrote:
> Looks correct to me.
>
> I had a slightly different patch in patchwork which changes
> protocol-to-docbook. It puts the enumeration into it's own statement, rather
> than a nested if statement, and it adds the object:: prefix to the
> documentation for all enumerations, not just the cross-object ones.
>
> On Sun, Nov 29, 2015 at 5:54 AM, Auke Booij <a...@tulcod.com> wrote:
>>
>> The enum attribute, for which scanner support was introduced in
>> 1771299, can be used to link message arguments to s. However,
>> some arguments refer to s in a different .
>>
>> This adds scanner support for referring to an  in a different
>>  using dot notation. It also sets the attributes in this
>> style in the wayland XML protocol (wl_shm_pool::create_buffer::format
>> to wl_shm::format, and wl_surface::set_buffer_transform::transform to
>> wl_output::transform), and updates the documentation XSL so that this
>> new style is supported.
>>
>> Changes since v1:
>>  - several implementation bugs fixed
>>
>> Signed-off-by: Auke Booij <a...@tulcod.com>
>> ---
>>  doc/publican/protocol-to-docbook.xsl | 17 +--
>>  protocol/wayland.xml |  4 +--
>>  src/scanner.c| 59
>> +++-
>>  3 files changed, 61 insertions(+), 19 deletions(-)
>>
>> diff --git a/doc/publican/protocol-to-docbook.xsl
>> b/doc/publican/protocol-to-docbook.xsl
>> index fad207a..1fa066d 100644
>> --- a/doc/publican/protocol-to-docbook.xsl
>> +++ b/doc/publican/protocol-to-docbook.xsl
>> @@ -103,9 +103,20 @@
>>  
>>  
>>
>> -
>> -  
>> -
>> +
>> +  
>> +
>> +  
>> +  ::
>> +  
>> +
>> +  
>> +  
>> +
>> +  
>> +
>> +  
>> +
>>  
>>
>>
>> diff --git a/protocol/wayland.xml b/protocol/wayland.xml
>> index f9e6d76..0873553 100644
>> --- a/protocol/wayland.xml
>> +++ b/protocol/wayland.xml
>> @@ -229,7 +229,7 @@
>>
>>
>>
>> -  
>> +  
>>  
>>
>>  
>> @@ -1292,7 +1292,7 @@
>> wl_output.transform enum the invalid_transform protocol error
>> is raised.
>>
>> -  
>> +  
>>  
>>
>>  
>> diff --git a/src/scanner.c b/src/scanner.c
>> index 406519f..e69bd2a 100644
>> --- a/src/scanner.c
>> +++ b/src/scanner.c
>> @@ -785,32 +785,62 @@ start_element(void *data, const char *element_name,
>> const char **atts)
>> }
>>  }
>>
>> +static struct enumeration *
>> +find_enumeration(struct protocol *protocol, struct interface *interface,
>> char *enum_attribute)
>> +{
>> +   struct interface *i;
>> +   struct enumeration *e, *f = NULL;
>> +   char *enum_name;
>> +   int idx = 0, j;
>> +
>> +for (j = 0; j + 1 < (int)strlen(enum_attribute); j++) {
>> +   if (enum_attribute[j] == '.') {
>> +   idx = j;
>> +   }
>> +   }
>> +
>> +   if (idx > 0) {
>> +   enum_name = enum_attribute + idx + 1;
>> +
>> +   wl_list_for_each(i, >interface_list, link)
>> +   if (strncmp(i->name, enum_attribute, idx) == 0)
>> +   wl_list_for_each(e, >enumeration_list,
>> link)
>> +   if(strcmp(e->name, enum_name) ==
>> 0)
>> +   f = e;
>> +   } else if (interface) {
>> +   enum_name = enum_attribute;
>> +
>> +   wl_list_for_each(e, >enumeration_list, link)
>> +   if(strcmp(e->name, enum_name) == 0)
>> +   f = e;
>> +   }
>> +
>> +   return f;
>> +}
>> +
>>  static void
>> -verify_argu

[PATCH v2 wayland] protocol: add support for cross-interface enum attributes

[Auke Booij]

The enum attribute, for which scanner support was introduced in
1771299, can be used to link message arguments to s. However,
some arguments refer to s in a different .

This adds scanner support for referring to an  in a different
 using dot notation. It also sets the attributes in this
style in the wayland XML protocol (wl_shm_pool::create_buffer::format
to wl_shm::format, and wl_surface::set_buffer_transform::transform to
wl_output::transform), and updates the documentation XSL so that this
new style is supported.

Changes since v1:
 - several implementation bugs fixed

Signed-off-by: Auke Booij <a...@tulcod.com>
---
 doc/publican/protocol-to-docbook.xsl | 17 +--
 protocol/wayland.xml |  4 +--
 src/scanner.c| 59 +++-
 3 files changed, 61 insertions(+), 19 deletions(-)

diff --git a/doc/publican/protocol-to-docbook.xsl 
b/doc/publican/protocol-to-docbook.xsl
index fad207a..1fa066d 100644
--- a/doc/publican/protocol-to-docbook.xsl
+++ b/doc/publican/protocol-to-docbook.xsl
@@ -103,9 +103,20 @@
 
 
   
-
-  
-
+
+  
+
+  
+  ::
+  
+
+  
+  
+
+  
+
+  
+
 
   
   
diff --git a/protocol/wayland.xml b/protocol/wayland.xml
index f9e6d76..0873553 100644
--- a/protocol/wayland.xml
+++ b/protocol/wayland.xml
@@ -229,7 +229,7 @@
   
   
   
-  
+  
 
 
 
@@ -1292,7 +1292,7 @@
wl_output.transform enum the invalid_transform protocol error
is raised.
   
-  
+  
 
 
 
diff --git a/src/scanner.c b/src/scanner.c
index 406519f..e69bd2a 100644
--- a/src/scanner.c
+++ b/src/scanner.c
@@ -785,32 +785,62 @@ start_element(void *data, const char *element_name, const 
char **atts)
}
 }
 
+static struct enumeration *
+find_enumeration(struct protocol *protocol, struct interface *interface, char 
*enum_attribute)
+{
+   struct interface *i;
+   struct enumeration *e, *f = NULL;
+   char *enum_name;
+   int idx = 0, j;
+
+for (j = 0; j + 1 < (int)strlen(enum_attribute); j++) {
+   if (enum_attribute[j] == '.') {
+   idx = j;
+   }
+   }
+
+   if (idx > 0) {
+   enum_name = enum_attribute + idx + 1;
+
+   wl_list_for_each(i, >interface_list, link)
+   if (strncmp(i->name, enum_attribute, idx) == 0)
+   wl_list_for_each(e, >enumeration_list, link)
+   if(strcmp(e->name, enum_name) == 0)
+   f = e;
+   } else if (interface) {
+   enum_name = enum_attribute;
+
+   wl_list_for_each(e, >enumeration_list, link)
+   if(strcmp(e->name, enum_name) == 0)
+   f = e;
+   }
+
+   return f;
+}
+
 static void
-verify_arguments(struct parse_context *ctx, struct wl_list *messages, struct 
wl_list *enumerations)
+verify_arguments(struct parse_context *ctx, struct interface *interface, 
struct wl_list *messages, struct wl_list *enumerations)
 {
struct message *m;
wl_list_for_each(m, messages, link) {
struct arg *a;
wl_list_for_each(a, >arg_list, link) {
-   struct enumeration *e, *f;
+   struct enumeration *e;
 
if (!a->enumeration_name)
continue;
 
-   f = NULL;
-   wl_list_for_each(e, enumerations, link) {
-   if(strcmp(e->name, a->enumeration_name) == 0)
-   f = e;
-   }
 
-   if (f == NULL)
+   e = find_enumeration(ctx->protocol, interface, 
a->enumeration_name);
+
+   if (e == NULL)
fail(>loc,
 "could not find enumeration %s",
 a->enumeration_name);
 
switch (a->type) {
case INT:
-   if (f->bitfield)
+   if (e->bitfield)
fail(>loc,
 "bitfield-style enum must only be 
referenced by uint");
break;
@@ -848,12 +878,13 @@ end_element(void *data, const XML_Char *name)
 ctx->enumeration->name);
}
ctx->enumeration = NULL;

Re: [PATCH wayland] Add support for cross-interface enum attributes

[Auke Booij]

On 29 November 2015 at 01:27, Auke Booij <a...@tulcod.com> wrote:
> Signed-off-by: Auke Booij <a...@tulcod.com>

I realized this is the wrong search algorithm. I will fix it later today.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH wayland] Add support for cross-interface enum attributes

[Auke Booij]

Signed-off-by: Auke Booij <a...@tulcod.com>
---
 doc/publican/protocol-to-docbook.xsl | 17 +---
 protocol/wayland.xml |  4 +--
 src/scanner.c| 51 +++-
 3 files changed, 55 insertions(+), 17 deletions(-)

diff --git a/doc/publican/protocol-to-docbook.xsl 
b/doc/publican/protocol-to-docbook.xsl
index fad207a..1fa066d 100644
--- a/doc/publican/protocol-to-docbook.xsl
+++ b/doc/publican/protocol-to-docbook.xsl
@@ -103,9 +103,20 @@
 
 
   
-
-  
-
+
+  
+
+  
+  ::
+  
+
+  
+  
+
+  
+
+  
+
 
   
   
diff --git a/protocol/wayland.xml b/protocol/wayland.xml
index f9e6d76..0873553 100644
--- a/protocol/wayland.xml
+++ b/protocol/wayland.xml
@@ -229,7 +229,7 @@
   
   
   
-  
+  
 
 
 
@@ -1292,7 +1292,7 @@
wl_output.transform enum the invalid_transform protocol error
is raised.
   
-  
+  
 
 
 
diff --git a/src/scanner.c b/src/scanner.c
index 406519f..d6edf1a 100644
--- a/src/scanner.c
+++ b/src/scanner.c
@@ -785,6 +785,28 @@ start_element(void *data, const char *element_name, const 
char **atts)
}
 }
 
+static struct enumeration *
+find_enumeration(struct protocol *protocol, char *enum_attribute, size_t 
interface_length)
+{
+   struct interface *i;
+   struct enumeration *e, *f;
+   char *enum_name;
+
+   if (interface_length == 0)
+   enum_name = enum_attribute;
+   else
+   enum_name = enum_attribute + interface_length + 1;
+
+   f = NULL;
+   wl_list_for_each(i, >interface_list, link)
+   if (strncmp(i->name, enum_attribute, interface_length) == 0)
+   wl_list_for_each(e, >enumeration_list, link)
+   if(strcmp(e->name, enum_name) == 0)
+   f = e;
+
+   return f;
+}
+
 static void
 verify_arguments(struct parse_context *ctx, struct wl_list *messages, struct 
wl_list *enumerations)
 {
@@ -792,25 +814,29 @@ verify_arguments(struct parse_context *ctx, struct 
wl_list *messages, struct wl_
wl_list_for_each(m, messages, link) {
struct arg *a;
wl_list_for_each(a, >arg_list, link) {
-   struct enumeration *e, *f;
+   struct enumeration *e;
+   int i, j = 0;
 
if (!a->enumeration_name)
continue;
 
-   f = NULL;
-   wl_list_for_each(e, enumerations, link) {
-   if(strcmp(e->name, a->enumeration_name) == 0)
-   f = e;
+   for (i = 0; i < a->enumeration_name[i]; i++) {
+   if (a->enumeration_name[i] == '.') {
+   j = i;
+   }
+
}
 
-   if (f == NULL)
+   e = find_enumeration(ctx->protocol, 
a->enumeration_name, j);
+
+   if (e == NULL)
fail(>loc,
 "could not find enumeration %s",
 a->enumeration_name);
 
switch (a->type) {
case INT:
-   if (f->bitfield)
+   if (e->bitfield)
fail(>loc,
 "bitfield-style enum must only be 
referenced by uint");
break;
@@ -848,12 +874,13 @@ end_element(void *data, const XML_Char *name)
 ctx->enumeration->name);
}
ctx->enumeration = NULL;
-   } else if (strcmp(name, "interface") == 0) {
-   struct interface *i = ctx->interface;
-
-   verify_arguments(ctx, >request_list, >enumeration_list);
-   verify_arguments(ctx, >event_list, >enumeration_list);
+   } else if (strcmp(name, "protocol") == 0) {
+   struct interface *i;
 
+   wl_list_for_each(i, >protocol->interface_list, link) {
+   verify_arguments(ctx, >request_list, 
>enumeration_list);
+   verify_arguments(ctx, >event_list, 
>enumeration_list);
+   }
}
 }
 
-- 
2.6.2

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [RFC] Declare enumeration wl_output.transform as bitfield.

[Auke Booij]

On 25 November 2015 at 13:43, Daniel Stone  wrote:
> Hi,
>
> On 25 November 2015 at 12:08, Nils Christopher Brause
>  wrote:
>> 3. The API change is negligible. Every request that accepts this bitfield
>>expects a number in the range of zero to seven. These numbers can be
>>represented in and converted between signed and unsigned 32 bit integers
>>without any loss and generated compiler warnings are harmless.
>
> Warnings aren't harmless though; there's a reason we don't (shouldn't)
> have any in our build. Every 'oh, just ignore that' warning drowns out
> legitimate compiler warnings, and dumping warnings on our users
> because we broke a stable API isn't really acceptable. So, sorry, but
> NAK from me. :(
>
> Cheers,
> Daniel

Note that this these warnings are resolvable: e.g. the weston code can
be fixed so that it does not generate warnings under this change.
(Although I understand that that is not quite sufficient.)

Under what conditions would you agree to this protocol change? E.g.
would more documentation towards developers regarding the warnings
help? Or some sneaky way to avoid compiler warnings?
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: Changing wl_output.transform type to unsigned?

[Auke Booij]

On 24 November 2015 at 12:41, Nils Chr. Brause <nilschrbra...@gmail.com> wrote:
> On Mon, Nov 16, 2015 at 11:46 AM, Pekka Paalanen <ppaala...@gmail.com> wrote:
>> On Sun, 15 Nov 2015 22:17:38 +0100
>> "Nils Chr. Brause" <nilschrbra...@gmail.com> wrote:
>>
>>> On Sun, Nov 15, 2015 at 9:48 PM, Auke Booij <a...@tulcod.com> wrote:
>>> > On 9 November 2015 at 18:17, Bill Spitzak <spit...@gmail.com> wrote:
>>> >> I really do not see the problem with allowing it to be an int argument as
>>> >> long as the enum value 2^31 is not used. Though I am also stumped as to 
>>> >> why
>>> >> you can't change the current misused ints into uint in the protocol. It 
>>> >> will
>>> >> not change the bit layout in the messages and therefore is not a protocol
>>> >> change.
>>> >
>>> > I don't really know what to do with this final claim. I like the idea,
>>> > and it makes sense. Finally, it will solve this issue and potentially
>>> > future ones as well. Is there any chance it could be implemented or is
>>> > it a crazy idea?
>>>
>>> Bill is absolutely right. And it also doesn't even really change the C API,
>>> because nobody is passing negetive numbers or number greater than 2^31-1
>>> there anyway. Therefore, I am all for a change. :)
>>
>> Hi,
>
> Hi,
>
> sorry I was not answering in a while.
>
>> your logic seems sound at first.
>>
>> The things we would need to change in the protocol are:
>> - wl_surface.set_buffer_transform
>> - wl_output.geometry
>> - (possible third party extensions)
>>
>> It would break existing bindings for strongly typed languages that do
>> not allow implicit conversion between signed and unsigned. (Does Java
>> fall into that category?)
>
> Strongly typed bindings would use special types for enumerations/bitfields
> anyway, so this probably isn't a problem.
>
>> You don't see any change on the wire, but changing the type changes the
>> C API, which then changes types of variables used in C programs. I
>> can't imagine this having an impact in this particular case, though.
>
> The changes in the C API  doesn't even produce any warings. Take a look
> at the following:
>
> #include 
> int foo(uint32_t c)
> {
>   return c;
> }
> int main()
> {
>   int32_t bar = 7;
>   return foo(bar);
> }
>
> It compiles without any warings, even with -Wall. Tested an all available C
> and C++ Standards with GCC 5.2.
>
>>
>> Weston seems to use mostly 'enum wl_buffer_transform' as the type, but
>> struct weston_buffer_viewport already uses uint32_t.
>>
>> Ok, I suppose we could try this.
>>
>> The next step would be for someone to propose a patch to change the
>> ints to uints in wayland.xml. Special attention should be given to the
>> commit message: why change this, what benefits it gives, and even
>> though it is breaking the protocol, why it cannot break anything in
>> practice.
>>
>> It is important to write a good commit message, because I expect people
>> to be looking at it a lot, since it is changing stable interfaces.
>
> I tried my best to come up with an extensive commit message that should
> explain everything:
>
>
> Declare enumeration wl_output.transform as bitfield.
>
> The enumeration wl_output.transform is clearly a bitfield.

(if this is true, then why do you continue explaining this clear fact?
either it's clear or it requires explanation, not both)

> The definition of a bitfield is that each bit has a distinct
> meaning. This is clearly the case in the enumeration
> wl_output.transform:
>
> - bit 0: rotate by 90 degree
> - bit 1: rotate by 180 degree
> - bit 2: flip around vertical axis

As I brought up previously: if you want to specify this, you might as
well go the extra mile and figure out which direction the rotation is
in, and if the flipping goes before or after the rotation. otherwise
you're not really saying anything people cannot figure out on their
own.

> Every other value can be constructed by ORing together the
> above bits:
>
> - "270" = "90" | "180"
> - "flipped_90" = "90" | "flipped"
> - "flipped_180" = "180" | "flipped"
> - "flipped_270" = "90" | "180" | "flipped"
>
> Therefore the bitfield="true" attribute has been added to
> the enumeration declaration.

I believe commit messages should be written in an imperative mood.

https:/

Re: [PATCH v5 wayland] protocol: add wl_pointer.frame, axis_source, axis_stop, and axis_discrete

[Auke Booij]

On 28 October 2015 at 05:34, Peter Hutterer  wrote:
> The frame event groups separate pointer events together. The primary use-case
> for this at the moment is diagonal scrolling - a vertical/horizontal scroll
> event can be grouped together to calculate the correct motion vector.
> Frame events group all wl_pointer events. An example sequence of motion events
> followed by a diagonal scroll followed by a button event is:
> wl_pointer.motion
> wl_pointer.frame
> wl_pointer.motion
> wl_pointer.frame
> wl_pointer.axis
> wl_pointer.axis
> wl_pointer.frame
> wl_pointer.button
> wl_pointer.frame
>
> In the future, other extensions may insert additional information about an
> event into the frame. For example, an extension may add information about the
> physical device that generated an event into the frame. For this reason,
> enter/leave events are grouped by a frame event too.
>
> The axis_source event determines how an axis event was generated. That enables
> clients to judge when to use kinetic scrolling. Only one axis_source event is
> allowed per frame and applies to all events in this frame.
>
> The axis_stop event notifies a client about the termination of a scroll
> sequence, likewise needed to calculate kinetic scrolling parameters.
> Multiple axis_stop events within the same frame indicate that scrolling has
> stopped in all these axis at the same time.
>
> The axis_discrete event provides the wheel click count. Previously the axis
> value was some hardcoded number (10), with the discrete steps this enables a
> client to differ between line-based scrolling on a mouse wheel and smooth
> scrolling with a touchpad.
>
> We can't extend the existing wl_pointer.axis events so we introduce a new
> concept: latching events. These events (currently only axis_discrete)
> are prefixed before a wl_pointer.axis event. A client must build the full
> state of the event until the respective top-level event arrives.
> i.e. a single event frame for a diagonal scroll with discrete information may
> be:
>
> wl_pointer.axis_source
> wl_pointer.axis_discrete
> wl_pointer.axis
> wl_pointer.axis_discrete
> wl_pointer.axis
> wl_pointer.frame
>
> Signed-off-by: Peter Hutterer 
> ---

(as for your tablet protocol patch) Perhaps we can use the enum
attribute here as well:

 
...
 
...
 
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH] Declare enumeration wl_output.transform as bitfield.

[Auke Booij]

On 9 November 2015 at 18:17, Bill Spitzak  wrote:
> Making the transform into a bitfield allows bitfield tests for useful facts:
> it can see if it is a mirror image by testing the flip bit, and check for
> transposition of the axes by checking the 90 degree bit. I believe this is
> the reason behind the desire to declare it a bitfield and I agree this is
> nice to have.
>
> I really do not see the problem with allowing it to be an int argument as
> long as the enum value 2^31 is not used. Though I am also stumped as to why
> you can't change the current misused ints into uint in the protocol. It will
> not change the bit layout in the messages and therefore is not a protocol
> change.

I don't really know what to do with this final claim. I like the idea,
and it makes sense. Finally, it will solve this issue and potentially
future ones as well. Is there any chance it could be implemented or is
it a crazy idea?
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland-protocols] Add the tablet protocol

[Auke Booij]

On 6 November 2015 at 04:24, Peter Hutterer  wrote:
> Signed-off-by: Peter Hutterer 
> ---
> This is the revamped version of the tablet protocol for graphics tablets
> (e.g. Wacom tablets). Too many changes from the last version (a year ago or
> so), so I won't detail them, best to look at it with fresh eyes.

Would it be possible to cross-reference  definitions in s,
as was made possible by the introduction of the enum attribute
recently? E.g.

 
...
 
...
 
...
 
...
 
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH] Declare enumeration wl_output.transform as bitfield.

[Auke Booij]

On 9 November 2015 at 14:39, Auke Booij <a...@tulcod.com> wrote:
> and it was a mistake

Sorry, let me retract this - that was maybe a bit harsh, since I don't
know the exact history.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH] Declare enumeration wl_output.transform as bitfield.

[Auke Booij]

On 9 November 2015 at 11:54, Nils Chr. Brause <nilschrbra...@gmail.com> wrote:
> Hi,
>
> On Mon, Nov 9, 2015 at 12:04 PM, Pekka Paalanen <ppaala...@gmail.com> wrote:
>> On Fri, 6 Nov 2015 16:05:10 +0100
>> "Nils Chr. Brause" <nilschrbra...@gmail.com> wrote:
>>
>>> Hi,
>>>
>>> On Fri, Nov 6, 2015 at 3:48 PM, Auke Booij <a...@tulcod.com> wrote:
>>> > On 6 November 2015 at 13:03, Nils Christopher Brause
>>> > <nilschrbra...@gmail.com> wrote:
>>> >> The enumeration wl_output.transform is clearly a bitfield.
>>> >> The definition of a bitfield is that each bit has a distinct
>>> >> meaning. This is clearly the case in the enumeration
>>> >> wl_output.transform:
>>> >>
>>> >> - bit 0: rotate by 90 degree
>>> >> - bit 1: rotate by 180 degree
>>> >> - bit 2: flip around vertical axis
>>> >
>>> > Just to complete the definitions: what is the right order of
>>> > operations here? How do I transform from "normal" to
>>> > wl_output::transform = 0b101, for example? First rotating 90 degrees
>>> > and then flipping is not the same as first flipping and then rotating
>>> > 90 degrees. In mathematical terms, the symmetry group of the square is
>>> > not a commutative group.
>>
>> Hi,
>>
>> for the record, what these values actually mean always totally confuses
>> me even without the interpretation as a bitfield.
>>
>>> >> Every other value can be constructed by ORing together the
>>> >> above bits:
>>> >>
>>> >> - "270" = "90" | "180"
>>> >> - "flipped_90" = "90" | "flipped"
>>> >> - "flipped_180" = "180" | "flipped"
>>> >> - "flipped_270" = "90" | "180" | "flipped"
>>> >
>>> > While I agree with this, this does not seem to be used anywhere in
>>> > weston's source code. Not saying I disagree with your claim that
>>> > wl_output::transform should be considered a bitfield, but why is it
>>> > necessary to do so?
>>>
>>> I think everything that is a bitfield should be markted as such. Even if
>>
>> Otherwise I'd agree, but in this case we have to add exceptions in the
>> documentation, scanner, and language definition just because
>> historically we happen to use int rather than uint here. Too bad, but
>> in this case we have to live with not defining it a bitfield. I believe
>> this is the lesser evil.
>>
>>> weston doesent make use of the bitfield nature of wl_output::transform,
>>> users might want to do so. For example a user might want to check if
>>> a particual transformation includes a flip. Instead of having to do four
>>> comparisions (wich even might not be enough if more transformations
>>> are added in the future), a single AND would suffice here.
>>
>> I find it hard to imagine any more cases to be added. These already
>> cover all cases you can rotate a 2D-rectangle in 3D while maintaining
>> axis-aligment. This geometry comes from the physical reality of how
>> devices showing a 2D image work.
>>
>>> >> Therefore the bitfield="true" attribute has been added to
>>> >> the enumeration declaration. Since this bitfield is
>>> >> transferred as signed integer, the scanner had to be
>>> >> modified to accept that behaviour. This was also noted in
>>> >> the documentation.
>>> >
>>> > As I made clear in our previous discusions, I don't think that it is a
>>> > safe idea to allow signed integers to be bitfields. Requiring
>>> > unsignedness is a sanity check. So I would not like this patch, if
>>> > only for the reason that it invalidates this check.
>>>
>>> I don't see why the signedness should even matter. A bitfield doesn't
>>> have a numerical value after all. It is just a collection of bits.
>>
>> I totally agree with Auke here.
>>
>> Let's reject this patch. It's just one more historical accident we have
>> to live with.
>
> I still got no explaination why the signdness of a bitfield that has no
> numerical meaning even matters. (Rasons like "it feels natural" aside.)
> It is totally irrelavant whether a collection of bits is marked as signed or
> as unsigned. Therefore there is no sensible rea

Re: [PATCH wayland v2] Remove protocol/wayland.dtd

[Auke Booij]

On 9 November 2015 at 11:41, Pekka Paalanen  wrote:
> I believe comments are enough. I suppose the DTD error message from
> wayland-scanner could refer to built-in DTD, that would be a small
> change and make it obvious. Just add one word in the warning message.

Okay, your reasoning makes sense - this is a solution I can live with.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland v2] Remove protocol/wayland.dtd

[Auke Booij]

On 9 November 2015 at 10:54, Peter Hutterer <peter.hutte...@who-t.net> wrote:
> On 9/11/2015 20:39 , Auke Booij wrote:
>>
>> On 6 November 2015 at 11:26, Pekka Paalanen <ppaala...@gmail.com> wrote:
>>>
>>> On Fri, 6 Nov 2015 09:47:03 +1000
>>> Peter Hutterer <peter.hutte...@who-t.net> wrote:
>>>
>>>> On Thu, Nov 05, 2015 at 04:58:09PM +0200, Pekka Paalanen wrote:
>>>>>
>>>>> On Mon, 19 Oct 2015 11:30:47 +1000
>>>>> Peter Hutterer <peter.hutte...@who-t.net> wrote:
>>>>>
>>>>>> On Fri, Oct 16, 2015 at 11:42:21AM +0300, Pekka Paalanen wrote:
>>>
>>>
>>>>>>> If the original objection to a DTD was because it required manually
>>>>>>> writing a lint phase in every project build system using the XML
>>>>>>> files,
>>>>>>> then having wayland-scanner invoke the check automatically solves
>>>>>>> that.
>>>>>>
>>>>>>
>>>>>> the question that remains though is: the dtd must be an external file
>>>>>> for
>>>>>> extensions to be validated. Which means we need to either pass the dtd
>>>>>> as
>>>>>> argument to the scanner (requires makefile changes everywhere), or we
>>>>>> hardcode the path into wayland-scanner (issues with running the
>>>>>> scanner from
>>>>>> within the source tree) or we add it as variable to pkgconfig
>>>>>> (requires
>>>>>> makefile changes again). any other solutions to fix this are welcome.
>>>>>> even if all we do is call out to xmllint we still run into that issue.
>>>>>
>>>>>
>>>>> Or, hopefully we can embed the DTD file into the scanner binary - is
>>>>> there no way to let libxml2 read it as a plain string?
>>>>
>>>>
>>>> yep, just tested it here, it's a 3-line change to the patch to load from
>>>> a
>>>> string.
>>>
>>>
>>> Cool.
>>>
>>>>> I used the following trick to embed some files in Wesgr:
>>>>>
>>>>> https://github.com/ppaalanen/wesgr/commit/02a840cb7db7eeb80071a353f66865d729738ae5
>>>>
>>>>
>>>> nice one. though if we're going for the embedded route anyway, we could
>>>> just
>>>> have it a const char in the scanner.c file. The dtd isn't really
>>>> expected to
>>>> change and when it does, the scanner needs to change too. Maybe we
>>>> should
>>>> just go for the easy route?
>>>
>>>
>>> I'll let you pick the way. Should be easy to change it later if needed
>>> anyway.
>>>
>>> Would it be bad if wayland-scanner embedded the DTD and in addition we
>>> installed the DTD as a file too? So scanner used only its internal
>>> copy. I'd probably try that if someone wants the DTD as a separate file
>>> for their own use. In that case the trick would be handy.
>>>
>>>
>>> Thanks,
>>> pq
>>
>>
>> That sounds like the scanner will become confusing to use ("wait, so
>> where DOES it get the DTD from, if not this file?"), unless you'd
>> actually read the source code. Or this should be very clearly
>> documented.
>
>
> commenting in the code - sure. but communicating this to the user isn't
> needed IMO, it's not really different to the normal scanner behaviour of
> failing when there's something it doesn't expect (though unfortunately right
> now the scanner also ignores things where IMO it should just fail).
>
> Cheers,
>   Peter

I'm not sure we're talking about the same issue here. What I'm
imagining is a user/developer trying to get into the internals of
wayland dev, encountering the protocols, and then seeing a DTD file
next to them. He* thinks "ah, surely that's used for verification, and
it's where I can start implementing my dirty hack". He modifies the
DTD , but the scanner doesn't behave differently. He removes the DTD,
but the scanner still passes the DTD check. Then he thinks "wait, so
where DOES it get the DTD from, if not this file?", at which point he
opens up the source code and learns that the DTD exists in two places.

Documenting this clearly will save every future person who wants to
get involved in protocol development 1 hour of figuring things out
when they don't work. That, or not installing the DTD in two places
(so either install as a file or embedded in the executable, but not
both).

 * or she
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland v2] Remove protocol/wayland.dtd

[Auke Booij]

On 6 November 2015 at 11:26, Pekka Paalanen  wrote:
> On Fri, 6 Nov 2015 09:47:03 +1000
> Peter Hutterer  wrote:
>
>> On Thu, Nov 05, 2015 at 04:58:09PM +0200, Pekka Paalanen wrote:
>> > On Mon, 19 Oct 2015 11:30:47 +1000
>> > Peter Hutterer  wrote:
>> >
>> > > On Fri, Oct 16, 2015 at 11:42:21AM +0300, Pekka Paalanen wrote:
>
>> > > > If the original objection to a DTD was because it required manually
>> > > > writing a lint phase in every project build system using the XML files,
>> > > > then having wayland-scanner invoke the check automatically solves that.
>> > >
>> > > the question that remains though is: the dtd must be an external file for
>> > > extensions to be validated. Which means we need to either pass the dtd as
>> > > argument to the scanner (requires makefile changes everywhere), or we
>> > > hardcode the path into wayland-scanner (issues with running the scanner 
>> > > from
>> > > within the source tree) or we add it as variable to pkgconfig (requires
>> > > makefile changes again). any other solutions to fix this are welcome.
>> > > even if all we do is call out to xmllint we still run into that issue.
>> >
>> > Or, hopefully we can embed the DTD file into the scanner binary - is
>> > there no way to let libxml2 read it as a plain string?
>>
>> yep, just tested it here, it's a 3-line change to the patch to load from a
>> string.
>
> Cool.
>
>> > I used the following trick to embed some files in Wesgr:
>> > https://github.com/ppaalanen/wesgr/commit/02a840cb7db7eeb80071a353f66865d729738ae5
>>
>> nice one. though if we're going for the embedded route anyway, we could just
>> have it a const char in the scanner.c file. The dtd isn't really expected to
>> change and when it does, the scanner needs to change too. Maybe we should
>> just go for the easy route?
>
> I'll let you pick the way. Should be easy to change it later if needed anyway.
>
> Would it be bad if wayland-scanner embedded the DTD and in addition we
> installed the DTD as a file too? So scanner used only its internal
> copy. I'd probably try that if someone wants the DTD as a separate file
> for their own use. In that case the trick would be handy.
>
>
> Thanks,
> pq

That sounds like the scanner will become confusing to use ("wait, so
where DOES it get the DTD from, if not this file?"), unless you'd
actually read the source code. Or this should be very clearly
documented.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland 1/3] Revert "Remove protocol/wayland.dtd"

[Auke Booij]

On 9 November 2015 at 04:14, Peter Hutterer  wrote:
> This reverts commit 06fb8bd371403d43bc192577abd6b0a0c8b29c59.
>
> Having a DTD hooked up gives an indication of what we expect the protocol to
> be, which is a clearer documentation than the current "whatever scanner.c
> manages to parse".
>
> Signed-off-by: Peter Hutterer 
> ---
>  Makefile.am  |  4 ++--
>  protocol/wayland.dtd | 29 +
>  2 files changed, 31 insertions(+), 2 deletions(-)
>  create mode 100644 protocol/wayland.dtd
>
> diff --git a/Makefile.am b/Makefile.am
> index 4c57ecf..9114d98 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -16,8 +16,8 @@ dist_aclocal_DATA = wayland-scanner.m4
>
>  dist_pkgdata_DATA =\
> wayland-scanner.mk  \
> -   protocol/wayland.xml
> -
> +   protocol/wayland.xml\
> +   protocol/wayland.dtd
>
>  pkgconfigdir = $(libdir)/pkgconfig
>  pkgconfig_DATA =
> diff --git a/protocol/wayland.dtd b/protocol/wayland.dtd
> new file mode 100644
> index 000..b8b1573
> --- /dev/null
> +++ b/protocol/wayland.dtd
> @@ -0,0 +1,29 @@
> +
> +  
> +
> +
> +  
> +  
> +
> +  
> +  
> +  
> +
> +  
> +  
> +
> +  
> +  
> +
> +  
> +  
> +  
> +  
> +
> +  
> +  
> +  
> +  
> +  
> +
> +  
> --
> 2.4.3
>
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Since the introduction of the bitfield and enum attributes, this DTD
is now incomplete. (I was hoping to settle the DTD thing so that my
patches could skip the DTD discussion altogether, but it seems my
strategy backfired.)

You additionally need:
 
 
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH 2.5/3 wayland] protocol: add the new bitfields to the dtd

[Auke Booij]

On 9 November 2015 at 23:34, Peter Hutterer <peter.hutte...@who-t.net> wrote:
> See 851614fa78862499e016c5718e730fefbb8e3b73
>
> Signed-off-by: Peter Hutterer <peter.hutte...@who-t.net>

Reviewed-by: Auke Booij <a...@tulcod.com>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH] Declare enumeration wl_output.transform as bitfield.

[Auke Booij]

On 6 November 2015 at 13:03, Nils Christopher Brause
 wrote:
> The enumeration wl_output.transform is clearly a bitfield.
> The definition of a bitfield is that each bit has a distinct
> meaning. This is clearly the case in the enumeration
> wl_output.transform:
>
> - bit 0: rotate by 90 degree
> - bit 1: rotate by 180 degree
> - bit 2: flip around vertical axis

Just to complete the definitions: what is the right order of
operations here? How do I transform from "normal" to
wl_output::transform = 0b101, for example? First rotating 90 degrees
and then flipping is not the same as first flipping and then rotating
90 degrees. In mathematical terms, the symmetry group of the square is
not a commutative group.

> Every other value can be constructed by ORing together the
> above bits:
>
> - "270" = "90" | "180"
> - "flipped_90" = "90" | "flipped"
> - "flipped_180" = "180" | "flipped"
> - "flipped_270" = "90" | "180" | "flipped"

While I agree with this, this does not seem to be used anywhere in
weston's source code. Not saying I disagree with your claim that
wl_output::transform should be considered a bitfield, but why is it
necessary to do so?

> Therefore the bitfield="true" attribute has been added to
> the enumeration declaration. Since this bitfield is
> transferred as signed integer, the scanner had to be
> modified to accept that behaviour. This was also noted in
> the documentation.

As I made clear in our previous discusions, I don't think that it is a
safe idea to allow signed integers to be bitfields. Requiring
unsignedness is a sanity check. So I would not like this patch, if
only for the reason that it invalidates this check.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH] compositor: Disallow negative geometries in backend output configs

[Auke Booij]

On 24 October 2015 at 08:23, Jasper St. Pierre  wrote:
> I'm struggling to understand the motivation for this patch.
>
> krh has always said that you need to think of uint and int as two
> entirely separate types -- mixing both in math will likely screw up.
> You can see this in other places -- widths are often expressed as
> signed ints in the protocol, not unsigned ints.

Is it maybe time to fix the protocol before it becomes a problem? Not
sure what a sane process towards this would be...
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH] doc: Use enum argument type to make links in protocol documentation

[Auke Booij]

On 6 November 2015 at 16:27,  <spit...@gmail.com> wrote:
> From: Bill Spitzak <spit...@gmail.com>

This is good preparation for when we'll get cross-interface enums.

Reviewed-by: Auke Booij <a...@tulcod.com>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland v2] Remove protocol/wayland.dtd

[Auke Booij]

On 5 November 2015 at 14:58, Pekka Paalanen  wrote:
> On Mon, 19 Oct 2015 11:30:47 +1000
> Peter Hutterer  wrote:
>
>> On Fri, Oct 16, 2015 at 11:42:21AM +0300, Pekka Paalanen wrote:
>> > On Fri, 16 Oct 2015 12:29:11 +1000
>> > Peter Hutterer  wrote:
>> >
>> > > On Fri, Oct 09, 2015 at 01:16:49PM +0200, Nils Chr. Brause wrote:
>> > > > Hi,
>> > > >
>> > > > Reviewed-by: Nils Christopher Brause 
>> > > >
>> > > > I ran distcheck and it worked. :)
>> > >
>> > > a bit late, but I would like to register my disagreement with this patch 
>> > > :)
>> > >
>> > > Having the DTD is a much simpler and less bug-prone description of what 
>> > > the
>> > > protocol should look like. Having the scanner define the protocol means 
>> > > the
>> > > protocol is whatever the current version of the scanner supports, which 
>> > > is
>> > > not a good contract.
>> >
>> > Hi Peter,
>> >
>> > can't argue with that. It's just that the DTD was unused since
>> > 6292fe2af6a45decb7fd39090e74dd87bc4e22b2, Feb 2014.
>> >
>> > > I'd argue for reverting this and fixing any mismatch if there is one. And
>> > > using the DTD to validate before the scanner even runs.
>> >
>> > We talked in IRC about this, and came to the conclusion that maybe we
>> > could have wayland-scanner invoke a validity checker against a DTD or
>> > an XSD.
>>
>> I played around with that. As a quick basic solution we can hook validation
>> with libxml2 into the scanner and print a warning if the xml doesn't
>> validate. That's less than 50 LOC and won't break things since it doesn't
>> touch the actual parsing. and it won't break existing protocols that use
>> "creative" tags, all it will do is warn, not fail. See the diff below (after
>> reverting 06fb8bd37.
>>
>> it's an ugly solution though, but without scanner tests probably the best
>> thing we can do.
>
> Yeah, I agree. I think I'd be ok going forward with this. The patch
> looks like a good start if we can solve the DTD file loading.
>
> Libxml2 dependency should probably be build-time optional so far.
>
>> > If the original objection to a DTD was because it required manually
>> > writing a lint phase in every project build system using the XML files,
>> > then having wayland-scanner invoke the check automatically solves that.
>>
>> the question that remains though is: the dtd must be an external file for
>> extensions to be validated. Which means we need to either pass the dtd as
>> argument to the scanner (requires makefile changes everywhere), or we
>> hardcode the path into wayland-scanner (issues with running the scanner from
>> within the source tree) or we add it as variable to pkgconfig (requires
>> makefile changes again). any other solutions to fix this are welcome.
>> even if all we do is call out to xmllint we still run into that issue.
>
> Or, hopefully we can embed the DTD file into the scanner binary - is
> there no way to let libxml2 read it as a plain string?
>
> I used the following trick to embed some files in Wesgr:
> https://github.com/ppaalanen/wesgr/commit/02a840cb7db7eeb80071a353f66865d729738ae5
>
>
> Thanks,
> pq


If I understand correctly, you want to require the scanner to read the
DTD. Hence, since the scanner defines the protocol, the DTD is now
officially part of the protocol. So you might as well require that
wayland.dtd can be found in the same directory as the protocol XML
file itself.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH wayland v3 3/4] scanner: enforce correct argument type for enums

[Auke Booij]

The scanner now checks whether arguments that have an associated
 have the right type.
An argument with an enum attribute must be of type int or uint,
and if the  with that name has the bitfield attribute
set to true, then the argument must be of type uint.

Changes since v3:
 - Remove useless allow_null check
 - Switch to using bool
 - Clearer message on errorous input
 - Minor formatting fix

Signed-off-by: Auke Booij <a...@tulcod.com>
---
 src/scanner.c | 69 +++
 1 file changed, 69 insertions(+)

diff --git a/src/scanner.c b/src/scanner.c
index f456aa5..c83363a 100644
--- a/src/scanner.c
+++ b/src/scanner.c
@@ -128,6 +128,7 @@ struct arg {
char *interface_name;
struct wl_list link;
char *summary;
+   char *enumeration_name;
 };

 struct enumeration {
@@ -136,6 +137,7 @@ struct enumeration {
struct wl_list entry_list;
struct wl_list link;
struct description *description;
+   bool bitfield;
 };

 struct entry {
@@ -540,6 +542,8 @@ start_element(void *data, const char *element_name, const 
char **atts)
const char *summary = NULL;
const char *since = NULL;
const char *allow_null = NULL;
+   const char *enumeration_name = NULL;
+   const char *bitfield = NULL;
int i, version = 0;

ctx->loc.line_number = XML_GetCurrentLineNumber(ctx->parser);
@@ -562,6 +566,10 @@ start_element(void *data, const char *element_name, const 
char **atts)
since = atts[i + 1];
if (strcmp(atts[i], "allow-null") == 0)
allow_null = atts[i + 1];
+   if (strcmp(atts[i], "enum") == 0)
+   enumeration_name = atts[i + 1];
+   if (strcmp(atts[i], "bitfield") == 0)
+   bitfield = atts[i + 1];
}

ctx->character_data_length = 0;
@@ -655,6 +663,11 @@ start_element(void *data, const char *element_name, const 
char **atts)
 "allow-null is only valid for objects, 
strings, and arrays");
}

+   if (enumeration_name == NULL || strcmp(enumeration_name, "") == 
0)
+   arg->enumeration_name = NULL;
+   else
+   arg->enumeration_name = xstrdup(enumeration_name);
+
if (summary)
arg->summary = xstrdup(summary);

@@ -665,6 +678,16 @@ start_element(void *data, const char *element_name, const 
char **atts)
fail(>loc, "no enum name given");

enumeration = create_enumeration(name);
+
+   if (bitfield == NULL || strcmp(bitfield, "false") == 0)
+   enumeration->bitfield = false;
+   else if (strcmp(bitfield, "true") == 0)
+   enumeration->bitfield = true;
+   else
+   fail(>loc,
+ "invalid value (%s) for bitfield attribute (only 
true/false are accepted)",
+ bitfield);
+
wl_list_insert(ctx->interface->enumeration_list.prev,
   >link);

@@ -701,6 +724,46 @@ start_element(void *data, const char *element_name, const 
char **atts)
 }

 static void
+verify_arguments(struct parse_context *ctx, struct wl_list *messages, struct 
wl_list *enumerations)
+{
+   struct message *m;
+   wl_list_for_each(m, messages, link) {
+   struct arg *a;
+   wl_list_for_each(a, >arg_list, link) {
+   struct enumeration *e, *f;
+
+   if (!a->enumeration_name)
+   continue;
+
+   f = NULL;
+   wl_list_for_each(e, enumerations, link) {
+   if(strcmp(e->name, a->enumeration_name) == 0)
+   f = e;
+   }
+
+   if (f == NULL)
+   fail(>loc,
+"could not find enumeration %s",
+a->enumeration_name);
+
+   switch (a->type) {
+   case INT:
+   if (f->bitfield)
+   fail(>loc,
+"bitfield-style enum must only be 
referenced by uint");
+   break;
+   case UNSIGNED:
+   break;
+   default:
+   fail(>loc,
+"enumeration-style argument has wrong 
type");
+   }
+ 

Re: [PATCH wayland v3 3/4] scanner: enforce correct argument type for enums

[Auke Booij]

On 26 October 2015 at 18:07, Bryce Harrington <br...@osg.samsung.com> wrote:
> On Sat, Oct 24, 2015 at 12:07:49PM +0100, Auke Booij wrote:
>> The scanner now checks whether arguments that have an associated
>>  have the right type.
>> An argument with an enum attribute must be of type int or uint,
>> and if the  with that name has the bitfield attribute
>> set to true, then the argument must be of type uint.
>>
>> Signed-off-by: Auke Booij <a...@tulcod.com>
>
> Reviewed-by: Bryce Harrington <br...@osg.samsung.com>
>
> A couple really minor nits below, not really worth doing unless you need
> to do another rev of this patch for some other reason.
>
>> ---
>>  src/scanner.c | 70 
>> +++
>>  1 file changed, 70 insertions(+)
>>
>> diff --git a/src/scanner.c b/src/scanner.c
>> index f456aa5..9856475 100644
>> --- a/src/scanner.c
>> +++ b/src/scanner.c
>> @@ -128,6 +128,7 @@ struct arg {
>>   char *interface_name;
>>   struct wl_list link;
>>   char *summary;
>> + char *enumeration_name;
>>  };
>>
>>  struct enumeration {
>> @@ -136,6 +137,7 @@ struct enumeration {
>>   struct wl_list entry_list;
>>   struct wl_list link;
>>   struct description *description;
>> + int bitfield;
>
> This appears to be used for tracking only a yes/no type value, so maybe
> consider making it a boolean?
>
>>  };
>>
>>  struct entry {
>> @@ -540,6 +542,8 @@ start_element(void *data, const char *element_name, 
>> const char **atts)
>>   const char *summary = NULL;
>>   const char *since = NULL;
>>   const char *allow_null = NULL;
>> + const char *enumeration_name = NULL;
>> + const char *bitfield = NULL;
>>   int i, version = 0;
>>
>>   ctx->loc.line_number = XML_GetCurrentLineNumber(ctx->parser);
>> @@ -562,6 +566,10 @@ start_element(void *data, const char *element_name, 
>> const char **atts)
>>   since = atts[i + 1];
>>   if (strcmp(atts[i], "allow-null") == 0)
>>   allow_null = atts[i + 1];
>> + if (strcmp(atts[i], "enum") == 0)
>> + enumeration_name = atts[i + 1];
>> + if (strcmp(atts[i], "bitfield") == 0)
>> + bitfield = atts[i + 1];
>>   }
>>
>>   ctx->character_data_length = 0;
>> @@ -655,6 +663,14 @@ start_element(void *data, const char *element_name, 
>> const char **atts)
>>"allow-null is only valid for objects, 
>> strings, and arrays");
>>   }
>>
>> + if (enumeration_name == NULL || strcmp(enumeration_name, "") 
>> == 0)
>> + arg->enumeration_name = NULL;
>> + else
>> + arg->enumeration_name = xstrdup(enumeration_name);
>> +
>> + if (allow_null != NULL && !is_nullable_type(arg))
>> + fail(>loc, "allow-null is only valid for objects, 
>> strings, and arrays");
>> +
>>   if (summary)
>>   arg->summary = xstrdup(summary);
>>
>> @@ -665,6 +681,14 @@ start_element(void *data, const char *element_name, 
>> const char **atts)
>>   fail(>loc, "no enum name given");
>>
>>   enumeration = create_enumeration(name);
>> +
>> + if (bitfield == NULL || strcmp(bitfield, "false") == 0)
>> + enumeration->bitfield = 0;
>> + else if (strcmp(bitfield, "true") == 0)
>> + enumeration->bitfield =1;
>
> Space needed after the =
>
>> + else
>> + fail(>loc, "invalid value for bitfield attribute 
>> (%s)", bitfield);
>> +
>>   wl_list_insert(ctx->interface->enumeration_list.prev,
>>  >link);
>>
>> @@ -701,6 +725,46 @@ start_element(void *data, const char *element_name, 
>> const char **atts)
>>  }
>>
>>  static void
>> +verify_arguments(struct parse_context *ctx, struct wl_list *messages, 
>> struct wl_list *enumerations)
>> +{
>> + struct message *m;
>> + wl_list_for_each(m, messages, link) {
>> + struct arg *a;
>

[PATCH wayland v3 0/4] Enum and bitfield XML attributes

[Auke Booij]

There has been plenty of discussion regarding the introduction of new XML
attributes. This series of patches improves on my earlier attempt to find
common ground in this.

Major exclusions from these patches are:

 - Support for cross-interface enum referencing (e.g.
   wl_shm_pool::create_buffer::format to wl_shm::format, and
   wl_surface::set_buffer_transform::transform to wl_output::transform).
   There is consensus that this should be added sooner rather than later.
   My proposal is to get these patches in first, and then look at that.

 - Open/closed enum specification. There is still too much debate on this.
   Until there is any firm specification of open/closed enums, every enum
   should be considered open, and although some legal values might be
   listed, others might not be, and some might only be legal sometimes.
   New values may be added (but not changed or removed) to protocol
   specifications without introducing any compatibility issues.

Changes since v2:
 - Incorporate new documentation drawn up during v2 discussion
 - Enable wl_shell_surface::transform bitfield

Changes since v1:
 - Split up documentation patches
 - Set enum="fullscreen_method" on
   wl_shell_surface::set_fullscreen::method

Auke Booij (4):
  doc: document the enum and bitfield attributes
  protocol: specify enum and bitfield attributes
  scanner: enforce correct argument type for enums
  doc: output enum and bitfield attributes in the documentation

 doc/publican/protocol-to-docbook.xsl |  9 +
 doc/publican/sources/Protocol.xml| 41 +
 protocol/wayland.xml | 36 +--
 src/scanner.c| 70 
 4 files changed, 132 insertions(+), 24 deletions(-)

--
2.6.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH wayland v3 1/4] doc: document the enum and bitfield attributes

[Auke Booij]

Introduce the enum and bitfield attributes, which allow you to refer to the enum
you are expecting in an argument, and specify which enums are to be thought of
as bitfields.

Signed-off-by: Auke Booij <a...@tulcod.com>
---
 doc/publican/sources/Protocol.xml | 41 +--
 1 file changed, 35 insertions(+), 6 deletions(-)

diff --git a/doc/publican/sources/Protocol.xml 
b/doc/publican/sources/Protocol.xml
index 477063b..c51cc72 100644
--- a/doc/publican/sources/Protocol.xml
+++ b/doc/publican/sources/Protocol.xml
@@ -15,6 +15,38 @@
   identifies which method in the interface to invoke.
 
 
+  The protocol is message-based.  A message sent by a client to the server
+  is called request.  A message from the server to a client is called 
event.
+  A message has a number of arguments, each of which has a certain type 
(see
+   for a list of argument 
types).
+
+
+  Additionally, the protocol can specify enums which associate
+  names to specific numeric enumeration values.  These are primarily just
+  description in nature: at the wire format level enums are just integers.
+  But they also serve a secondary purpose to enhance type safety or
+  otherwise add context for use in language bindings or other such code.
+  This latter usage is only supported so long as code written before these
+  attributes were introduced still works after; in other words, adding an
+  enum should not break API, otherwise it puts backwards compatibility at
+  risk.
+
+
+  enums can be defined as just a set of integers, or as
+  bitfields.  This is specified via the bitfield boolean
+  attribute in the enum definition.  If this attribute is 
true,
+  the enum is intended to be accessed primarily using bitwise operations,
+  for example when arbitrarily many choices of the enum can be ORed
+  together; if it is false, or the attribute is omitted, then the enum
+  arguments are a just a sequence of numerical values.
+
+
+  The enum attribute can be used on either uint
+  or int arguments, however if the enum is
+  defined as a bitfield, it can only be used on
+  uint args.
+
+
   The server sends back events to the client, each event is emitted from
   an object.  Events can be error conditions.  The event includes the
   object ID and the event opcode, from which the client can determine
@@ -62,14 +94,11 @@
   The protocol is sent over a UNIX domain stream socket, where the endpoint
   usually is named wayland-0
   (although it can be changed via WAYLAND_DISPLAY
-  in the environment).  The protocol is message-based.  A
-  message sent by a client to the server is called request.  A message
-  from the server to a client is called event.  Every message is
-  structured as 32-bit words, values are represented in the host's
-  byte-order.
+  in the environment).
 
 
-  The message header has 2 words in it:
+  Every message is structured as 32-bit words; values are represented in 
the
+  host's byte-order.  The message header has 2 words in it:
   

  
-- 
2.6.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH wayland v3 3/4] scanner: enforce correct argument type for enums

[Auke Booij]

The scanner now checks whether arguments that have an associated
 have the right type.
An argument with an enum attribute must be of type int or uint,
and if the  with that name has the bitfield attribute
set to true, then the argument must be of type uint.

Signed-off-by: Auke Booij <a...@tulcod.com>
---
 src/scanner.c | 70 +++
 1 file changed, 70 insertions(+)

diff --git a/src/scanner.c b/src/scanner.c
index f456aa5..9856475 100644
--- a/src/scanner.c
+++ b/src/scanner.c
@@ -128,6 +128,7 @@ struct arg {
char *interface_name;
struct wl_list link;
char *summary;
+   char *enumeration_name;
 };
 
 struct enumeration {
@@ -136,6 +137,7 @@ struct enumeration {
struct wl_list entry_list;
struct wl_list link;
struct description *description;
+   int bitfield;
 };
 
 struct entry {
@@ -540,6 +542,8 @@ start_element(void *data, const char *element_name, const 
char **atts)
const char *summary = NULL;
const char *since = NULL;
const char *allow_null = NULL;
+   const char *enumeration_name = NULL;
+   const char *bitfield = NULL;
int i, version = 0;
 
ctx->loc.line_number = XML_GetCurrentLineNumber(ctx->parser);
@@ -562,6 +566,10 @@ start_element(void *data, const char *element_name, const 
char **atts)
since = atts[i + 1];
if (strcmp(atts[i], "allow-null") == 0)
allow_null = atts[i + 1];
+   if (strcmp(atts[i], "enum") == 0)
+   enumeration_name = atts[i + 1];
+   if (strcmp(atts[i], "bitfield") == 0)
+   bitfield = atts[i + 1];
}
 
ctx->character_data_length = 0;
@@ -655,6 +663,14 @@ start_element(void *data, const char *element_name, const 
char **atts)
 "allow-null is only valid for objects, 
strings, and arrays");
}
 
+   if (enumeration_name == NULL || strcmp(enumeration_name, "") == 
0)
+   arg->enumeration_name = NULL;
+   else
+   arg->enumeration_name = xstrdup(enumeration_name);
+
+   if (allow_null != NULL && !is_nullable_type(arg))
+   fail(>loc, "allow-null is only valid for objects, 
strings, and arrays");
+
if (summary)
arg->summary = xstrdup(summary);
 
@@ -665,6 +681,14 @@ start_element(void *data, const char *element_name, const 
char **atts)
fail(>loc, "no enum name given");
 
enumeration = create_enumeration(name);
+
+   if (bitfield == NULL || strcmp(bitfield, "false") == 0)
+   enumeration->bitfield = 0;
+   else if (strcmp(bitfield, "true") == 0)
+   enumeration->bitfield =1;
+   else
+   fail(>loc, "invalid value for bitfield attribute 
(%s)", bitfield);
+
wl_list_insert(ctx->interface->enumeration_list.prev,
   >link);
 
@@ -701,6 +725,46 @@ start_element(void *data, const char *element_name, const 
char **atts)
 }
 
 static void
+verify_arguments(struct parse_context *ctx, struct wl_list *messages, struct 
wl_list *enumerations)
+{
+   struct message *m;
+   wl_list_for_each(m, messages, link) {
+   struct arg *a;
+   wl_list_for_each(a, >arg_list, link) {
+   struct enumeration *e, *f;
+
+   if (!a->enumeration_name)
+   continue;
+
+   f = NULL;
+   wl_list_for_each(e, enumerations, link) {
+   if(strcmp(e->name, a->enumeration_name) == 0)
+   f = e;
+   }
+
+   if (f == NULL)
+   fail(>loc,
+"could not find enumeration %s",
+a->enumeration_name);
+
+   switch (a->type) {
+   case INT:
+   if (f->bitfield)
+   fail(>loc,
+"bitfield-style enum must be 
referenced by uint");
+   break;
+   case UNSIGNED:
+   break;
+   default:
+   fail(>loc,
+"enumeration-style argument has wrong 
type");
+   }
+   }
+   }
+
+}
+
+static

[PATCH wayland v3 2/4] protocol: specify enum and bitfield attributes

[Auke Booij]

Signed-off-by: Auke Booij <a...@tulcod.com>
---
 protocol/wayland.xml | 36 ++--
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/protocol/wayland.xml b/protocol/wayland.xml
index 59819e9..9c22d45 100644
--- a/protocol/wayland.xml
+++ b/protocol/wayland.xml
@@ -367,7 +367,7 @@
can be used for buffers. Known formats include
argb and xrgb.
   
-  
+  
 
   
 
@@ -746,7 +746,7 @@
   
 
 
-
+
   
These values are used to indicate which edge of a surface
is being dragged in a resize operation. The server may
@@ -774,7 +774,7 @@
   
   
   
-  
+  
 
 
 
@@ -785,7 +785,7 @@
   
 
 
-
+
   
These flags specify details of the expected behaviour
of transient surfaces. Used in the set_transient request.
@@ -807,7 +807,7 @@
   
   
   
-  
+  
 
 
 
@@ -858,7 +858,7 @@
with the dimensions for the output on which the surface will
be made fullscreen.
   
-  
+  
   
   
 
@@ -891,7 +891,7 @@
   
   
   
-  
+  
 
 
 
@@ -972,7 +972,7 @@
in surface local coordinates.
   
 
-  
+  
   
   
 
@@ -1337,7 +1337,7 @@
   maintains a keyboard focus and a pointer focus.
 
 
-
+
   
 This is a bitmask of capabilities this seat has; if a member is
 set, then it is present on the seat.
@@ -1353,7 +1353,7 @@
keyboard or touch capabilities.  The argument is a capability
enum containing the complete set of capabilities this seat has.
   
-  
+  
 
 
 
@@ -1530,7 +1530,7 @@
   
   
   
-  
+  
 
 
 
@@ -1562,7 +1562,7 @@
   
 
   
-  
+  
   
 
 
@@ -1602,7 +1602,7 @@
This event provides a file descriptor to the client which can be
memory-mapped to provide a keyboard mapping description.
   
-  
+  
   
   
 
@@ -1647,7 +1647,7 @@
   
   
   
-  
+  
 
 
 
@@ -1828,17 +1828,17 @@
   summary="width in millimeters of the output"/>
   
-  
   
   
-  
 
 
-
+
   
These flags describe properties of an output mode.
They are used in the flags bitfield of the mode event.
@@ -1865,7 +1865,7 @@
 the output may be scaled, as described in wl_output.scale,
 or transformed , as described in wl_output.transform.
   
-  
+  
   
   
   
-- 
2.6.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH v2 1/4] doc: document the enum and bitfield attributes

[Auke Booij]

On 22 October 2015 at 02:46, Bill Spitzak <spit...@gmail.com> wrote:
> Sorry if this is a duplicate, I am having trouble with gmail.
>
> On 10/20/2015 12:38 AM, Bryce Harrington wrote:
>>
>> On Tue, Oct 20, 2015 at 12:01:14AM -0700, Bryce Harrington wrote:
>>>
>>> On Mon, Oct 19, 2015 at 11:21:23PM +0100, Auke Booij wrote:
>>>>
>>>> Introduce the enum and bitfield attributes, which allow you to refer to
>>>> the enum
>>>> you are expecting in an argument, and specify which enums are to be
>>>> thought of
>>>> as bitfields.
>>>>
>>>> +  Additionally, the protocol can specify enums.  These
>>>> are used
>>>> +  to list options for int and uint type
>>>> arguments.
>>>> +  Arguments can refer to the specific enumeration that is
>>>> semantically
>>>> +  implied.  Only in the case that the argument is of type
>>>> uint,
>>>> +  it can be specified that the primary interface to its numeric
>>>> value deals
>>>> +  with bitwise operations, for example when arbitrarily many
>>>> choices of the
>>>> +  enum can be ORed together.
>>>> +
>>>> +
>>>> +  The purpose of the enum and bitfield
>>>> attributes
>>>> +  is to document what arguments refer to which enums, and to
>>>> document which
>>>> +  numeric enum values are primarily accessed using bitwise
>>>> operations.
>>>> +  Additionally, the enum and bitfield attributes may be used by
>>>> other code,
>>>> +  such as bindings to other languages, for example to enhance type
>>>> safety of
>>>> +  code.  However, such usage is only supported if the following
>>>> property is
>>>> +  satisfied: code written prior to the specification of these
>>>> attributes
>>>> +  still works after their specification.  In other words,
>>>> specifying an
>>>> +  attribute for an argument, that previously did not have an enum
>>>> or
>>>> +  bitfield attribute, should not break API.  Code that does not
>>>> satisfy this
>>>> +  rule is not guaranteed to obey backwards compatibility.
>>>
>>> This next chunk gets a bit too jarringly technical too quickly.  I think
>>> your second paragraph gives a better intro to these attributes, but it
>>> doesn't work to simply swap them.  Let me take a shot at copyediting
>>> this a bit:
>>>
>>> I think this is clearer, and hopefully hasn't lost any meaning.  I'm not
>>> sure it's improved the technicality of this prose...  perhaps this
>>> section would be better promoted to its own section, with maybe just a
>>> reference sentence included here?  Not sure.
>>
>> I'm noticing now that I've misunderstood what the bitfield attribute is;
>> so the above text is incorrect.  Let me try again.
>>   Additionally, the protocol can specify enums which
>> associate specific numeric enumeration values.  These are
>> primarily just description in nature: at the wire format level
>> enums are just integers.  But they also serve a secondary purpose
>> to enhance type safety or otherwise add context for use in
>> language bindings or other such code.  This latter usage is only
>> supported so long as code written before these attributes were
>> introduced still works after; in other words, adding an enum
>> should not break API, otherwise it puts backwards compatibility
>> at risk.
>>
>> enums can be defined as bitfields or just a set of
>> integers.  This is specified via the bitfield
>> boolean attribute in the enum definition.  If this
>> attribute is true, the enum is intended to be accessed primarily
>> using bitwise operations, for example when arbitrarily many
>> choices of the enum can be ORed together; if it is false, or the
>> attribute is omitted, then the enum arguments are a just a
>> sequence of numerical values.
>>
>> The enum attribute can be used on either
>> uint or int arguments, however if the
>> enum is defined as a bitfield, it can
>> only be used on uint args.
>
>
> This version definitely better, though you might be able to skip the "This
> is specified via..."

Re: [PATCH v2 3/4] scanner: enforce correct argument type for enums

[Auke Booij]

On 21 October 2015 at 19:13, Bryce Harrington <br...@osg.samsung.com> wrote:
> On Wed, Oct 21, 2015 at 02:23:53PM +0100, Auke Booij wrote:
>> On 20 October 2015 at 08:57, Bryce Harrington <br...@osg.samsung.com> wrote:
>> > On Mon, Oct 19, 2015 at 11:21:25PM +0100, Auke Booij wrote:
>> >> Signed-off-by: Auke Booij <a...@tulcod.com>
>> >> ---
>> >>  src/scanner.c | 70 
>> >> +++
>> >>  1 file changed, 70 insertions(+)
>> >>
>> >> diff --git a/src/scanner.c b/src/scanner.c
>> >> index f456aa5..9856475 100644
>> >> --- a/src/scanner.c
>> >> +++ b/src/scanner.c
>> >> @@ -128,6 +128,7 @@ struct arg {
>> >>   char *interface_name;
>> >>   struct wl_list link;
>> >>   char *summary;
>> >> + char *enumeration_name;
>> >>  };
>> >>
>> >>  struct enumeration {
>> >> @@ -136,6 +137,7 @@ struct enumeration {
>> >>   struct wl_list entry_list;
>> >>   struct wl_list link;
>> >>   struct description *description;
>> >> + int bitfield;
>> >
>> > Looks like this code only sets this to 0 or 1; may as well make this bool.
>> > Other places in scanner.c use bool, so seems to be perfectly allowable.
>>
>> Sure, I will change this.
>>
>> >>  };
>> >>
>> >>  struct entry {
>> >> @@ -540,6 +542,8 @@ start_element(void *data, const char *element_name, 
>> >> const char **atts)
>> >>   const char *summary = NULL;
>> >>   const char *since = NULL;
>> >>   const char *allow_null = NULL;
>> >> + const char *enumeration_name = NULL;
>> >> + const char *bitfield = NULL;
>> >>   int i, version = 0;
>> >>
>> >>   ctx->loc.line_number = XML_GetCurrentLineNumber(ctx->parser);
>> >> @@ -562,6 +566,10 @@ start_element(void *data, const char *element_name, 
>> >> const char **atts)
>> >>   since = atts[i + 1];
>> >>   if (strcmp(atts[i], "allow-null") == 0)
>> >>   allow_null = atts[i + 1];
>> >> + if (strcmp(atts[i], "enum") == 0)
>> >> + enumeration_name = atts[i + 1];
>> >> + if (strcmp(atts[i], "bitfield") == 0)
>> >> + bitfield = atts[i + 1];
>> >>   }
>> >>
>> >>   ctx->character_data_length = 0;
>> >> @@ -655,6 +663,14 @@ start_element(void *data, const char *element_name, 
>> >> const char **atts)
>> >>"allow-null is only valid for objects, 
>> >> strings, and arrays");
>> >>   }
>> >>
>> >> + if (enumeration_name == NULL || strcmp(enumeration_name, 
>> >> "") == 0)
>> >> + arg->enumeration_name = NULL;
>> >> + else
>> >> + arg->enumeration_name = xstrdup(enumeration_name);
>> >> +
>> >> + if (allow_null != NULL && !is_nullable_type(arg))
>> >> + fail(>loc, "allow-null is only valid for 
>> >> objects, strings, and arrays");
>> >> +
>> >>   if (summary)
>> >>   arg->summary = xstrdup(summary);
>> >>
>> >> @@ -665,6 +681,14 @@ start_element(void *data, const char *element_name, 
>> >> const char **atts)
>> >>   fail(>loc, "no enum name given");
>> >>
>> >>   enumeration = create_enumeration(name);
>> >> +
>> >> + if (bitfield == NULL || strcmp(bitfield, "false") == 0)
>> >> + enumeration->bitfield = 0;
>> >> + else if (strcmp(bitfield, "true") == 0)
>> >> + enumeration->bitfield =1;
>> >
>> > Would it be worth allowing True/False as well?
>>
>> Well this might be worth thinking about a bit more. I don't think
>> there are any boolean fields in other wayland configuration (correct
>> me if I'm wrong), so whatever we choose, it will set a precedence.
>> Should I als

[PATCH] doc: document the enum and bitfield attributes

[Auke Booij]

Introduce the enum and bitfield attributes, which allow you to refer to the enum
you are expecting in an argument, and specify which enums are to be thought of
as bitfields.

Signed-off-by: Auke Booij <a...@tulcod.com>
---
 doc/publican/sources/Protocol.xml | 41 +--
 1 file changed, 35 insertions(+), 6 deletions(-)

diff --git a/doc/publican/sources/Protocol.xml 
b/doc/publican/sources/Protocol.xml
index 477063b..c51cc72 100644
--- a/doc/publican/sources/Protocol.xml
+++ b/doc/publican/sources/Protocol.xml
@@ -15,6 +15,38 @@
   identifies which method in the interface to invoke.
 
 
+  The protocol is message-based.  A message sent by a client to the server
+  is called request.  A message from the server to a client is called 
event.
+  A message has a number of arguments, each of which has a certain type 
(see
+   for a list of argument 
types).
+
+
+  Additionally, the protocol can specify enums which associate
+  names to specific numeric enumeration values.  These are primarily just
+  description in nature: at the wire format level enums are just integers.
+  But they also serve a secondary purpose to enhance type safety or
+  otherwise add context for use in language bindings or other such code.
+  This latter usage is only supported so long as code written before these
+  attributes were introduced still works after; in other words, adding an
+  enum should not break API, otherwise it puts backwards compatibility at
+  risk.
+
+
+  enums can be defined as just a set of integers, or as
+  bitfields.  This is specified via the bitfield boolean
+  attribute in the enum definition.  If this attribute is 
true,
+  the enum is intended to be accessed primarily using bitwise operations,
+  for example when arbitrarily many choices of the enum can be ORed
+  together; if it is false, or the attribute is omitted, then the enum
+  arguments are a just a sequence of numerical values.
+
+
+  The enum attribute can be used on either uint
+  or int arguments, however if the enum is
+  defined as a bitfield, it can only be used on
+  uint args.
+
+
   The server sends back events to the client, each event is emitted from
   an object.  Events can be error conditions.  The event includes the
   object ID and the event opcode, from which the client can determine
@@ -62,14 +94,11 @@
   The protocol is sent over a UNIX domain stream socket, where the endpoint
   usually is named wayland-0
   (although it can be changed via WAYLAND_DISPLAY
-  in the environment).  The protocol is message-based.  A
-  message sent by a client to the server is called request.  A message
-  from the server to a client is called event.  Every message is
-  structured as 32-bit words, values are represented in the host's
-  byte-order.
+  in the environment).
 
 
-  The message header has 2 words in it:
+  Every message is structured as 32-bit words; values are represented in 
the
+  host's byte-order.  The message header has 2 words in it:
   

  
-- 
2.6.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH v2 1/4] doc: document the enum and bitfield attributes

[Auke Booij]

On 21 October 2015 at 13:13, Nils Chr. Brause <nilschrbra...@gmail.com> wrote:
> Hi,
>
> On Tue, Oct 20, 2015 at 9:38 AM, Bryce Harrington <br...@osg.samsung.com> 
> wrote:
>> On Tue, Oct 20, 2015 at 12:01:14AM -0700, Bryce Harrington wrote:
>>> On Mon, Oct 19, 2015 at 11:21:23PM +0100, Auke Booij wrote:
>>> > Introduce the enum and bitfield attributes, which allow you to refer to 
>>> > the enum
>>> > you are expecting in an argument, and specify which enums are to be 
>>> > thought of
>>> > as bitfields.
>>> >
>>> > +  Additionally, the protocol can specify enums.  These 
>>> > are used
>>> > +  to list options for int and uint type 
>>> > arguments.
>>> > +  Arguments can refer to the specific enumeration that is 
>>> > semantically
>>> > +  implied.  Only in the case that the argument is of type 
>>> > uint,
>>> > +  it can be specified that the primary interface to its numeric 
>>> > value deals
>>> > +  with bitwise operations, for example when arbitrarily many choices 
>>> > of the
>>> > +  enum can be ORed together.
>>> > +
>>> > +
>>> > +  The purpose of the enum and bitfield 
>>> > attributes
>>> > +  is to document what arguments refer to which enums, and to 
>>> > document which
>>> > +  numeric enum values are primarily accessed using bitwise 
>>> > operations.
>>> > +  Additionally, the enum and bitfield attributes may be used by 
>>> > other code,
>>> > +  such as bindings to other languages, for example to enhance type 
>>> > safety of
>>> > +  code.  However, such usage is only supported if the following 
>>> > property is
>>> > +  satisfied: code written prior to the specification of these 
>>> > attributes
>>> > +  still works after their specification.  In other words, specifying 
>>> > an
>>> > +  attribute for an argument, that previously did not have an enum or
>>> > +  bitfield attribute, should not break API.  Code that does not 
>>> > satisfy this
>>> > +  rule is not guaranteed to obey backwards compatibility.
>>>
>>> This next chunk gets a bit too jarringly technical too quickly.  I think
>>> your second paragraph gives a better intro to these attributes, but it
>>> doesn't work to simply swap them.  Let me take a shot at copyediting
>>> this a bit:
>>>
>>> I think this is clearer, and hopefully hasn't lost any meaning.  I'm not
>>> sure it's improved the technicality of this prose...  perhaps this
>>> section would be better promoted to its own section, with maybe just a
>>> reference sentence included here?  Not sure.
>>
>> I'm noticing now that I've misunderstood what the bitfield attribute is;
>> so the above text is incorrect.  Let me try again.
>>
>>Additionally, the protocol can specify enums which
>>associate specific numeric enumeration values.  These are
>>primarily just description in nature: at the wire format level
>>enums are just integers.  But they also serve a secondary purpose
>>to enhance type safety or otherwise add context for use in
>>language bindings or other such code.  This latter usage is only
>>supported so long as code written before these attributes were
>>introduced still works after; in other words, adding an enum
>>should not break API, otherwise it puts backwards compatibility
>>at risk.
>>
>>enums can be defined as bitfields or just a set of
>>integers.  This is specified via the bitfield
>>boolean attribute in the enum definition.  If this
>>attribute is true, the enum is intended to be accessed primarily
>>using bitwise operations, for example when arbitrarily many
>>choices of the enum can be ORed together; if it is false, or the
>>attribute is omitted, then the enum arguments are a just a
>>sequence of numerical values.
>
> I  am fine with that wording, but it actually is much simpler than that:
> In a bitfield every bit has a distinct meaning. In an enumeration, that
> is not the case. :)

Like any suggestion us foreign language binders make, while yours is
perfectly reasonable in principle, C's abuse of everything makes me
want to be a bit careful in this. Add

Re: [PATCH v2 2/4] protocol: specify enum and bitfield attributes

[Auke Booij]

On 21 October 2015 at 18:49, Nils Chr. Brause  wrote:
> You are missing bitfield="true" for wl_shell_surface::resize and
> wl_output::transform.

Scanning the weston source code, it seems you would have been right
about wl_shell_surface had it not been replaced by xdg_shell_surface.
But I am happy to add this for wl_shell_surface.

wl_output::transform is not missing, I deliberately did not add this.
Although it looks like a bit field at first sight, (according to the
weston source code) its primary interface is case-wise processing. I
could not find a single bitwise operation on it in the weston source
code. If you'd like to add this (I would not be an opponent because I
can see the value of bitfield="true" here), I propose you send in a
later patch, because it will probably stir up a new discussion that
I'd like to keep outside of the current one (of whether to do this at
all).
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH v2 3/4] scanner: enforce correct argument type for enums

[Auke Booij]

On 20 October 2015 at 08:57, Bryce Harrington <br...@osg.samsung.com> wrote:
> On Mon, Oct 19, 2015 at 11:21:25PM +0100, Auke Booij wrote:
>> Signed-off-by: Auke Booij <a...@tulcod.com>
>> ---
>>  src/scanner.c | 70 
>> +++
>>  1 file changed, 70 insertions(+)
>>
>> diff --git a/src/scanner.c b/src/scanner.c
>> index f456aa5..9856475 100644
>> --- a/src/scanner.c
>> +++ b/src/scanner.c
>> @@ -128,6 +128,7 @@ struct arg {
>>   char *interface_name;
>>   struct wl_list link;
>>   char *summary;
>> + char *enumeration_name;
>>  };
>>
>>  struct enumeration {
>> @@ -136,6 +137,7 @@ struct enumeration {
>>   struct wl_list entry_list;
>>   struct wl_list link;
>>   struct description *description;
>> + int bitfield;
>
> Looks like this code only sets this to 0 or 1; may as well make this bool.
> Other places in scanner.c use bool, so seems to be perfectly allowable.

Sure, I will change this.

>>  };
>>
>>  struct entry {
>> @@ -540,6 +542,8 @@ start_element(void *data, const char *element_name, 
>> const char **atts)
>>   const char *summary = NULL;
>>   const char *since = NULL;
>>   const char *allow_null = NULL;
>> + const char *enumeration_name = NULL;
>> + const char *bitfield = NULL;
>>   int i, version = 0;
>>
>>   ctx->loc.line_number = XML_GetCurrentLineNumber(ctx->parser);
>> @@ -562,6 +566,10 @@ start_element(void *data, const char *element_name, 
>> const char **atts)
>>   since = atts[i + 1];
>>   if (strcmp(atts[i], "allow-null") == 0)
>>   allow_null = atts[i + 1];
>> + if (strcmp(atts[i], "enum") == 0)
>> + enumeration_name = atts[i + 1];
>> + if (strcmp(atts[i], "bitfield") == 0)
>> + bitfield = atts[i + 1];
>>   }
>>
>>   ctx->character_data_length = 0;
>> @@ -655,6 +663,14 @@ start_element(void *data, const char *element_name, 
>> const char **atts)
>>"allow-null is only valid for objects, 
>> strings, and arrays");
>>   }
>>
>> + if (enumeration_name == NULL || strcmp(enumeration_name, "") 
>> == 0)
>> + arg->enumeration_name = NULL;
>> + else
>> + arg->enumeration_name = xstrdup(enumeration_name);
>> +
>> + if (allow_null != NULL && !is_nullable_type(arg))
>> + fail(>loc, "allow-null is only valid for objects, 
>> strings, and arrays");
>> +
>>   if (summary)
>>   arg->summary = xstrdup(summary);
>>
>> @@ -665,6 +681,14 @@ start_element(void *data, const char *element_name, 
>> const char **atts)
>>   fail(>loc, "no enum name given");
>>
>>   enumeration = create_enumeration(name);
>> +
>> + if (bitfield == NULL || strcmp(bitfield, "false") == 0)
>> + enumeration->bitfield = 0;
>> + else if (strcmp(bitfield, "true") == 0)
>> + enumeration->bitfield =1;
>
> Would it be worth allowing True/False as well?

Well this might be worth thinking about a bit more. I don't think
there are any boolean fields in other wayland configuration (correct
me if I'm wrong), so whatever we choose, it will set a precedence.
Should I also allow 0/1? Yes/No? (Xorg seems to be rather lax in all
this.)

>> + else
>> + fail(>loc, "invalid value for bitfield attribute 
>> (%s)", bitfield);
>
> May as well say here that only true/false are accepted in bitfields.
> Might save anyone that hits this from a trip to the documentation.

I will fix this.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH v2 0/4] Enum and bitfield XML attributes

[Auke Booij]

On 19 October 2015 at 23:21, Auke Booij <a...@tulcod.com> wrote:
> There has been plenty of discussion regarding the introduction of new XML
> attributes. This series of patches improves on my earlier attempt to find
> common ground in this.
>
> Major exclusions from these patches are:
>
> - Support for cross-interface enum referencing (e.g.
> wl_shm_pool::create_buffer::format to wl_shm::format, and
> wl_surface::set_buffer_transform::transform to wl_output::transform).

Just to be clear on this, I think there is consensus that this should
be added in a later stage.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH v2 1/4] doc: document the enum and bitfield attributes

[Auke Booij]

Introduce the enum and bitfield attributes, which allow you to refer to the enum
you are expecting in an argument, and specify which enums are to be thought of
as bitfields.

Signed-off-by: Auke Booij <a...@tulcod.com>
---
 doc/publican/sources/Protocol.xml | 35 +--
 1 file changed, 29 insertions(+), 6 deletions(-)

diff --git a/doc/publican/sources/Protocol.xml 
b/doc/publican/sources/Protocol.xml
index 477063b..c6178b7 100644
--- a/doc/publican/sources/Protocol.xml
+++ b/doc/publican/sources/Protocol.xml
@@ -15,6 +15,32 @@
   identifies which method in the interface to invoke.
 
 
+  The protocol is message-based.  A message sent by a client to the server
+  is called request.  A message from the server to a client is called 
event.
+  A message has a number of arguments, each of which has a certain type 
(see
+   for a list of argument 
types).
+  Additionally, the protocol can specify enums.  These are 
used
+  to list options for int and uint type 
arguments.
+  Arguments can refer to the specific enumeration that is semantically
+  implied.  Only in the case that the argument is of type 
uint,
+  it can be specified that the primary interface to its numeric value deals
+  with bitwise operations, for example when arbitrarily many choices of the
+  enum can be ORed together.
+
+
+  The purpose of the enum and bitfield attributes
+  is to document what arguments refer to which enums, and to document which
+  numeric enum values are primarily accessed using bitwise operations.
+  Additionally, the enum and bitfield attributes may be used by other code,
+  such as bindings to other languages, for example to enhance type safety 
of
+  code.  However, such usage is only supported if the following property is
+  satisfied: code written prior to the specification of these attributes
+  still works after their specification.  In other words, specifying an
+  attribute for an argument, that previously did not have an enum or
+  bitfield attribute, should not break API.  Code that does not satisfy 
this
+  rule is not guaranteed to obey backwards compatibility.
+
+
   The server sends back events to the client, each event is emitted from
   an object.  Events can be error conditions.  The event includes the
   object ID and the event opcode, from which the client can determine
@@ -62,14 +88,11 @@
   The protocol is sent over a UNIX domain stream socket, where the endpoint
   usually is named wayland-0
   (although it can be changed via WAYLAND_DISPLAY
-  in the environment).  The protocol is message-based.  A
-  message sent by a client to the server is called request.  A message
-  from the server to a client is called event.  Every message is
-  structured as 32-bit words, values are represented in the host's
-  byte-order.
+  in the environment).
 
 
-  The message header has 2 words in it:
+  Every message is structured as 32-bit words, values are represented in 
the
+  host's byte-order.  The message header has 2 words in it:
   

  
-- 
2.6.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH v2 0/4] Enum and bitfield XML attributes

[Auke Booij]

There has been plenty of discussion regarding the introduction of new XML
attributes. This series of patches improves on my earlier attempt to find
common ground in this.

Major exclusions from these patches are:

- Support for cross-interface enum referencing (e.g.
wl_shm_pool::create_buffer::format to wl_shm::format, and
wl_surface::set_buffer_transform::transform to wl_output::transform).

- Open/closed enum specification. There is still too much debate in this.
Until there is any firm specification of open/closed enums, every enum 
should be considered open, and although some legal values might be listed,
others might not be, and some might only be legal sometimes. New values
may be added (but not changed or removed) to protocol specifications
without introducing any compatibility issues.

Auke Booij (4):
  doc: document the enum and bitfield attributes
  protocol: specify enum and bitfield attributes
  scanner: enforce correct argument type for enums
  doc: output enum and bitfield attributes in the documentation

 doc/publican/protocol-to-docbook.xsl |  9 +
 doc/publican/sources/Protocol.xml| 35 ++
 protocol/wayland.xml | 34 +-
 src/scanner.c| 70 
 4 files changed, 125 insertions(+), 23 deletions(-)

-- 
2.6.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH v2 2/4] protocol: specify enum and bitfield attributes

[Auke Booij]

Signed-off-by: Auke Booij <a...@tulcod.com>
---
 protocol/wayland.xml | 34 +-
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/protocol/wayland.xml b/protocol/wayland.xml
index 59819e9..a3e6900 100644
--- a/protocol/wayland.xml
+++ b/protocol/wayland.xml
@@ -367,7 +367,7 @@
can be used for buffers. Known formats include
argb and xrgb.
   
-  
+  
 
   
 
@@ -774,7 +774,7 @@
   
   
   
-  
+  
 
 
 
@@ -785,7 +785,7 @@
   
 
 
-
+
   
These flags specify details of the expected behaviour
of transient surfaces. Used in the set_transient request.
@@ -807,7 +807,7 @@
   
   
   
-  
+  
 
 
 
@@ -858,7 +858,7 @@
with the dimensions for the output on which the surface will
be made fullscreen.
   
-  
+  
   
   
 
@@ -891,7 +891,7 @@
   
   
   
-  
+  
 
 
 
@@ -972,7 +972,7 @@
in surface local coordinates.
   
 
-  
+  
   
   
 
@@ -1337,7 +1337,7 @@
   maintains a keyboard focus and a pointer focus.
 
 
-
+
   
 This is a bitmask of capabilities this seat has; if a member is
 set, then it is present on the seat.
@@ -1353,7 +1353,7 @@
keyboard or touch capabilities.  The argument is a capability
enum containing the complete set of capabilities this seat has.
   
-  
+  
 
 
 
@@ -1530,7 +1530,7 @@
   
   
   
-  
+  
 
 
 
@@ -1562,7 +1562,7 @@
   
 
   
-  
+  
   
 
 
@@ -1602,7 +1602,7 @@
This event provides a file descriptor to the client which can be
memory-mapped to provide a keyboard mapping description.
   
-  
+  
   
   
 
@@ -1647,7 +1647,7 @@
   
   
   
-  
+  
 
 
 
@@ -1828,17 +1828,17 @@
   summary="width in millimeters of the output"/>
   
-  
   
   
-  
 
 
-
+
   
These flags describe properties of an output mode.
They are used in the flags bitfield of the mode event.
@@ -1865,7 +1865,7 @@
 the output may be scaled, as described in wl_output.scale,
 or transformed , as described in wl_output.transform.
   
-  
+  
   
   
   
-- 
2.6.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH v2 3/4] scanner: enforce correct argument type for enums

[Auke Booij]

The scanner now checks whether arguments that have an associated
 have the right type.
An argument with an enum attribute must be of type int or uint,
and if the  with that name has the bitfield attribute
set to true, then the argument must be of type uint.

Signed-off-by: Auke Booij <a...@tulcod.com>
---
 src/scanner.c | 70 +++
 1 file changed, 70 insertions(+)

diff --git a/src/scanner.c b/src/scanner.c
index f456aa5..9856475 100644
--- a/src/scanner.c
+++ b/src/scanner.c
@@ -128,6 +128,7 @@ struct arg {
char *interface_name;
struct wl_list link;
char *summary;
+   char *enumeration_name;
 };
 
 struct enumeration {
@@ -136,6 +137,7 @@ struct enumeration {
struct wl_list entry_list;
struct wl_list link;
struct description *description;
+   int bitfield;
 };
 
 struct entry {
@@ -540,6 +542,8 @@ start_element(void *data, const char *element_name, const 
char **atts)
const char *summary = NULL;
const char *since = NULL;
const char *allow_null = NULL;
+   const char *enumeration_name = NULL;
+   const char *bitfield = NULL;
int i, version = 0;
 
ctx->loc.line_number = XML_GetCurrentLineNumber(ctx->parser);
@@ -562,6 +566,10 @@ start_element(void *data, const char *element_name, const 
char **atts)
since = atts[i + 1];
if (strcmp(atts[i], "allow-null") == 0)
allow_null = atts[i + 1];
+   if (strcmp(atts[i], "enum") == 0)
+   enumeration_name = atts[i + 1];
+   if (strcmp(atts[i], "bitfield") == 0)
+   bitfield = atts[i + 1];
}
 
ctx->character_data_length = 0;
@@ -655,6 +663,14 @@ start_element(void *data, const char *element_name, const 
char **atts)
 "allow-null is only valid for objects, 
strings, and arrays");
}
 
+   if (enumeration_name == NULL || strcmp(enumeration_name, "") == 
0)
+   arg->enumeration_name = NULL;
+   else
+   arg->enumeration_name = xstrdup(enumeration_name);
+
+   if (allow_null != NULL && !is_nullable_type(arg))
+   fail(>loc, "allow-null is only valid for objects, 
strings, and arrays");
+
if (summary)
arg->summary = xstrdup(summary);
 
@@ -665,6 +681,14 @@ start_element(void *data, const char *element_name, const 
char **atts)
fail(>loc, "no enum name given");
 
enumeration = create_enumeration(name);
+
+   if (bitfield == NULL || strcmp(bitfield, "false") == 0)
+   enumeration->bitfield = 0;
+   else if (strcmp(bitfield, "true") == 0)
+   enumeration->bitfield =1;
+   else
+   fail(>loc, "invalid value for bitfield attribute 
(%s)", bitfield);
+
wl_list_insert(ctx->interface->enumeration_list.prev,
   >link);
 
@@ -701,6 +725,46 @@ start_element(void *data, const char *element_name, const 
char **atts)
 }
 
 static void
+verify_arguments(struct parse_context *ctx, struct wl_list *messages, struct 
wl_list *enumerations)
+{
+   struct message *m;
+   wl_list_for_each(m, messages, link) {
+   struct arg *a;
+   wl_list_for_each(a, >arg_list, link) {
+   struct enumeration *e, *f;
+
+   if (!a->enumeration_name)
+   continue;
+
+   f = NULL;
+   wl_list_for_each(e, enumerations, link) {
+   if(strcmp(e->name, a->enumeration_name) == 0)
+   f = e;
+   }
+
+   if (f == NULL)
+   fail(>loc,
+"could not find enumeration %s",
+a->enumeration_name);
+
+   switch (a->type) {
+   case INT:
+   if (f->bitfield)
+   fail(>loc,
+"bitfield-style enum must be 
referenced by uint");
+   break;
+   case UNSIGNED:
+   break;
+   default:
+   fail(>loc,
+"enumeration-style argument has wrong 
type");
+   }
+   }
+   }
+
+}
+
+static

Re: Enums, bitfields and wl_arrays

[Auke Booij]

On 15 October 2015 at 09:18, Erik De Rijcke <derijcke.e...@gmail.com> wrote:
> If wayland enums would not explicitly declare values, you'd have the same
> problem in C. However since wayland enums do explicitly declare their value,
> *and* because C allows you to override an enum 'internal value', you don't
> have that problem here. Unfortunately, Sun in all it's infinite wisdom
> decided that Java enum 'internal values' can not be overridden. Only new
> 'properties' can be attached.
>
>>
>> But if this is indeed the issue you are discussing, then this must
>> have already been a problem before the introduction of additional XML
>> attributes.
>
>
> Correct. My current Java bindings create a Java enum with the wayland enum
> value as an additional property. If the order would change, that would be a
> potentially breaking change.
>
>> Can we agree on the following? Until there is any firm specification
>> of open/closed enums, every enum should be considered open, and
>> although some legal values might be listed, others might not be, and
>> some might only be legal sometimes.
>
>
> Ok for me but it would be very nice to have the open/closed information ;)
>
>>
>> New values may be added (but not
>> changed or removed) to protocol specifications without introducing any
>> compatibility issues.
>
>
> This is not clear for me. Compatibility issues for the current C bindings or
> for all language bindings? If it's for all language bindings than this might
> introduce a whole explosion of derived implicit specifications for each
> language. Eg. the enum order in Java. Better would be to specify a goal "no
> compatibility issues" and a set of form specifications eg. "the order of an
> enum shall not be changed".

This is exactly the kind of specification that I do not intend to
make, since it is (ostensibly) caused solely by a broken
implementation of enums in Java. It's not a problem in C, and not a
problem in any language that understands the concept of a variable.

On 13 October 2015 at 23:15, Bryce Harrington <br...@osg.samsung.com> wrote:
> On Tue, Oct 13, 2015 at 08:27:58PM +0100, Auke Booij wrote:
>> But if this is indeed the issue you are discussing, then this must
>> have already been a problem before the introduction of additional XML
>> attributes.
>>
>> Can we agree on the following? Until there is any firm specification
>> of open/closed enums, every enum should be considered open, and
>> although some legal values might be listed, others might not be, and
>> some might only be legal sometimes. New values may be added (but not
>> changed or removed) to protocol specifications without introducing any
>> compatibility issues.
>
> So, worst case if the binding language is inflexible here, then it'd
> need to maintain a mapping for the values to resequence them?

Yes, that is my point exactly.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: Enums, bitfields and wl_arrays

[Auke Booij]

On 13 October 2015 at 16:19, Solerman Kaplon  wrote:
> Em 13-10-2015 11:35, Nils Chr. Brause escreveu:
>>
>> In C++ the order doesn't matter either, since each entry has a defined
>> value. I wonder why this is different in Java?
>
>
> Java Enums doesn't have "value". It just a class instance. But since it's a
> class one can add regular fields to it, like this (writing of the top of my
> head):
>
> public static enum Orientation {
>   Portrait(1), Landscape(2);
>
>   final int value;
>
>   Orientation(int value) {
> this.value = value;
>   }
>
>   public int getValue() {
> return this.value;
>   }
> }
>
> Solerman


I really don't understand this discussion. Is the claim that the usage
of enums in java is problematic, because inserting a new value in an
existing enum might change the index of later values, thereby creating
an inconsistency?

If so, all I can say that clearly the object you want to associate
with wayland-style enums is not whatever Java has invented for an
"enum". The values that are associated to names inside wayland enums
are very clear-defined, and if a language cannot safely couple names
back to values, then that is a language that does not understand the
concept of a variable. If this is a fundamental problem about Java
enums, you better find a way around it, but I don't see how this is in
any way a problem on the wayland side: there are enums, and enums
contain names, and those names have values.

But if this is indeed the issue you are discussing, then this must
have already been a problem before the introduction of additional XML
attributes.

Can we agree on the following? Until there is any firm specification
of open/closed enums, every enum should be considered open, and
although some legal values might be listed, others might not be, and
some might only be legal sometimes. New values may be added (but not
changed or removed) to protocol specifications without introducing any
compatibility issues.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland v2] Remove protocol/wayland.dtd

[Auke Booij]

Yeah, that was a pretty embarrassing mistake by me, for such a simple
patch. Thanks to Bryce for catching it.

On 8 October 2015 at 15:05, Pekka Paalanen <ppaala...@gmail.com> wrote:
> On Thu,  8 Oct 2015 14:35:34 +0100
> Auke Booij <a...@tulcod.com> wrote:
>
>> The wayland scanner defines the protocol. The DTD specification is not used.
>> ---
>>  Makefile.am  |  4 ++--
>>  protocol/wayland.dtd | 29 -
>>  2 files changed, 2 insertions(+), 31 deletions(-)
>>  delete mode 100644 protocol/wayland.dtd
>
> Reviewed-by: Pekka Paalanen <pekka.paala...@collabora.co.uk>
>
> No, I didn't run distcheck this time either. ;-)
>
>
> Thanks,
> pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland 0/3] "enum" and "bitfield" attributes for protocol XML files

[Auke Booij]

On 5 October 2015 at 14:56, Pekka Paalanen <ppaala...@gmail.com> wrote:
> On Fri, 26 Jun 2015 16:02:44 +0200
> Auke Booij <a...@tulcod.com> wrote:
>
>> As per last April/May's "enum" attribute discussion, these patches
>> introduce two new attributes to the protocol XML files.
>> The "enum" attribute is given to uments of type (u)int, and
>> indicates which  should be used for that argument.
>> The "bitfield" attribute can be given to s, and, if set to "true",
>> indicates that the enum is to be thought of as a bitmask-style value.
>>
>> Although arguments can only refer to enums in specific cases (see the
>> scanner.c changes), this new protocol data should not break the C bindings.
>> It is thinkable that other bindings *do* use the data in a way that breaks
>> the protocol; however such usage will be considered nonstandard.
>>
>> Auke Booij (3):
>>   protocol: introduce the enum argument type
>>   scanner: enforce correct argument type for enums
>>   doc: document new enum attributes and use such data in generated docs
>
> Hi Auke,
>
> so we've had a huge amount of discussion recently and I'm sure you have
> lots of ideas already. I'm not going to summarize that discussion, but
> propose more technical improvements here.
>
> I think it would make sense to order the patch series in reverse:
> 1. XML language spec (and the DTD if you want to update that)
> 2. wayland.xml
> 3. scanner
> 4. doc generator
>
> That way we start with the XML language specification. Then add the new
> things to wayland.xml. At this point we see that old wayland-scanner
> still works. Then update wayland-scanner, and finally the doc
> generator. So the doc patch would be split in two.
>
> Reviewers will then verify the (absence of) changes in the generated C
> bindings.
>
> I think I discussed with Victor in IRC about testing old
> wayland-scanners. It shouldn't be too difficult to write a script that
> checks out each of the ten 1.x.0 Wayland releases, builds
> wayland-scanner from it, and runs it on the tip wayland.xml. Maybe
> someone could contribute such a script. I think it might be handy in
> the future too.
>
> In Patch 1 for the XML language spec, I think you could start a new
> section "XML Language" or such, and specify all these new concepts and
> attributes there. You can start it with a blanket paragraph that says
> the authoritative basic definition of the language is what
> wayland-scanner implements, and this section just adds to that. This
> way we will have a place to start gathering the language doc.
>
> I think the section "code generation" is more like how to do language
> bindings, so if someone wants to get started on that, that would be the
> place. What is right now in "code generation" could be split between
> "basic principles" (we have this XML files thing) and C bindings.
>
> If you want to avoid revising all the patches, you could start by
> posting an RFC of only the first patch. That's what everyone has been
> fighting over anyway. When people are generally happy with that, you
> could do the full series.
>
> How's that?
>
>
> Thanks,
> pq


I will work on this. Thanks for the constructive comments!
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH wayland v2] Remove protocol/wayland.dtd

[Auke Booij]

The wayland scanner defines the protocol. The DTD specification is not used.
---
 Makefile.am  |  4 ++--
 protocol/wayland.dtd | 29 -
 2 files changed, 2 insertions(+), 31 deletions(-)
 delete mode 100644 protocol/wayland.dtd

diff --git a/Makefile.am b/Makefile.am
index 58f5595..ffad16b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -16,8 +16,8 @@ dist_aclocal_DATA = wayland-scanner.m4
 
 dist_pkgdata_DATA =\
wayland-scanner.mk  \
-   protocol/wayland.xml\
-   protocol/wayland.dtd
+   protocol/wayland.xml
+
 
 pkgconfigdir = $(libdir)/pkgconfig
 pkgconfig_DATA =
diff --git a/protocol/wayland.dtd b/protocol/wayland.dtd
deleted file mode 100644
index b8b1573..000
--- a/protocol/wayland.dtd
+++ /dev/null
@@ -1,29 +0,0 @@
-
-  
-
-
-  
-  
-
-  
-  
-  
-
-  
-  
-
-  
-  
-
-  
-  
-  
-  
-
-  
-  
-  
-  
-  
-
-  
-- 
2.5.3

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH] Remove protocol/wayland.dtd

[Auke Booij]

The wayland scanner defines the protocol. The DTD specification is not used.
---
 protocol/wayland.dtd | 29 -
 1 file changed, 29 deletions(-)
 delete mode 100644 protocol/wayland.dtd

diff --git a/protocol/wayland.dtd b/protocol/wayland.dtd
deleted file mode 100644
index b8b1573..000
--- a/protocol/wayland.dtd
+++ /dev/null
@@ -1,29 +0,0 @@
-
-  
-
-
-  
-  
-
-  
-  
-  
-
-  
-  
-
-  
-  
-
-  
-  
-  
-  
-
-  
-  
-  
-  
-  
-
-  
-- 
2.5.3

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: Enums, bitfields and wl_arrays

[Auke Booij]

This email contains a clear suggestions that I'd like everyone to
read, not just Nils and Erik. But Nils and Erik brought up a few
points that I'd like to have settled, since they have been brought up
quite frequently now.

On 3 October 2015 at 11:14, Nils Chr. Brause <nilschrbra...@gmail.com> wrote:
> Pekka Paalanen wrote:
>> Later that thread also brought up two incompatible definitions of a
>> "bitfield":
>> - a type where bit-wise logic operations make sense
>> - a type where all listed values are powers of two (POT), and bit-wise
>>   logic operations make sense
>>
>> As a practical example, is wl_output::transform a bitfield or an enum?
>> - it is used as an int, not uint
>> - it lists all possible values
>> - the listed values are not all POT
>> - bit-wise operations make sense (check for flipped)
>>
>> Arguably it should have been an uint, but I don't think we can change
>> it now.
>
> I have a rather pragmatic view at this: If it looks and feels like a
> bitfield, it is a bitfield.
> And when looking an the definition, it for all the wold looks like a
> bitfield to me.
> Bit 0 means 90°, bit 1 means 180° and bit 3 means flipped.
> The signedness doesn't matter for a bitfield, since it is just a
> collection of bits without a numerical meaning.

So perhaps we should say:

"An enum can be specified to be a bitfield if the primary interface to
its numeric value deals with bitwise operations."

Is that agreeable?

> Pekka Paalanen wrote:
>> yeah, adding the interface name makes perfect sense. It could be
>> optional if wanted. No dot would mean the enum is in the same
>> interface, a dot would signify a specific interface. I don't think
>> there is any use for an anonymous global namespace like Bill mentioned.
>
> Sounds good to me. Although I think it would be easier to implement,
> if we'd just use the dot notation everywhere. This would result in
> less code and (statistically) less bugs.

It is the "dotless" version that is already supported by my patches.
The dotted one requires new code. So apart from a single if statement,
this is not really more work. But I guess this is getting into
bikeshedding terrain.

> Auke Booij wrote:
>> The enum and bitfield attributes are in principle for documentation
>> purposes only.
>
> No. The whole point of the enum an bitfield attributes is to help
> non-C languages to generate type-safe bindings.
>
> Auke Booij wrote:
>> The enum and bitfield attributes may also be used by
>> bindings, but only in such a way that code written prior to the
>> specification of these attributes still works after their
>> specification.
>
> I think every language binding should ultimately decide for
> themselves, how they are using the new attributes. There is no way you
> can dictate anyone how to use them.

I think that we have an agreement in principle, just not in terms (and
this also goes back to my comment to Victor Berger). What I meant to
express is that any bindings that violate this rule are on the risk of
the bindings writers, rather than the wayland and wayland protocol
developers.

Would you agree with the following? Otherwise please suggest a
phrasing you would agree with.

[start]
The purpose of the enum and bitfield attributes is to document what
arguments refer to which enums, and to document which numeric enum
values are primarily accessed using bitwise operations.
Additionally, the enum and bitfield attributes may be used by other
code, such as bindings to other languages, for example to enhance type
safety of code. However, such usage is only supported if the following
property is satisfied: code written prior to the specification of
these attributes still works after their specification. In other
words, specifying an attribute for an argument, that previously did
not have an enum or bitfield attribute, should not break API. Code
that does not satisfy this rule is not guaranteed to obey backwards
compatibility.
[end]

> Auke Booij wrote:
>> I agree with your concern. However, note that "for documentation
>> purposes" does not mean that it is just a documentation string: we
>> document a very clear and precise fact, for the purpose of
>> understanding the API (ie documentation). My previous patches indeed
>> do some checking wrt enum name cross-matching. This would have to be
>> extended for enum names that refer to a different interface.
>
> You shouldn't write "for documentation purposes only" then, as it is
> clearly not. It is mainly for the type-safety of non-C
> language-bindings.

Please do comment on the above, as I understand your point, but also
want to be pragmatic here.

On 3 October 2015 at 20:05, Erik De Rijcke <derijcke.e...

Re: Enums, bitfields and wl_arrays

[Auke Booij]

On 2 October 2015 at 13:12, Auke Booij <a...@tulcod.com> wrote:
> The wayland protocol currently does not specify the enum attribute,
> and I see no way how to write an API whose entire purpose is to
> *break* when you erroneously mix up enum attribute data, without
> breaking API as this data is added.

Actually, for my particular scenario, I just thought of a very elegant
solution that would introduce no new compatibility issues. With this
solution, *more* code would compile *with* the right enum attributes
than *without*. So we would have the best of both worlds: the type
safety that modern languages want, without new compatibility issues to
keep track of. (If anyone is interested in the technicalities behind
this, contact me. But it is a solution that is rather specific to
Haskell. Essentially, it would work by, rather than passing enum
values as ints until we have a proper data type for them, we would
pass each field as a different data type, until we know that two enum
fields should have been the same type. In haskell, this can be done in
a backwards-compatible way.)

Maybe a solution can be found in more languages, and we can make the
compatibility, that e.g. Pekka is looking for, a requirement for
bindings, rather than make compatibility a requirement for the
protocol writers. So something along these lines would be in the
specification:

[start]
The enum and bitfield attributes are in principle for documentation
purposes only. The enum and bitfield attributes may also be used by
bindings, but only in such a way that code written prior to the
specification of these attributes still works after their
specification. In other words, specifying an attribute for an
argument, that previously did not have it, should not break API.
[end]

Obviously C is not rich enough to do this in an elegant way. (Maybe
it's possible in C++ with some template magic?) But it definitely
solves the "backwards compatibility" debate, since, while anyone is
free to ignore this bit in the specification, solutions might
sometimes be possible, and it guards the entire C world from issues in
the non-C bindings.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: Enums, bitfields and wl_arrays

[Auke Booij]

On 1 October 2015 at 20:00, Nils Chr. Brause  wrote:
>> Since Auke's patchset proposalis the most recent, let's take that one as
>> the candidate for landing.  Gentlemen, I'd like to ask you to review
>> these three patches [5,6,7] and either give your Reviewed-by's or flag
>> specific improvements needed.  If you have a more conceptual
>> disagreement, and don't think the patchset is landable as implemented,
>> please raise that issue asap too.
>
> There are some enum attributes missing, namely:
> - wl_shm_pool::create_buffer::format (it's wl_shm::format)
> - wl_shell_surface::set_fullscreen::method (it's
> wl_shell_surface::fullscreen_method)
> - wl_surface::set_buffer_transform::transform (it's wl_output::transform)

wl_shell_surface::set_fullscreen::method is a mistake by me. You are
right, that should have been in there. The reason I left out the other
two is because of what you write here:

> I would prefer, if the enum attributes would also name the interface,
> where the enum can be found, e.g.:
> 
> If two enums in different interfaces happen to have the same name (if
> that's possible?), this would lead to ambiguities otherwise. Also a
> scanner wouldn't have to look up the interface name that way.

While in principle I think this is a great idea, this will need a few
specifications, which is why I decided not to add those in just yet.
Are cross-XML references allowed in this sense? In that case, the
scanner cannot verify their correctness, since only the current XML
file is available to it. Additionally, moving a certain interface from
xdg_shell to the core wayland protocol would now mean potentially
having to weaken the type safety of an interface, or having to copy
the enum over.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: Enums, bitfields and wl_arrays

[Auke Booij]

Thanks for bringing this up again, Bryce.

On 1 October 2015 at 18:59, Jasper St. Pierre  wrote:
> We have a few constraints. First off, not all enums are closed. Some
> are intentionally open, like xdg_shell.state. So we definitely need a
> distinction between a closed enum and an open enum. I'm not familiar
> enough with Rust to be able to apply something to that.

Yes, this is a point that was raised before (IIRC especially by
Pekka). This is why, in my patches, I document these attributes as
being for documentation purposes only, although obviously they might
be abused by some bindings. Yes, a closed enum could be encoded into
more strongly typed languages (in which case a compilable program only
specifies what it does on the "legal" values, and simply crashes or
errors out on the unknown ones - this would be justified by the broken
promise that the enum was closed). But I agree that this is a
guarantee you cannot always make, and will open up a debate far bigger
than the actual problems these patches set out to solve.

> There's also a compatibility issue that has been brought up, but I
> never understood that one. Somebody else would be able to talk about
> that better.

Can you quote what you are referring to here?
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: Enums, bitfields and wl_arrays

[Auke Booij]

On 2 October 2015 at 12:31, Pekka Paalanen  wrote:
>> I know that several people have proposed patches on this - Bill, Nils
>> and Auke at least.  Since there's a definite need for this, and since
>> agreement appears to be not far off, I would like to get this landed
>> this release.  And ideally I'd like to get this landed early in the
>> release so we give plenty of time for testing.
>
> The things that have been lacking in my opinion are specifications,
> especially answering the questions listed here by Victor after our irc
> discussion:
> http://lists.freedesktop.org/archives/wayland-devel/2015-September/024382.html
>
> The "do not break existing code" rule applies also proactively, not
> just at the time we decide to start using the new attributes, so it is
> important to define what things are under the "stable protocol"
> definition and cannot change after publishing. IOW, what are we carving
> in stone.
>
> There is a paradox: if the new attributes affect codegen for some
> languages in a way that will break the build (which is exactly the
> reason why they are wanted: to enforce type-safety), we cannot add
> these attributes to the existing protocols, most importantly
> wayland.xml, because it would "break existing code". You might escape
> that paradox once, by claiming that there is no "existing code", after
> all the generators for these languages will be new. But this escape
> hatch will only work once. When these generators get into use, we can
> no longer add the attributes to stable protocols without breaking
> things. Not even if the generators had a "legacy mode" that ignores the
> new attributes, because they would then regress on arguments that
> already had the new attributes set.
>
> The important thing to realize is that once an XML file starts using
> these new attributes, the attributes can no longer be added later into
> more arguments. We need to get it right on the first go, but hey,
> that's how extending already stabilized protocols goes.
>
> A further complication here is that currently, the definition of the
> XML language is literally "what wayland-scanner accepts", bugs and
> oversights in wayland-scanner notwithstanding. This makes it tricky to
> add XML language features that are not really used in the C bindings.
>
> Or, would it be ok to dismiss all non-C languages as second-class
> citizens, and say that we can change whatever as long as it does not
> break the wire format, wayland-scanner or the bindings generated by it?
> Somehow I don't think that would be an acceptable position.
>
> This is actually a fundamental question:
>
> Should we make any stability guarantees towards non-C bindings
> generators that exceed what we need for C?
>
> If no, then there's nothing to solve, but like I said, that would be
> pretty harsh and inconsiderate.
>
> Since most XML file authors are likely going to test only with
> wayland-scanner, wayland-scanner should detect as many problems as
> possible. This is why I have asked for generator patches to at least use
> the new attributes somehow. Otherwise we would be breaking the non-C
> generators all the time.

I agree (if that's what you're saying) that this point about
compatibility is the main debate towards the new attributes.

However, I'm not sure who you are trying to protect here. Everyone
agrees that the new attributes should not change anything for C/C++,
and in the current patches, they don't. And the other bindings writers
understand the compatibility issues regarding this change, and, if I
may speak for the majority of them, care much less about compatibility
issues than about being able to provide proper APIs for their users.

In haskell, and many other modern languages, an easily fixable compile
issue (we keep calling it a compatibility issue, which I think is a
misnomer, although I don't know what category it should fall under) is
*vastly* preferred over a potential bug. Arguably, that's the entire
point of the language. Modern languages attempt to cross-check as many
properties as possible, and the enum attribute would contribute
greatly to this, even if that means breaking API every so often.

The wayland protocol currently does not specify the enum attribute,
and I see no way how to write an API whose entire purpose is to
*break* when you erroneously mix up enum attribute data, without
breaking API as this data is added. More importantly, this problem
does not matter because it is not a problem but a *solution*.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: Enums, bitfields and wl_arrays

[Auke Booij]

On 2 October 2015 at 14:49, Victor Berger <victor.ber...@m4x.org> wrote:
> Le 2015-10-02 15:16, Pekka Paalanen a écrit :
>>
>> On Fri, 2 Oct 2015 13:50:42 +0100
>> Auke Booij <a...@tulcod.com> wrote:
>>>
>>>
>>> [start]
>>> The enum and bitfield attributes are in principle for documentation
>>> purposes only. The enum and bitfield attributes may also be used by
>>> bindings, but only in such a way that code written prior to the
>>> specification of these attributes still works after their
>>> specification. In other words, specifying an attribute for an
>>> argument, that previously did not have it, should not break API.
>>> [end]
>>
>>
>> I like this very much. Let's see if anyone disagrees.
>>
>> Do you intend to allow also changing rather than only adding these new
>> attributes in the wording above?
>
>
> While I don't disagree, I have a small concern:
>
> Do we agree that this involves at some point writing a specification of the
> format of the XML files?
>
> Because otherwise, if the XML format remains defined by the implementation
> of the C scanner, and that these attributes are explicitly defined as for
> documentation only and ignored by the C scanner, this means the XML writers
> would be allowed to write any garbage they want as a value for these fields.
>
> As a consequence, bindings writers cannot give any value to these fields,
> and they become basically useless. I mean, it seems logical that the value
> of the "enum" field should be something like "enum_name" or
> "interface_name.enum_name", or whatever format will be chosen. But these
> fields have practically no value if we cannot expect this format to be
> respected (documentation-only fields can also simply be written in the
> "description" field, if they are not used anywhere).
>
> --
> Victor
>

I agree with your concern. However, note that "for documentation
purposes" does not mean that it is just a documentation string: we
document a very clear and precise fact, for the purpose of
understanding the API (ie documentation). My previous patches indeed
do some checking wrt enum name cross-matching. This would have to be
extended for enum names that refer to a different interface.

Pekka's proposal to include a "strict" mode will help here, for
cross-XML-interface-checking. Even though this would be optional, it
means we'll have to let go of the idea that protocol files are mostly
independent (if there ever was any such belief).
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH wayland 3/3] doc: document new enum attributes and use such data in generated docs

[Auke Booij]

The newly introduced enum and bitfield protocol XML attributes give
additional semantic information, which we can use when generating the
documentation.
---
 doc/publican/protocol-to-docbook.xsl |  9 +
 doc/publican/sources/Protocol.xml| 23 +--
 2 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/doc/publican/protocol-to-docbook.xsl 
b/doc/publican/protocol-to-docbook.xsl
index 7b45969..fad207a 100644
--- a/doc/publican/protocol-to-docbook.xsl
+++ b/doc/publican/protocol-to-docbook.xsl
@@ -102,6 +102,12 @@
 termxsl:value-of select=@name//term
 listitem
 simpara
+  xsl:if test=@enum
+link linkend=protocol-spec-{../../@name}-enum-{@enum}
+  xsl:value-of select=@enum/
+/link
+   xsl:text /xsl:text
+  /xsl:if
   xsl:value-of select=@type/
   xsl:if test=@summary 
 - xsl:value-of select=@summary/
@@ -171,6 +177,9 @@
   section id=protocol-spec-{../@name}-{name()}-{@name}
 title
   xsl:value-of select=../@name/::xsl:value-of select=@name /
+  xsl:if test=@bitfield
+- bitfield
+  /xsl:if
   xsl:if test=description/@summary
 - xsl:value-of select=description/@summary /
   /xsl:if
diff --git a/doc/publican/sources/Protocol.xml 
b/doc/publican/sources/Protocol.xml
index 477063b..facdcdc 100644
--- a/doc/publican/sources/Protocol.xml
+++ b/doc/publican/sources/Protocol.xml
@@ -15,6 +15,20 @@
   identifies which method in the interface to invoke.
 /para
 para
+  The protocol is message-based.  A message sent by a client to the server
+  is called request.  A message from the server to a client is called 
event.
+  A message has a number of arguments, each of which has a certain type 
(see
+  xref linkend=sect-Protocol-Wire-Format/ for a list of argument 
types).
+  Additionally, the protocol can specify typeenum/types.  These are 
used
+  to list options for typeint/type and typeuint/type type 
arguments.
+  For documentation purposes, arguments can refer to the specific
+  enumeration that is semantically implied.  Only in the case that the
+  argument is of type typeuint/type, it can specify that the values 
that
+  are passed around are to be thought of as emphasisbitfields/emphasis,
+  for example when arbitrarily many choices of the enum can be ORed
+  together.
+/para
+para
   The server sends back events to the client, each event is emitted from
   an object.  Events can be error conditions.  The event includes the
   object ID and the event opcode, from which the client can determine
@@ -62,14 +76,11 @@
   The protocol is sent over a UNIX domain stream socket, where the endpoint
   usually is named systemitem class=servicewayland-0/systemitem
   (although it can be changed via emphasisWAYLAND_DISPLAY/emphasis
-  in the environment).  The protocol is message-based.  A
-  message sent by a client to the server is called request.  A message
-  from the server to a client is called event.  Every message is
-  structured as 32-bit words, values are represented in the host's
-  byte-order.
+  in the environment).
 /para
 para
-  The message header has 2 words in it:
+  Every message is structured as 32-bit words, values are represented in 
the
+  host's byte-order.  The message header has 2 words in it:
   itemizedlist
listitem
  para
--
2.4.4
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH wayland 2/3] scanner: enforce correct argument type for enums

[Auke Booij]

The scanner now checks whether arguments that have an associated
enum have the right type.
An argument with an enum attribute must be of type int or uint,
and if the enum with that name has the bitfield attribute
set to true, then the argument must be of type uint.
---
 src/scanner.c | 68 ++-
 1 file changed, 67 insertions(+), 1 deletion(-)

diff --git a/src/scanner.c b/src/scanner.c
index 7d8cfb9..75dda55 100644
--- a/src/scanner.c
+++ b/src/scanner.c
@@ -126,6 +126,7 @@ struct arg {
char *interface_name;
struct wl_list link;
char *summary;
+   char *enumeration_name;
 };

 struct enumeration {
@@ -134,6 +135,7 @@ struct enumeration {
struct wl_list entry_list;
struct wl_list link;
struct description *description;
+   int bitfield;
 };

 struct entry {
@@ -326,7 +328,7 @@ start_element(void *data, const char *element_name, const 
char **atts)
struct entry *entry;
struct description *description;
const char *name, *type, *interface_name, *value, *summary, *since;
-   const char *allow_null;
+   const char *allow_null, *enumeration_name, *bitfield;
char *end;
int i, version;

@@ -340,6 +342,8 @@ start_element(void *data, const char *element_name, const 
char **atts)
description = NULL;
since = NULL;
allow_null = NULL;
+   enumeration_name = NULL;
+   bitfield = NULL;
for (i = 0; atts[i]; i += 2) {
if (strcmp(atts[i], name) == 0)
name = atts[i + 1];
@@ -357,6 +361,10 @@ start_element(void *data, const char *element_name, const 
char **atts)
since = atts[i + 1];
if (strcmp(atts[i], allow-null) == 0)
allow_null = atts[i + 1];
+   if (strcmp(atts[i], enum) == 0)
+   enumeration_name = atts[i + 1];
+   if (strcmp(atts[i], bitfield) == 0)
+   bitfield = atts[i + 1];
}

ctx-character_data_length = 0;
@@ -488,6 +496,11 @@ start_element(void *data, const char *element_name, const 
char **atts)
else
fail(ctx-loc, invalid value for allow-null attribute 
(%s), allow_null);

+   if (enumeration_name == NULL || strcmp(enumeration_name, ) == 
0)
+   arg-enumeration_name = NULL;
+   else
+   arg-enumeration_name = xstrdup(enumeration_name);
+
if (allow_null != NULL  !is_nullable_type(arg))
fail(ctx-loc, allow-null is only valid for objects, 
strings, and arrays);

@@ -507,6 +520,13 @@ start_element(void *data, const char *element_name, const 
char **atts)
enumeration-description = NULL;
wl_list_init(enumeration-entry_list);

+   if (bitfield == NULL || strcmp(bitfield, false) == 0)
+   enumeration-bitfield = 0;
+   else if (strcmp(bitfield, true) == 0)
+   enumeration-bitfield =1;
+   else
+   fail(ctx-loc, invalid value for bitfield attribute 
(%s), bitfield);
+
wl_list_insert(ctx-interface-enumeration_list.prev,
   enumeration-link);

@@ -545,6 +565,46 @@ start_element(void *data, const char *element_name, const 
char **atts)
 }

 static void
+verify_arguments(struct parse_context *ctx, struct wl_list *messages, struct 
wl_list *enumerations)
+{
+   struct message *m;
+   wl_list_for_each(m, messages, link) {
+   struct arg *a;
+   wl_list_for_each(a, m-arg_list, link) {
+   struct enumeration *e, *f;
+
+   if (!a-enumeration_name)
+   continue;
+
+   f = NULL;
+   wl_list_for_each(e, enumerations, link) {
+   if(strcmp(e-name, a-enumeration_name) == 0)
+   f = e;
+   }
+
+   if (f == NULL)
+   fail(ctx-loc,
+could not find enumeration %s,
+a-enumeration_name);
+
+   switch (a-type) {
+   case INT:
+   if (f-bitfield)
+   fail(ctx-loc,
+bitfield-style enum must be 
referenced by uint);
+   break;
+   case UNSIGNED:
+   break;
+   default:
+   fail(ctx-loc,
+enumeration-style argument has wrong 
type);
+   }
+   }
+   }
+
+}
+
+static void
 end_element(void 

[PATCH wayland 0/3] enum and bitfield attributes for protocol XML files

[Auke Booij]

As per last April/May's enum attribute discussion, these patches
introduce two new attributes to the protocol XML files.
The enum attribute is given to arguments of type (u)int, and
indicates which enum should be used for that argument.
The bitfield attribute can be given to enums, and, if set to true,
indicates that the enum is to be thought of as a bitmask-style value.

Although arguments can only refer to enums in specific cases (see the
scanner.c changes), this new protocol data should not break the C bindings.
It is thinkable that other bindings *do* use the data in a way that breaks
the protocol; however such usage will be considered nonstandard.

Auke Booij (3):
  protocol: introduce the enum argument type
  scanner: enforce correct argument type for enums
  doc: document new enum attributes and use such data in generated docs

 doc/publican/protocol-to-docbook.xsl |  9 +
 doc/publican/sources/Protocol.xml| 23 
 protocol/wayland.dtd |  2 ++
 protocol/wayland.xml | 32 -
 src/scanner.c| 68 +++-
 5 files changed, 111 insertions(+), 23 deletions(-)

--
2.4.4
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland 0/3] enum and bitfield attributes for protocol XML files

[Auke Booij]

On 26 June 2015 at 16:02, Auke Booij a...@tulcod.com wrote:
 Although arguments can only refer to enums in specific cases (see the
 scanner.c changes), this new protocol data should not break the C bindings.
 It is thinkable that other bindings *do* use the data in a way that breaks
 the protocol; however such usage will be considered nonstandard.

In retrospect, I phrased this a bit poorly. Obviously this new code
should not break any existing bindings: it merely introduces two new
datums that no one is using so far. Instead, the three points I am
making here are:

- The scanner.c code checks whether arguments that have an enum
attribute are of type (u)int, and if they refer to an enum with
bitfield=true, then it checks they are of type uint (not int!).
- In the discussion a few months ago, it emerged that whenever a
protocol starts supplying such data, when it did not specify it
before, this should not change the bindings API (being more specific
shouldn't break anything). Indeed this is not the case for the C
scanner, since it still doesn't use the data - these patches only
block builds when the new attributes aren't consistent in the sense
made by the previous point.
- Other bindings, in theory, can use the data given by these new
attributes, for example to strengthen the type safety of the generated
API. However, this might introduce API breakages when the protocol is
updated in a non-breaking way. While this is definitely not endorsed
by this set of changes, these changes do provide a way to convey more
type safety in strongly typed languages. My own (ab)use-case here is
the Haskell bindings I wrote.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH wayland 1/3] protocol: introduce the enum argument type

[Auke Booij]

This improvement to the protocol allows you to refer to the kind of enum you 
are expecting.
It also introduces a distinction between enums that are bitfields, ie
that can be OR'ed together.
---
 protocol/wayland.dtd |  2 ++
 protocol/wayland.xml | 32 
 2 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/protocol/wayland.dtd b/protocol/wayland.dtd
index b8b1573..3b67ca8 100644
--- a/protocol/wayland.dtd
+++ b/protocol/wayland.dtd
@@ -14,6 +14,7 @@
 !ELEMENT enum (description?,entry*)
   !ATTLIST enum name CDATA #REQUIRED
   !ATTLIST enum since CDATA #IMPLIED
+  !ATTLIST enum bitfield CDATA #IMPLIED
 !ELEMENT entry (description?)
   !ATTLIST entry name CDATA #REQUIRED
   !ATTLIST entry value CDATA #REQUIRED
@@ -25,5 +26,6 @@
   !ATTLIST arg summary CDATA #IMPLIED
   !ATTLIST arg interface CDATA #IMPLIED
   !ATTLIST arg allow-null CDATA #IMPLIED
+  !ATTLIST arg enum CDATA #IMPLIED
 !ELEMENT description (#PCDATA)
   !ATTLIST description summary CDATA #REQUIRED
diff --git a/protocol/wayland.xml b/protocol/wayland.xml
index 42c9309..a1d594f 100644
--- a/protocol/wayland.xml
+++ b/protocol/wayland.xml
@@ -367,7 +367,7 @@
can be used for buffers. Known formats include
argb and xrgb.
   /description
-  arg name=format type=uint/
+  arg name=format type=uint enum=format/
 /event
   /interface

@@ -774,7 +774,7 @@
   /description
   arg name=seat type=object interface=wl_seat summary=the wl_seat 
whose pointer is used/
   arg name=serial type=uint summary=serial of the implicit grab on 
the pointer/
-  arg name=edges type=uint summary=which edge or corner is being 
dragged/
+  arg name=edges type=uint enum=resize summary=which edge or 
corner is being dragged/
 /request

 request name=set_toplevel
@@ -785,7 +785,7 @@
   /description
 /request

-enum name=transient
+enum name=transient bitfield=true
   description summary=details of transient behaviour
These flags specify details of the expected behaviour
of transient surfaces. Used in the set_transient request.
@@ -807,7 +807,7 @@
   arg name=parent type=object interface=wl_surface/
   arg name=x type=int/
   arg name=y type=int/
-  arg name=flags type=uint/
+  arg name=flags type=uint enum=transient/
 /request

 enum name=fullscreen_method
@@ -891,7 +891,7 @@
   arg name=parent type=object interface=wl_surface/
   arg name=x type=int/
   arg name=y type=int/
-  arg name=flags type=uint/
+  arg name=flags type=uint enum=transient/
 /request

 request name=set_maximized
@@ -972,7 +972,7 @@
in surface local coordinates.
   /description

-  arg name=edges type=uint/
+  arg name=edges type=uint enum=resize/
   arg name=width type=int/
   arg name=height type=int/
 /event
@@ -1337,7 +1337,7 @@
   maintains a keyboard focus and a pointer focus.
 /description

-enum name=capability
+enum name=capability bitfield=true
   description summary=seat capability bitmask
 This is a bitmask of capabilities this seat has; if a member is
 set, then it is present on the seat.
@@ -1353,7 +1353,7 @@
keyboard or touch capabilities.  The argument is a capability
enum containing the complete set of capabilities this seat has.
   /description
-  arg name=capabilities type=uint/
+  arg name=capabilities type=uint enum=capability/
 /event

 request name=get_pointer
@@ -1521,7 +1521,7 @@
   arg name=serial type=uint/
   arg name=time type=uint summary=timestamp with millisecond 
granularity/
   arg name=button type=uint/
-  arg name=state type=uint/
+  arg name=state type=uint enum=button_state/
 /event

 enum name=axis
@@ -1553,7 +1553,7 @@
   /description

   arg name=time type=uint summary=timestamp with millisecond 
granularity/
-  arg name=axis type=uint/
+  arg name=axis type=uint enum=axis/
   arg name=value type=fixed/
 /event

@@ -1593,7 +1593,7 @@
This event provides a file descriptor to the client which can be
memory-mapped to provide a keyboard mapping description.
   /description
-  arg name=format type=uint/
+  arg name=format type=uint enum=keymap_format/
   arg name=fd type=fd/
   arg name=size type=uint/
 /event
@@ -1638,7 +1638,7 @@
   arg name=serial type=uint/
   arg name=time type=uint summary=timestamp with millisecond 
granularity/
   arg name=key type=uint/
-  arg name=state type=uint/
+  arg name=state type=uint enum=key_state/
 /event

 event name=modifiers
@@ -1819,17 +1819,17 @@
   summary=width in millimeters of the output/
   arg name=physical_height type=int
   summary=height in millimeters of the output/
-  arg name=subpixel type=int
+  arg name=subpixel type=int enum=subpixel
   summary=subpixel 

Re: [PATCH wayland 0/3] enum and bitfield attributes for protocol XML files

[Auke Booij]

On 26 June 2015 at 19:25, Bill Spitzak spit...@gmail.com wrote:
 I am very much in favor of this, and posted earlier a further patch that
 uses this information to produce better protocol documentation.

 The above design is exactly correct (in particular the bitfield indicator is
 on the enumeration, not the argument).

Thanks!

Somehow I've completely missed those patches you sent (but I found
them now) - apologies for that. Your XSLT code makes slightly more
sense than mine, and I would be fine with either solution. However, my
patch in particular also exposes the bitfield information to the
documentation, and explains the attributes in chapter 4 (Wayland
Protocol and Model of Operation).

I forgot to add my Signed-off-by to these three patches, but it
applies, so here you go:

Signed-off-by: Auke Booij a...@tulcod.com
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland] Add enum attribute to arg elements

[Auke Booij]

On 19 April 2015 at 14:51, Jeroen Bollen jbin...@gmail.com wrote:
 Hello,

 It seems like this discussion died off. Currently there is no way to tell,
 from the Wayland XML specification whether an argument is a bitfield, or
 whether the argument takes an enum and what enum this is.

 I am currently in the progress of writing a Wayland binding generator for
 the Rust language. This language, like many others is strongly typed. To
 make full usage of this type system, it would be beneficial to know from the
 specification whether an argument is a bitfield, and what enum type it
 takes.

 Surely there are more people who generate bindings to these strongly typed
 languages. How have you fixed the issue? Are there patched versions
 available, and maybe pending to be merged? I have looked around a bit, and
 didn't find anything, but then again, I'm not familiar with Wayland
 development. (This is the first time I use a mailing list!)

 Much appreciated,
 Jeroen Bollen

 ___
 wayland-devel mailing list
 wayland-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Based on the discussion I wrote two patches: one for the wayland.xml
(and wayland.dtd since the fact of the matter is that that file is
still there), and one for scanner.c

See 5dc0112f734d23d65a2bf9a78f11463d0294b6c7 and
b6d5659c650056f66267f3f20fdbaf724138a61b in [0].

Is this a good format for sending patches? Or are actual patch emails preferred?

The scanner.c code checks that the enum attribute of an arg is the
name of an enum in the same interface. In that case, it checks
that the type of the arg is an int or a uint, and if the enum had
bitfield set to true, that the type was indeed uint.

If these properties are not satisfied, the scanner dies. Is that what
was expected?

Auke.

[0] https://github.com/tulcod/wayland/tree/enum-attribute
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland] Add enum attribute to arg elements

[Auke Booij]

On 21 Apr 2015 09:18, Pekka Paalanen ppaala...@gmail.com wrote:
 Two things I came up with in the IRC discussion was that only arg
 types of int an uint are eligible for enums, and only uint for
 bitfields. I think wayland-scanner should enforce that.

I just realised another aspect of this. Can a (non-bitfield) enum be used
both as an int and a uint? So some *int* argument somewhere has enum=bla,
and a *uint* argument somewhere else has enum=bla as well. What about
enums that list negative values, can they be used as uints?

I feel that this would be extremely prone to bugs, and in fact I think this
is a situation where semantic information of enum style arguments can help
catch bugs in C code (which so far we haven't seen yet in this debate).

Should this be enforced by the scanner? Or is there some crazy situation in
which it makes sense?
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland] Add enum attribute to arg elements

[Auke Booij]

On 27 April 2015 at 14:49, Pekka Paalanen ppaala...@gmail.com wrote:
 On Mon, 27 Apr 2015 13:26:39 +0200
 Auke Booij a...@tulcod.com wrote:

 Apologies for my lack of responses, I have been abroad for a few days.

 On 23 April 2015 at 10:38, Pekka Paalanen ppaala...@gmail.com wrote:
  This is a sort of sanity condition on being a bitfield: it does not
  require all combinations are valid, but it also distinguishes it from
  a regular enum.
 
  Is that an important distinction to make? That a bitfield with too
  many restrictions must not be a bitfield?

 Yes, because what the bitfield flag indicates is that it is useful to
 think of the values as flags. It means that the values can somehow be
 combined. If there is no way to validly combine them, then it is not
 useful to think of them like flags. And if the right way to combine
 them is not OR'ing, then what you are talking about is not a bitfield.
 I am not saying it is a kind of value that is unwelcome in the wayland
 API, merely that it is not in the same category as what is usually
 considered a bitfield.

 And indeed some things (e.g. wl_shell_surface.resize and
 xdg_surface.resize_edge) kinda look like a bitfield, but it is up for
 discussion whether they actually are, since it also looks like all of
 the options are explicitly listed, so in that sense we do not need to
 combine values by OR'ing, and it is hence not a bitfield.

 Bill's 23rd of April (pseudo)code is very accurate, and highlights the
 issues I am trying to address in my bindings. Bill, thanks for that.

 Pekka, you keep suggesting docenum only be used for documentation.
 However, types in richly typed languages *are* documentation (*). They
 indicate how you should use certain values. The fact that values are
 packaged as a certain enum type *documents* that you should probably
 use them in this or that way, without completely blocking use of the
 raw data (unless Jeroen's plan goes through).

 Ok, let me clarify: changing documentation alone cannot cause
 failures. That is what I mean by documentation-only here.

 Sure, you can use language constructs in a documentative way, but the
 main point here is can this cause build or runtime failures if it
 suddenly changes.

 (*) In fact, in the case of Haskell, more often than not, you can find
 functions in an API purely by searching for its type: the type
 information of functions documents enough to know their behaviour. In
 some languages, the difference between code and documentation is not
 so clear-cut.

 The entire reason I am asking for this enum attribute is so that the
 code I generate is more self-documenting. It does not enable the
 bindings to compute things it otherwise wouldn't be able to: packaged
 (u)ints are still (u)ints. It's to help the user understand the API,
 and unless they know better, suggest how they should interact with
 wayland.

 Then I didn't understand your intent before. I thought you wanted to use
 the enum types in the API you generate, which in strictly typed
 languages means you get build failures if you try to use a value not in
 the enum.

 If it cannot cause build or runtime failures, then it's fine.

 Jeroen's request, I think, stands separately from this: Jeroen is
 asking for strict guarantees on what values are and are not allowed in
 enums. Perhaps this should be moved into a separate thread?

 I (mistakenly?) took that using the types in the API means that you get
 these strict value guarantees Jeroen is asking for.

 Can you explain how you will use the enum types in the API without
 hitting the type checks of a strictly typed language? The answer is
 probably language-specific, but that is ok. We only need to answer the
 can it break? question, and define the attribute semantics so that it
 cannot break.

 In fact, that is the definition for the docenum attribute: changing,
 adding, or removing it from the XML file cannot cause any new build or
 runtime failures. Otherwise use it any way in a generator you want. It
 is similar to the summary attribute and description tag.

 So in summary:

 - Currently, there is no systematic way to match (u)ints that are
 passed around with enums that are defined by the protocol. This makes
 the enums, from a technical point of view, somewhat disconnected
 from the (u)int arguments.

 - Currently, there is no systematic way to tell if bitwise
 computations on enum values are supposed to be possible (as per Bill's
 example code).

 - Currently, there is no guarantee that only certain (u)int values
 will be exchanged under a given version of a protocol.

 - It would be much welcomed by bindings and documentation generators
 if there would be a *semantic* correspondence between (u)ints and
 enums. This correspondence would merely indicate how certain (u)ints
 are to be interpreted by matching them with an enum. This feature
 alone should not generate an API that is in any sense stronger or
 weaker than without such information (iow, access to the underlying

Re: [PATCH wayland] Add enum attribute to arg elements

[Auke Booij]

Apologies for my lack of responses, I have been abroad for a few days.

On 23 April 2015 at 10:38, Pekka Paalanen ppaala...@gmail.com wrote:
 This is a sort of sanity condition on being a bitfield: it does not
 require all combinations are valid, but it also distinguishes it from
 a regular enum.

 Is that an important distinction to make? That a bitfield with too
 many restrictions must not be a bitfield?

Yes, because what the bitfield flag indicates is that it is useful to
think of the values as flags. It means that the values can somehow be
combined. If there is no way to validly combine them, then it is not
useful to think of them like flags. And if the right way to combine
them is not OR'ing, then what you are talking about is not a bitfield.
I am not saying it is a kind of value that is unwelcome in the wayland
API, merely that it is not in the same category as what is usually
considered a bitfield.

And indeed some things (e.g. wl_shell_surface.resize and
xdg_surface.resize_edge) kinda look like a bitfield, but it is up for
discussion whether they actually are, since it also looks like all of
the options are explicitly listed, so in that sense we do not need to
combine values by OR'ing, and it is hence not a bitfield.

Bill's 23rd of April (pseudo)code is very accurate, and highlights the
issues I am trying to address in my bindings. Bill, thanks for that.

Pekka, you keep suggesting docenum only be used for documentation.
However, types in richly typed languages *are* documentation (*). They
indicate how you should use certain values. The fact that values are
packaged as a certain enum type *documents* that you should probably
use them in this or that way, without completely blocking use of the
raw data (unless Jeroen's plan goes through).

(*) In fact, in the case of Haskell, more often than not, you can find
functions in an API purely by searching for its type: the type
information of functions documents enough to know their behaviour. In
some languages, the difference between code and documentation is not
so clear-cut.

The entire reason I am asking for this enum attribute is so that the
code I generate is more self-documenting. It does not enable the
bindings to compute things it otherwise wouldn't be able to: packaged
(u)ints are still (u)ints. It's to help the user understand the API,
and unless they know better, suggest how they should interact with
wayland.

Jeroen's request, I think, stands separately from this: Jeroen is
asking for strict guarantees on what values are and are not allowed in
enums. Perhaps this should be moved into a separate thread?


So in summary:

- Currently, there is no systematic way to match (u)ints that are
passed around with enums that are defined by the protocol. This makes
the enums, from a technical point of view, somewhat disconnected
from the (u)int arguments.

- Currently, there is no systematic way to tell if bitwise
computations on enum values are supposed to be possible (as per Bill's
example code).

- Currently, there is no guarantee that only certain (u)int values
will be exchanged under a given version of a protocol.

- It would be much welcomed by bindings and documentation generators
if there would be a *semantic* correspondence between (u)ints and
enums. This correspondence would merely indicate how certain (u)ints
are to be interpreted by matching them with an enum. This feature
alone should not generate an API that is in any sense stronger or
weaker than without such information (iow, access to the underlying
(u)ints should still be available), but it may look different (e.g.
(u)ints packaged in a type that refers to the right enum).

- In strongly typed languages, hints on generating the right API shape
for bitfield-style arguments would be welcomed (as per Bill's example
code). However, details of when something is a bitfield need to be
discussed.

- In strongly typed languages, guarantees on which enum values are
allowed in the protocol would be welcomed. However, worries about
compatibility (both between languages and between versions) need to be
worked out in detail. Also, how do such guarantees combine with the
bitfield flag?

- Whereas interface names are global, enums are named locally in
interfaces. However, there might be a need to refer to another
interface's enum. However, this is a problem that can be solved
later as it comes up (for now the enum attribute would use local
names). As far as I am aware, there is no such situation in any common
protocol right now.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland] Add enum attribute to arg elements

[Auke Booij]

On 23 April 2015 at 08:38, Pekka Paalanen ppaala...@gmail.com wrote:
 On Wed, 22 Apr 2015 11:47:51 +0200
 Auke Booij a...@tulcod.com wrote:

 On 22 April 2015 at 08:34, Pekka Paalanen ppaala...@gmail.com wrote:
  I also think this discussion is going off-topic. You wanted to add
  annotations to the XML, so you could find out about enum and bitfield
  arguments, so let's keep to that. There is value in simplicity.
 
 
  How about this:
 
  Add three new, mutually exclusive attributes for arg tags:
 
  docenum=enumname
 
  Docenum would be for documentation linking only, and should not affect
  code generation. The effect would be in the documentation to add a link
  to the definition of the enumname enum. This attribute is
  applicable for both int and uint type args.
 
  enumeration=enumname
 
  Enumeration would be a doc link, but also specify that the enum
  enumname is a complete enumeration: no other values are legal. You
  can use this for code generation. You will rely on interface version
  negotiation to avoid unknown values in case you have an old definition
  of the interface and your opponent is using a new definition which
  added values. This attribute is applicable for both int and uint type
  args.
 
  bitfield=enumname
 
  Bitfield would be a doc link, but also specify that the values listed
  in enum enumanme can be orred together to form new legal values.
  Bits that are not settable by using the listed values must be zero. You
  rely on the interface versioning to avoid getting undefined bits set,
  just like enumeration relies for adding new values. This attribute is
  applicable only to uint type args.
 
 
  So, both enumeration and bitfield could be used for codegen, docenum
  would not. Docenum or nothing would be used for cases that do not fit
  as enumeration or bitfield, including cases where unknown values are
  always allowed but the interface specification defines what to do when
  encountering one (ignore, use as a literal value, ...).
 
  Wayland-scanner would generate the doclink references in header
  comments, and check that the referenced enum exists and the arg type.

 Two comments on this.

 1. I do not think it is necessary to distinguish between docenum and
 enumeration. At least for me, I am not asking for any new guarantees
 with regards to completeness: sure, new constants may be added on the
 fly. All I want is semantic information: which (u)int refers to which
 enum? And this data would be provided by your docenum (which I would
 just call enum).

 Jeroen seemed to be wanting that. That's why it gets complicated.

I agree. Well, if there is support for Jeroen's ideas, I will add that
to the scanner.c patch. I am not against them, but I would expect the
guarantees to be broken by C code all the time, so there needs to be a
solution for that.

 I agree that it should express that OR'ing together any number of
 values from the list should *at least* not be considered a protocol
 error. They may still be disregarded for other reasons, but passing
 OR'ed values in a bitfield should not break protocol, and this is the
 guarantee a bitfield flag should express (IMO).

 I'm not sure about that. The allowance to or things together was more
 related to the strict enumeration enforcement.

 Interfaces should be able to still say, that a certain combination is
 illegal and will lead to a protocol error.

 None of it will ever break protocol on the wire level, as it is all
 just uints.

 Given this and that they must not affect codegen, what are the remaining
 differences between enums and bitfields? Enum can be an int, but a
 bitfield cannot? Is it worth to have the distinction in the language at
 all?

 In the end, is the only really useful thing left the doc linking?

Well, for one, there is a semantic difference, which in richly typed
languages is not to be underestimated. Perhaps the requirement should
be that:
at least one (non-empty) combination of listed values OR'ed together
results in a new valid value (ie one not listed)

This is a sort of sanity condition on being a bitfield: it does not
require all combinations are valid, but it also distinguishes it from
a regular enum.

 It seems the main difference between your and Jeroen's requirements is
 that Jeroen wants strict guarantees so that codegen can generate the
 strictest APIs in his language.

I agree. Like I said, such guarantees are welcome, as long as the
situation is also clear for C, so that we don't get any annoying
incompatibilities due to misunderstandings between languages. At this
point I am not asking for them.


I fixed Bill's note on my sample patch, and will publish my
intermediary work on my github [0] (probably this will be limited to
just two commits, but you can subscribe if you want to).

[0] https://github.com/tulcod/wayland/tree/enum-attribute
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http

Re: [PATCH wayland] Add enum attribute to arg elements

[Auke Booij]

On 22 April 2015 at 08:34, Pekka Paalanen ppaala...@gmail.com wrote:
 I also think this discussion is going off-topic. You wanted to add
 annotations to the XML, so you could find out about enum and bitfield
 arguments, so let's keep to that. There is value in simplicity.


 How about this:

 Add three new, mutually exclusive attributes for arg tags:

 docenum=enumname

 Docenum would be for documentation linking only, and should not affect
 code generation. The effect would be in the documentation to add a link
 to the definition of the enumname enum. This attribute is
 applicable for both int and uint type args.

 enumeration=enumname

 Enumeration would be a doc link, but also specify that the enum
 enumname is a complete enumeration: no other values are legal. You
 can use this for code generation. You will rely on interface version
 negotiation to avoid unknown values in case you have an old definition
 of the interface and your opponent is using a new definition which
 added values. This attribute is applicable for both int and uint type
 args.

 bitfield=enumname

 Bitfield would be a doc link, but also specify that the values listed
 in enum enumanme can be orred together to form new legal values.
 Bits that are not settable by using the listed values must be zero. You
 rely on the interface versioning to avoid getting undefined bits set,
 just like enumeration relies for adding new values. This attribute is
 applicable only to uint type args.


 So, both enumeration and bitfield could be used for codegen, docenum
 would not. Docenum or nothing would be used for cases that do not fit
 as enumeration or bitfield, including cases where unknown values are
 always allowed but the interface specification defines what to do when
 encountering one (ignore, use as a literal value, ...).

 Wayland-scanner would generate the doclink references in header
 comments, and check that the referenced enum exists and the arg type.

Two comments on this.

1. I do not think it is necessary to distinguish between docenum and
enumeration. At least for me, I am not asking for any new guarantees
with regards to completeness: sure, new constants may be added on the
fly. All I want is semantic information: which (u)int refers to which
enum? And this data would be provided by your docenum (which I would
just call enum).

Completeness of enums is information that can be encoded in strongly
typed languages, but I do not think such guarantees are necessary.
Bindings should be able to deal with new constants not listed in the
protocol. If we guarantee completeness of enums, I expect we will get
all kinds of backwards compatibility issues.

2. I would say the property of being a bitfield belongs to an enum,
not to an argument: the type that is generated corresponds to an enum,
not to an argument. And as such, the properties of this type should
not depend on the argument's attributes. So I would say the bitfield
attribute should go in the enum. Otherwise, what is meant if one
argument uses an enum as a bitfield, and another argument uses the
same enum as a regular enum?

I agree that it should express that OR'ing together any number of
values from the list should *at least* not be considered a protocol
error. They may still be disregarded for other reasons, but passing
OR'ed values in a bitfield should not break protocol, and this is the
guarantee a bitfield flag should express (IMO).

 However, a remaining problem is that an interface cannot reference an
 enum defined for a different interface. That is a special case I
 don't know if we should support in this scheme, because it would
 require the depended-on XML file to be used during parsing, and we do
 not require that for wayland-scanner. So I'd rather leave that special
 case for pure documentation and no attribute, at least for now.
 Developing a way to reference external interfaces is for another time.

I agree.

 All that said, this is just a quick draft I haven't really thought
 through. You could name the attributes differently, change docenum to
 be an attribute for description rather than arg, etc.

I will see if I can write a wayland-scanner patch for the above ideas,
so that we all know what we're talking about.

 Thanks,
 pq

Thanks to you for listening and taking these ideas seriously!
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland] Add enum attribute to arg elements

[Auke Booij]

On 20 April 2015 at 09:03, Pekka Paalanen ppaala...@gmail.com wrote:
 Hi,

Thanks for taking the time to respond. I'll address your issues one by
one, but the overarching picture is that such typing information
should and would not be interpreted by bindings as a promise, but
rather as a hint.

 I'm starting to think that using the name enum may have been a minor
 mistake. We are using it to define namespaced constants in the XML
 files, similar to how you use #define in C with some naming conventions.

 True, we use the enum type defitions to define these constants in C
 rather than #defines. I'm not sure if that has any more rationale
 behind it than it was convenient and allows debuggers to explain the
 values when cast to the enum type.

 Nothing guarantees, that an enum element strictly describes a complete
 enumeration or even a bitfield. You can have entries which contain
 several bits of a bitfield. You don't have to list all allowed
 entries, and so on. You could even have an enum that lists only
 special values and then say a set of other values are allowed, e.g.
 list some negative specials and say all positive values are also
 allowed. We have never excluded this kind of use, though I don't think
 we use it in the core. It doesn't say anything about extensions not in
 core, though.

Perhaps it is worth emphasizing that this is not about making
guarantees (although they would be much appreciated). This is about
being able to make an educated guess what the most appropriate API
would be.

Currently, when my bindings encounter an enum, they basically tell the
user uhh, it's an integer..., which, in the context of richly typed
languages, is extremely awkward. It would be much better if I could
tell the user Well, here's this bitfield object, and here are some
flags for you to play with. Oh and by the way, really it's just a
packaged integer. This would expose the full richness of the
underlying C API (without breaking in the way you suggested), but does
a much better job at providing something more sane (in the context of
richly typed languages).

 Enumerations could be incomplete. For instance in the current
 xdg-shell.xml proposal in Weston, xdg_surface.state definition says
 that others are welcome to use other values from their own ranges.

... and as per the above, this would be fully allowed, since the
internal integer representation is still available.

 Also, adding the strict type information to the XML spec has no benefit
 for C, which is the de facto language for Wayland core developers. A C
 compiler also does not raise errors if you violate the rules.

It also does not raise errors if you mismanage your memory
(uninitialized memory, memory leaks, buffer overflow...), or if you
forget to deal with a certain input combination, or if you declare but
not define a function, ...

With all due respect, this is not a good reason to block other
languages from attempting to provide such information. If bindings
erroneously not compile (ie. they error our while they shouldn't),
then that's their problem, not yours. But this will typically hint at
some underlying misunderstanding. In fact, in many occassions it was
due to the Haskell typing system that I investigated internals of
wayland, which taught me about all kinds subtleties I'm glad I didn't
miss. The enum debate fits squarely in this kind of situation.

 Making the enum rules more strict has a possibility to break existing
 users, but to me it is unclear if the benefit would outweigh that
 con or the freedom.

Again, consider this the responsibility of bindings writers.

 On the wire, an enum or bitfield is still just an uint (or int), and a
 buggy client or server may cause the other to receive illegal values.
 Do the strongly typed languages have checks against that? Can you
 define what happens if the value is illegal for an enum? Or do you have
 to write that check manually in any case?

In my case, I would really just expose a nicely packaged integer value
(ie. the wire value), along with an API to e.g. match flags, or match
enums. In other words, this would be perfectly allowed. But the, say,
bitfield API would make much, much more sense from the user's point of
view: bitwise computations are not just uncommon in Haskell, they're
virtually nonexistent. And as long as the user wants to follow the
hint and interpret the value as a bitfield, say, the user can do that.
And if the user knows that something new was introduced in the
protocol, then this information is available as well.


I hope this answers your questions.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: Protocol (scanner) proposals

[Auke Booij]

As discussed in IRC, nevermind this. I misread the header files.

On 4 August 2014 01:12, Auke Booij a...@tulcod.com wrote:
 On 30 July 2014 20:27, Pekka Paalanen ppaala...@gmail.com wrote:
 Actually no. Binding writers are expected to write their own code
 generator for their language, like wayland-scanner is for C. You
 are expected to not use the static inline generated C functions.

 All the static inline functions are not part of the... how was it
 defined, ABI? Inline functions obviously get compiled into the app,
 not exported by the library.

 what about the wl_interface structs? they are exposed by the
 wayland-*-protocol.h files, so presumably off-limits, but opaque
 structs otherwise (the definition in wayland-util.h is deprecated), so
 I can't create my own wl_interface structs.

 hence, what am I supposed to pass to wl_proxy_marshal_constructor ?
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: Protocol (scanner) proposals

[Auke Booij]

On 3 August 2014 09:08, Pekka Paalanen ppaala...@gmail.com wrote:
 It was, hidden in a small paragraph in the documentation...

I have to apologize, you are right.

 I guess that this is convention in C?

 [...]

Thanks for your perfectly clear explanation, that does clarify it a lot.

 Speaking of which, is there a reason the code samples don't show up
 correctly in the online documentation?

 Which code samples exactly?

http://wayland.freedesktop.org/docs/html/chap-Library.html
- wl_list
- wl_display_prepare_read
- wl_proxy_marshal (but this code sample doesn't seem to be in the
current git source?)
http://wayland.freedesktop.org/docs/html/sect-Library-Server.html
- (again wl_list)
- wl_listener
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: Protocol (scanner) proposals

[Auke Booij]

On 30 July 2014 20:27, Pekka Paalanen ppaala...@gmail.com wrote:
 Actually no. Binding writers are expected to write their own code
 generator for their language, like wayland-scanner is for C. You
 are expected to not use the static inline generated C functions.

 All the static inline functions are not part of the... how was it
 defined, ABI? Inline functions obviously get compiled into the app,
 not exported by the library.

what about the wl_interface structs? they are exposed by the
wayland-*-protocol.h files, so presumably off-limits, but opaque
structs otherwise (the definition in wayland-util.h is deprecated), so
I can't create my own wl_interface structs.

hence, what am I supposed to pass to wl_proxy_marshal_constructor ?
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: Protocol (scanner) proposals

[Auke Booij]

On 30 July 2014 20:27, Pekka Paalanen ppaala...@gmail.com wrote:
 Actually no. Binding writers are expected to write their own code
 generator for their language, like wayland-scanner is for C. You
 are expected to not use the static inline generated C functions.

Okay, that makes some amount of sense. But in that case, that should
be documented.

 Instead the functions actually exported by the library are the ABI.

I guess that this is convention in C?

 I would love to see documentation patches!

Speaking of which, is there a reason the code samples don't show up
correctly in the online documentation?

 In fact, you inspired me to draw a picture, attached. Maybe that
 could be integrated in the Wayland docs somehow.

Yes, this would be helpful.

 Only when those protocols a) become stable, and b) are actually
 intended to be public and not specific to Weston.

 I think the latest move was wl_subcompositor.

Got it.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Protocol (scanner) proposals

[Auke Booij]

I am in the process of writing bindings to Wayland for Haskell, and
have run into some issues which I think you may be interested in
solving.

They all relate to the protocol files.

1. Haskell is a very richly typed language, which means that if two
integers have different meaning (e.g. length of an array and a file
descriptor), then they should have different types. In the wayland
protocol, you have assigned some semantic meaning to a particular
class of integer values you guys call enum (*) (e.g. wl_shm.format).
This is great to have for both programmers and binding writers, except
that to properly process this in the bindings, I need to know which
enum is used where: which message arguments have the type of which
enum.

Therefore I propose to add a new enum attribute to the arg element
in the XML specifications, whose string value simply corresponds to
the name of the enum whose values it assumes - but this should only be
used when the type of the arg is an int or a uint (?).

1a. (*) enum is short for enumeration: a complete list of possible
values it can have. But some of your enums can actually assume
several values simultaneously (or none at all), such as
wl_seat.capability - such enums have a fundamentally different
meaning and technical usage, and I wouldn't call them enum.

Hence, I propose to add a bitfield attribute to the enum element
in the XML specifications. If its value is set to true, then any
number of the values are to be added additively (each at most once) to
be presented as the actual integer value (uniqueness of additions is
to be guaranteed by the protocol designers - in other words, use
powers of 2 for your values).

Note that in this understanding, wl_shell_surface.resize is /not/ a
bitfield, since indeed it /does/ list all possible combinations
explicitly.

This is not a very important proposal to me, but I think
a. it could help people understand the protocol (even though it should
be obvious from the fact that such enums consist of values which are
powers of 2), and
b. it would enable a very minor improvement in the bindings I'm
writing (again, a bitfield is a particular kind of type, which the
language has special libraries for - and actual enums correspond
very nicely to sum types without parameters, which in the Haskell
world is a big thing).


The general message behind the above is that the more precisely the
protocol is specified, the better I can write bindings. Indeed, to
bind the functions in, say, wayland-client.h, I often dive deep into
the code to see what it really does - a typical state of affairs for
library binders.


2. The current wayland-scanner produces a huge amount of static
inline functions. In C, this doesn't make a big impact, but the fact
that these functions are all static means that one cannot link to
them, so that anyone writing language bindings is forced to write C
code calling all the individual functions - whereas linkable functions
often don't even require a single line of C code.

Now, I assume that the inline is to help with efficiency, and that
the functions are static because you guys somewhat arbitrarily put
these definitions in the header files.

I propose to move the implementations of all these functions to the
various -protocol.c files, ie, to the library, and only put their
declarations in the header files.

This will force an extra call jump. However, this can be fixed by
placing wl_proxy_marshal and wl_proxy_marshal_constructor in the
headers, and making that static inline instead. You have now reduced
the number of static inline functions by like, a factor 1000 or
something, but with the same number of jumps, and a much more concise
header file.

Additionally, this would make it possible to place more of the
internal stuff actually internally: no more need to even declare
wl_proxy_marshal(_constructor), or the list of wl_interface structs.
Although presumably you guys like having access to internals.

(Don't forget to also move around the add_listener and
get/set_user_data functions.)

Now, I don't know the impact of this change on the ABI, and on
applications linked to the current setup. Presumably stuff linked
against the current wayland won't work anymore, because they link to
wl_proxy_marshal which has now been made static inline. But
recompilation should fix it, since the functions still exist with the
same names. I think.



I would be happy to work on patches for all presented ideas, but
thought I'd ask for your feedback first.


Bonus input:

3. I am working on a document listing the things I found nontrivial or
unexpected
https://github.com/tulcod/haskell-wayland/blob/master/NOTES.md
You may wish to look at it, to possibly incorporate in your documentation?

4. I think that you should place the protocol files that are currently
in the weston repository (weston/protocol/*.xml) to the wayland
repository, since it may be of interest to other implementations of
compositors as well (and then having to depend on weston would 

haskell libxkbcommon bindings (also: wayland bindings help request)

[Auke Booij]

I wrote haskell bindings for libxkbcommon.

https://github.com/tulcod/haskell-xkbcommon

I appreciate *any* feedback

Announcement in haskell-cafe:
http://www.haskell.org/pipermail/haskell-cafe/2013-August/108333.html

Once I am happy with these bindings (I would currently call them
alpha) I would like to continue writing haskell bindings for
wayland. If anyone has any ideas about this or would like to help,
please respond or contact me.

Disclaimer: I am not exactly an experienced haskell developer (nor
X11/wayland developer, for that matter).

PS: Thanks to Daniel Stone for information and ideas.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel