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. 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. > > > Thanks, > pq Cheers, Nils _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
