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

[Roland Scheidegger]

Oh no msvc enums strike again... (A real pity c doesn't allow to
explicitly make enums signed or unsigned.)
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).
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>

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 */
>