Hi, On Mon, Nov 9, 2015 at 3:39 PM, Auke Booij <a...@tulcod.com> wrote: > 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 reason to reject this patch. > > I commented on this last time. But let me bring up another point: if > everything that is just a collection of bits should be bitfield-able > (following your previous logic of "everything that can, should" here), > then so should the "fixed" type. And so "string" type (arguably it is > even better suited for this purpose than uint, since it allows a > practically unlimited number of flags, and if most of them are zero, > it can still transfer them efficiently). And so should "object" > (arguably it is even better than uint, because it is emphasized that > it should not be interpreted as an integer or natural number). > > This is the mess that I would like to avoid. For funny types like > those: well maybe you have a very good point in calling it a bitfield. > But once you exit the realm of uint, we're in nonstandard bitfield > terrain, and we cannot make any *systematic* promises from a scanner > point of view. So although you're allowed to package bitfields in an > int or a string or whatever you like, we cannot deal with these > systematically, since the only thing that we *define* systematically > is the uint case.
Would you be fine with having an exception for wl_output.transform being added to the C scanner? That way the bitfield could be properly marked as such and the scanner would still be able to enforce rules on all the other bitfields as well as new ones. > >> Nevertheless, this enumeration will be a bitfield in the C++ bindings, even >> if that means that I have to add an exception in the scanner. > > That is very reasonable. It's wl_output::transform that is the > exception to the rule, and everyone agrees that this is purely for > historical reasons: there is no deeper meaning behind it being an int, > and it was a mistake, and it's one all bindings will have to deal with > (if they deem it necessary) since the protocol is now stable. Cheers, Nils _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel