[PATCH] Declare enumeration wl_output.transform as bitfield.

[Pekka Paalanen]

On Fri, 6 Nov 2015 16:05:10 +0100
"Nils Chr. Brause" <nilschrbrause at gmail.com> wrote:

> Hi,
> 
> On Fri, Nov 6, 2015 at 3:48 PM, Auke Booij <auke at tulcod.com> wrote:
> > On 6 November 2015 at 13:03, Nils Christopher Brause
> > <nilschrbrause at 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.


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