[Mesa-dev] [PATCH] i915: Fix texcoord vs. varying collision in fragment programs

[Matt Turner]

On Wed, Aug 13, 2014 at 10:07 AM,  <ville.syrjala at linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> i915 fragment programs utilize the texture coordinate registers
> for both texture coordinates and varyings. Unfortunately the
> code doesn't check if the same index might be in use for both.
> It just naively uses the index to pick a texture unit, which
> could lead to collisions.
>
> Add an extra mapping step to allocate non conflicting texture
> units for both uses.
>
> The issue can be reproduced with a pair of simple shaders like
> these:
>  attribute vec4 in_mod;
>  varying vec4 mod;
>  void main() {
>    mod = in_mod;
>    gl_TexCoord[0] = gl_MultiTexCoord0;
>    gl_Position = gl_ModelViewProjectionMatrix * gl_Vertex;
>  }
>
>  varying vec4 mod;
>  uniform sampler2D tex;
>  void main() {
>    gl_FragColor = texture2D(tex, vec2(gl_TexCoord[0])) * mod;
>  }
>
> This was tested on a PNV.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  src/mesa/drivers/dri/i915/i915_context.h  | 14 ++---
>  src/mesa/drivers/dri/i915/i915_fragprog.c | 86 +++++++++++++++++++++++++------
>  2 files changed, 74 insertions(+), 26 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i915/i915_context.h b/src/mesa/drivers/dri/i915/i915_context.h
> index 34af202..2615bd6 100644
> --- a/src/mesa/drivers/dri/i915/i915_context.h
> +++ b/src/mesa/drivers/dri/i915/i915_context.h
> @@ -115,6 +115,8 @@ enum {
>     I915_RASTER_RULES_SETUP_SIZE,
>  };
>
> +#define I915_TEX_UNITS 8
> +
>  #define I915_MAX_CONSTANT      32
>  #define I915_CONSTANT_SIZE     (2+(4*I915_MAX_CONSTANT))
>
> @@ -194,7 +196,8 @@ struct i915_fragment_program
>
>     /* Helpers for i915_fragprog.c:
>      */
> -   GLuint wpos_tex;
> +   uint8_t texcoord_mapping[I915_TEX_UNITS];
> +   uint8_t wpos_tex;

I see idr committed this. Coverity is warning about "p->wpos_tex !=
-1" always being true, but I'm not sure why it wouldn't have
recognized that before -- wpos_tex was still unsigned.

It dates back to commit fceda4342cad.

Not sure if it's worth fixing.