[Mesa-dev] [PATCH] i965: Replace incorrect use of GLboolean with enum brw_compression.

Ian Romanick idr at freedesktop.org
Mon Oct 10 11:05:23 PDT 2011


On 10/07/2011 08:28 PM, Kenneth Graunke wrote:
> brw_set_compression_control took a GLboolean as an argument, then
> promptly used a switch statement to compare it with various enumeration
> values.  Clearly it's not actually a boolean.
>
> Introduce a new enumeration type, enum brw_compression, and use that.
>
> Found by converting GLboolean to bool; clang then gave warnings about
> switching on a boolean and ultimately duplicated case errors.
>
> Signed-off-by: Kenneth Graunke<kenneth at whitecape.org>
> ---
>   src/mesa/drivers/dri/i965/brw_defines.h |    8 +++++---
>   src/mesa/drivers/dri/i965/brw_eu.c      |    4 +++-
>   src/mesa/drivers/dri/i965/brw_eu.h      |    2 +-
>   3 files changed, 9 insertions(+), 5 deletions(-)
>
> Only compile tested.  I think this demonstrates that using stdbool instead of

I think this warrants at least a piglit run.  I'd like to avoid a repeat 
of ebca47a and fe006a7. :)

> GLboolean is worthwhile: the compiler actually recognizes it as a boolean
> data type and offers appropriate warnings---even errors!---when you do stupid
> things like this.
>
> I have a follow-on patch that actually does the GLboolean->bool conversion,
> should we decide to go that route.  Needs a bit more clean-up but shouldn't
> take too long.
>
> diff --git a/src/mesa/drivers/dri/i965/brw_defines.h b/src/mesa/drivers/dri/i965/brw_defines.h
> index a111630..21a115b 100644
> --- a/src/mesa/drivers/dri/i965/brw_defines.h
> +++ b/src/mesa/drivers/dri/i965/brw_defines.h
> @@ -486,9 +486,11 @@
>   #define BRW_CHANNEL_Z     2
>   #define BRW_CHANNEL_W     3
>
> -#define BRW_COMPRESSION_NONE          0
> -#define BRW_COMPRESSION_2NDHALF       1
> -#define BRW_COMPRESSION_COMPRESSED    2
> +enum brw_compression {
> +   BRW_COMPRESSION_NONE       = 0,
> +   BRW_COMPRESSION_2NDHALF    = 1,
> +   BRW_COMPRESSION_COMPRESSED = 2,
> +};
>
>   #define GEN6_COMPRESSION_1Q		0
>   #define GEN6_COMPRESSION_2Q		1
> diff --git a/src/mesa/drivers/dri/i965/brw_eu.c b/src/mesa/drivers/dri/i965/brw_eu.c
> index 0e04af9..b5a858b 100644
> --- a/src/mesa/drivers/dri/i965/brw_eu.c
> +++ b/src/mesa/drivers/dri/i965/brw_eu.c
> @@ -99,7 +99,9 @@ void brw_set_access_mode( struct brw_compile *p, GLuint access_mode )
>      p->current->header.access_mode = access_mode;
>   }
>
> -void brw_set_compression_control( struct brw_compile *p, GLboolean compression_control )
> +void
> +brw_set_compression_control(struct brw_compile *p,
> +			    enum brw_compression compression_control)
>   {
>      p->compressed = (compression_control == BRW_COMPRESSION_COMPRESSED);
>
> diff --git a/src/mesa/drivers/dri/i965/brw_eu.h b/src/mesa/drivers/dri/i965/brw_eu.h
> index 31334ce..8b2222b 100644
> --- a/src/mesa/drivers/dri/i965/brw_eu.h
> +++ b/src/mesa/drivers/dri/i965/brw_eu.h
> @@ -790,7 +790,7 @@ void brw_push_insn_state( struct brw_compile *p );
>   void brw_set_mask_control( struct brw_compile *p, GLuint value );
>   void brw_set_saturate( struct brw_compile *p, GLuint value );
>   void brw_set_access_mode( struct brw_compile *p, GLuint access_mode );
> -void brw_set_compression_control( struct brw_compile *p, GLboolean control );
> +void brw_set_compression_control(struct brw_compile *p, enum brw_compression c);
>   void brw_set_predicate_control_flag_value( struct brw_compile *p, GLuint value );
>   void brw_set_predicate_control( struct brw_compile *p, GLuint pc );
>   void brw_set_predicate_inverse(struct brw_compile *p, bool predicate_inverse);



More information about the mesa-dev mailing list