[Mesa-dev] [PATCH] gallium: increase pipe_sampler_view::target bitfield size for MSVC

[Brian Paul]

On 11/01/2017 10:33 AM, Roland Scheidegger wrote:
> Oh no msvc enums strike again... (A real pity c doesn't allow to
> explicitly make enums signed or unsigned.)

There's a proposal for that feature for a future std C version.

> We use bitfields for compact representation, so if we need an extra bit
> that seems a bit counterintuitive (even if it dosen't increase the
> overall size here). Also, I believe the 16 bits for format were chosen
> for slightly more efficient code.
> Worse though, needing one bit more than everybody thinks is necessary
> looks error-prone to me (albeit in this case it seems very unlikely the
> enum gets more values).

I'll certainly be on the lookout for this issue in the future.

> But I guess the alternative would be to just use unsigned:4 for the
> target. Not sure if that would be preferable, there's really no good
> solution, but in any case
>
> Reviewed-by: Roland Scheidegger <sroland at vmware.com>

Thanks.

-Brian

>
> Am 01.11.2017 um 15:45 schrieb Brian Paul:
>> MSVC treats enums as being signed.  The 4-bit target field isn't large
>> enough to correctly store the value 8 (for PIPE_TEXTURE_CUBE_ARRAY).
>> The bitfield value 0x8 was being interpreted as -8 so matching the
>> target with PIPE_TEXTURE_CUBE_ARRAY in switch statements, etc. was
>> failing.
>>
>> To keep the structure size the same, we reduce the format field from
>> 16 bits to 15.  There don't appear to be any other enum bitfields
>> which need to be adjusted.
>>
>> This fixes a number of Piglit cube map array tests.
>> ---
>>   src/gallium/include/pipe/p_state.h | 8 ++++++--
>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/gallium/include/pipe/p_state.h b/src/gallium/include/pipe/p_state.h
>> index 10bf678..90dc561 100644
>> --- a/src/gallium/include/pipe/p_state.h
>> +++ b/src/gallium/include/pipe/p_state.h
>> @@ -34,6 +34,10 @@
>>    * Basic notes:
>>    *   1. Want compact representations, so we use bitfields.
>>    *   2. Put bitfields before other (GLfloat) fields.
>> + *   3. enum bitfields need to be at least one bit extra in size so the most
>> + *      significant bit is zero.  MSVC treats enums as signed so if the high
>> + *      bit is set, the value will be interpreted as a negative number.
>> + *      That causes trouble in various places.
>>    */
>>
>>
>> @@ -436,8 +440,8 @@ struct pipe_surface
>>   struct pipe_sampler_view
>>   {
>>      struct pipe_reference reference;
>> -   enum pipe_format format:16;      /**< typed PIPE_FORMAT_x */
>> -   enum pipe_texture_target target:4; /**< PIPE_TEXTURE_x */
>> +   enum pipe_format format:15;      /**< typed PIPE_FORMAT_x */
>> +   enum pipe_texture_target target:5; /**< PIPE_TEXTURE_x */
>>      unsigned swizzle_r:3;         /**< PIPE_SWIZZLE_x for red component */
>>      unsigned swizzle_g:3;         /**< PIPE_SWIZZLE_x for green component */
>>      unsigned swizzle_b:3;         /**< PIPE_SWIZZLE_x for blue component */
>>
>