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

ville.syrjala at linux.intel.com ville.syrjala at linux.intel.com
Wed Aug 13 10:07:09 PDT 2014


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;
    bool depth_written;
 
    struct
@@ -205,15 +208,6 @@ struct i915_fragment_program
    GLuint nr_params;
 };
 
-
-
-
-
-
-
-#define I915_TEX_UNITS 8
-
-
 struct i915_hw_state
 {
    GLuint Ctx[I915_CTX_SETUP_SIZE];
diff --git a/src/mesa/drivers/dri/i915/i915_fragprog.c b/src/mesa/drivers/dri/i915/i915_fragprog.c
index 96fdd3c..05e1de6 100644
--- a/src/mesa/drivers/dri/i915/i915_fragprog.c
+++ b/src/mesa/drivers/dri/i915/i915_fragprog.c
@@ -72,6 +72,24 @@ static const GLfloat cos_constants[4] = { 1.0,
    -1.0 / (6 * 5 * 4 * 3 * 2 * 1)
 };
 
+/* texcoord_mapping[unit] = index | TEXCOORD_{TEX,VAR} */
+#define TEXCOORD_TEX (0<<7)
+#define TEXCOORD_VAR (1<<7)
+
+static GLint get_texcoord_mapping(struct i915_fragment_program *p,
+                                  uint8_t texcoord)
+{
+   GLint i;
+
+   for (i = 0; i < p->ctx->Const.MaxTextureCoordUnits; i++) {
+      if (p->texcoord_mapping[i] == texcoord)
+         return i;
+   }
+
+   /* blah */
+   return p->ctx->Const.MaxTextureCoordUnits - 1;
+}
+
 /**
  * Retrieve a ureg for the given source register.  Will emit
  * constants, apply swizzling and negation as needed.
@@ -82,6 +100,7 @@ src_vector(struct i915_fragment_program *p,
            const struct gl_fragment_program *program)
 {
    GLuint src;
+   GLint unit;
 
    switch (source->File) {
 
@@ -119,8 +138,10 @@ src_vector(struct i915_fragment_program *p,
       case VARYING_SLOT_TEX5:
       case VARYING_SLOT_TEX6:
       case VARYING_SLOT_TEX7:
+         unit = get_texcoord_mapping(p, (source->Index -
+                                         VARYING_SLOT_TEX0) | TEXCOORD_TEX);
          src = i915_emit_decl(p, REG_TYPE_T,
-                              T_TEX0 + (source->Index - VARYING_SLOT_TEX0),
+                              T_TEX0 + unit,
                               D0_CHANNEL_ALL);
 	 break;
 
@@ -132,8 +153,10 @@ src_vector(struct i915_fragment_program *p,
       case VARYING_SLOT_VAR0 + 5:
       case VARYING_SLOT_VAR0 + 6:
       case VARYING_SLOT_VAR0 + 7:
+         unit = get_texcoord_mapping(p, (source->Index -
+                                         VARYING_SLOT_VAR0) | TEXCOORD_VAR);
          src = i915_emit_decl(p, REG_TYPE_T,
-                              T_TEX0 + (source->Index - VARYING_SLOT_VAR0),
+                              T_TEX0 + unit,
                               D0_CHANNEL_ALL);
          break;
 
@@ -1190,27 +1213,55 @@ fixup_depth_write(struct i915_fragment_program *p)
    }
 }
 
+static void
+check_texcoord_mapping(struct i915_fragment_program *p)
+{
+   GLbitfield64 inputs = p->FragProg.Base.InputsRead;
+   GLint i;
+   GLint unit = 0;
+
+   for (i = 0; i < p->ctx->Const.MaxTextureCoordUnits; i++) {
+      if (inputs & VARYING_BIT_TEX(i)) {
+         if (unit >= p->ctx->Const.MaxTextureCoordUnits) {
+            unit++;
+            break;
+         }
+         p->texcoord_mapping[unit++] = i | TEXCOORD_TEX;
+      }
+      if (inputs & VARYING_BIT_VAR(i)) {
+         if (unit >= p->ctx->Const.MaxTextureCoordUnits) {
+            unit++;
+            break;
+         }
+         p->texcoord_mapping[unit++] = i | TEXCOORD_VAR;
+      }
+   }
+
+   if (unit > p->ctx->Const.MaxTextureCoordUnits)
+      i915_program_error(p, "Too many texcoord units");
+}
 
 static void
 check_wpos(struct i915_fragment_program *p)
 {
    GLbitfield64 inputs = p->FragProg.Base.InputsRead;
    GLint i;
+   GLint unit = 0;
 
    p->wpos_tex = -1;
 
+   if ((inputs & VARYING_BIT_POS) == 0)
+      return;
+
    for (i = 0; i < p->ctx->Const.MaxTextureCoordUnits; i++) {
-      if (inputs & (VARYING_BIT_TEX(i) | VARYING_BIT_VAR(i)))
-         continue;
-      else if (inputs & VARYING_BIT_POS) {
-         p->wpos_tex = i;
-         inputs &= ~VARYING_BIT_POS;
-      }
+      unit += !!(inputs & VARYING_BIT_TEX(i));
+      unit += !!(inputs & VARYING_BIT_VAR(i));
    }
 
-   if (inputs & VARYING_BIT_POS) {
+   if (unit < p->ctx->Const.MaxTextureCoordUnits)
+      p->wpos_tex = unit;
+   else
       i915_program_error(p, "No free texcoord for wpos value");
-   }
 }
 
 
@@ -1226,6 +1277,7 @@ translate_program(struct i915_fragment_program *p)
    }
 
    i915_init_program(i915, p);
+   check_texcoord_mapping(p);
    check_wpos(p);
    upload_program(p);
    fixup_depth_write(p);
@@ -1434,22 +1486,24 @@ i915ValidateFragmentProgram(struct i915_context *i915)
 
    for (i = 0; i < p->ctx->Const.MaxTextureCoordUnits; i++) {
       if (inputsRead & VARYING_BIT_TEX(i)) {
+         int unit = get_texcoord_mapping(p, i | TEXCOORD_TEX);
          int sz = VB->AttribPtr[_TNL_ATTRIB_TEX0 + i]->size;
 
-         s2 &= ~S2_TEXCOORD_FMT(i, S2_TEXCOORD_FMT0_MASK);
-         s2 |= S2_TEXCOORD_FMT(i, SZ_TO_HW(sz));
+         s2 &= ~S2_TEXCOORD_FMT(unit, S2_TEXCOORD_FMT0_MASK);
+         s2 |= S2_TEXCOORD_FMT(unit, SZ_TO_HW(sz));
 
          EMIT_ATTR(_TNL_ATTRIB_TEX0 + i, EMIT_SZ(sz), 0, sz * 4);
       }
-      else if (inputsRead & VARYING_BIT_VAR(i)) {
+      if (inputsRead & VARYING_BIT_VAR(i)) {
+         int unit = get_texcoord_mapping(p, i | TEXCOORD_VAR);
          int sz = VB->AttribPtr[_TNL_ATTRIB_GENERIC0 + i]->size;
 
-         s2 &= ~S2_TEXCOORD_FMT(i, S2_TEXCOORD_FMT0_MASK);
-         s2 |= S2_TEXCOORD_FMT(i, SZ_TO_HW(sz));
+         s2 &= ~S2_TEXCOORD_FMT(unit, S2_TEXCOORD_FMT0_MASK);
+         s2 |= S2_TEXCOORD_FMT(unit, SZ_TO_HW(sz));
 
          EMIT_ATTR(_TNL_ATTRIB_GENERIC0 + i, EMIT_SZ(sz), 0, sz * 4);
       }
-      else if (i == p->wpos_tex) {
+      if (i == p->wpos_tex) {
 	 int wpos_size = 4 * sizeof(float);
          /* If WPOS is required, duplicate the XYZ position data in an
           * unused texture coordinate:
-- 
1.8.5.5



More information about the mesa-dev mailing list