[Mesa-dev] [PATCH 20/25] mesa/r200/i915/i965: eliminate gl_fragment_program and use new shared shader_info

[Ian Romanick]

On 10/17/2016 11:12 PM, Timothy Arceri wrote:
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index 8cc76a4..9e9961b 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -1937,13 +1937,19 @@ struct gl_program
>  
>     GLboolean UsesGather; /**< Does this program use gather4 at all? */
>  
> -   /**
> -    * For vertex and geometry shaders, true if the program uses the
> -    * gl_ClipDistance output.  Ignored for fragment shaders.
> -    */
> -   unsigned ClipDistanceArraySize;
> -   unsigned CullDistanceArraySize;
> -
> +   union {
> +      /* Vertex and geometry shaders fields */
> +      struct {
> +         unsigned ClipDistanceArraySize;
> +         unsigned CullDistanceArraySize;
> +      };
> +
> +      /* Fragement shader only fields */
> +      struct {
> +         GLboolean OriginUpperLeft;
> +         GLboolean PixelCenterInteger;
> +      };
> +   };

NAK.  This makes it way too easy to have non-obvious bugs that are hard
to detect.  How long do you think someone would stare at the following
code without noticing the problem:

    p->ClipDistanceArraySize = some_value;
    ...
    p->OriginUpperLeft = false;
    p->PixelCenterInteger = false;

Other places where we use anonymous unions the inner structures have
names.  You might have some hope of noticing that this code is sketchy:

    p->vertex.ClipDistanceArraySize = some_value;
    ...
    p->fragment.OriginUpperLeft = false;
    p->fragment.PixelCenterInteger = false;


If we're that concerned about saving a few bytes per program, change the
ArraySize fields to be uint8_t.

>  
>     /** Named parameters, constants, etc. from program text */
>     struct gl_program_parameter_list *Parameters;