[PATCH] Declare enumeration wl_output.transform as bitfield.

[Nils Chr. Brause]

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

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

Cheers,
Nils