mesa: rework GLSL vertex attribute binding

Calls to glBindAttribLocation() should not take effect until the next time
that glLinkProgram() is called.
gl_shader_program::Attributes now just contains user-defined bindings.
gl_shader_program::VertexProgram->Attributes contains the actual/final bindings.

src/mesa/main/mtypes.h

@@ -2125,12 +2125,14 @@ struct gl_shader_program
    GLuint NumShaders;          /**< number of attached shaders */
    struct gl_shader **Shaders; /**< List of attached the shaders */
 
+   /** User-defined attribute bindings (glBindAttribLocation) */
+   struct gl_program_parameter_list *Attributes;
+
    /* post-link info: */
    struct gl_vertex_program *VertexProgram;     /**< Linked vertex program */
    struct gl_fragment_program *FragmentProgram; /**< Linked fragment prog */
    struct gl_uniform_list *Uniforms;
    struct gl_program_parameter_list *Varying;
-   struct gl_program_parameter_list *Attributes; /**< Vertex attributes */
    GLboolean LinkStatus;   /**< GL_LINK_STATUS */
    GLboolean Validated;
    GLchar *InfoLog;

src/mesa/shader/shader_api.c

@@ -1,6 +1,6 @@
 /*
  * Mesa 3-D graphics library
- * Version:  7.1
+ * Version:  7.2
  *
  * Copyright (C) 2004-2008  Brian Paul   All Rights Reserved.
  *
@@ -492,10 +492,14 @@ _mesa_get_attrib_location(GLcontext *ctx, GLuint program,
    if (!name)
       return -1;
 
-   if (shProg->Attributes) {
-      GLint i = _mesa_lookup_parameter_index(shProg->Attributes, -1, name);
-      if (i >= 0) {
-         return shProg->Attributes->Parameters[i].StateIndexes[0];
+   if (shProg->VertexProgram) {
+      const struct gl_program_parameter_list *attribs =
+         shProg->VertexProgram->Base.Attributes;
+      if (attribs) {
+         GLint i = _mesa_lookup_parameter_index(attribs, -1, name);
+         if (i >= 0) {
+            return attribs->Parameters[i].StateIndexes[0];
+         }
       }
    }
    return -1;
@@ -508,7 +512,7 @@ _mesa_bind_attrib_location(GLcontext *ctx, GLuint program, GLuint index,
 {
    struct gl_shader_program *shProg;
    const GLint size = -1; /* unknown size */
-   GLint i, oldIndex;
+   GLint i;
    GLenum datatype = GL_FLOAT_VEC4;
 
    shProg = _mesa_lookup_shader_program_err(ctx, program,
@@ -531,14 +535,6 @@ _mesa_bind_attrib_location(GLcontext *ctx, GLuint program, GLuint index,
       return;
    }
 
-   if (shProg->LinkStatus) {
-      /* get current index/location for the attribute */
-      oldIndex = _mesa_get_attrib_location(ctx, program, name);
-   }
-   else {
-      oldIndex = -1;
-   }
-
    /* this will replace the current value if it's already in the list */
    i = _mesa_add_attribute(shProg->Attributes, name, size, datatype, index);
    if (i < 0) {
@@ -546,12 +542,10 @@ _mesa_bind_attrib_location(GLcontext *ctx, GLuint program, GLuint index,
       return;
    }
 
-   if (shProg->VertexProgram && oldIndex >= 0 && oldIndex != index) {
-      /* If the index changed, need to search/replace references to that attribute
-       * in the vertex program.
-       */
-      _slang_remap_attribute(&shProg->VertexProgram->Base, oldIndex, index);
-   }
+   /*
+    * Note that this attribute binding won't go into effect until
+    * glLinkProgram is called again.
+    */
 }
 
 
@@ -763,24 +757,29 @@ _mesa_get_active_attrib(GLcontext *ctx, GLuint program, GLuint index,
                         GLsizei maxLength, GLsizei *length, GLint *size,
                         GLenum *type, GLchar *nameOut)
 {
+   const struct gl_program_parameter_list *attribs = NULL;
    struct gl_shader_program *shProg;
 
    shProg = _mesa_lookup_shader_program_err(ctx, program, "glGetActiveAttrib");
    if (!shProg)
       return;
 
-   if (!shProg->Attributes || index >= shProg->Attributes->NumParameters) {
+   if (shProg->VertexProgram)
+      attribs = shProg->VertexProgram->Base.Attributes;
+
+   if (!attribs || index >= attribs->NumParameters) {
       _mesa_error(ctx, GL_INVALID_VALUE, "glGetActiveAttrib(index)");
       return;
    }
 
-   copy_string(nameOut, maxLength, length,
-               shProg->Attributes->Parameters[index].Name);
+   copy_string(nameOut, maxLength, length, attribs->Parameters[index].Name);
+
    if (size)
-      *size = shProg->Attributes->Parameters[index].Size
-         / sizeof_glsl_type(shProg->Attributes->Parameters[index].DataType);
+      *size = attribs->Parameters[index].Size
+         / sizeof_glsl_type(attribs->Parameters[index].DataType);
+
    if (type)
-      *type = shProg->Attributes->Parameters[index].DataType;
+      *type = attribs->Parameters[index].DataType;
 }
 
 
@@ -878,6 +877,7 @@ static void
 _mesa_get_programiv(GLcontext *ctx, GLuint program,
                     GLenum pname, GLint *params)
 {
+   const struct gl_program_parameter_list *attribs;
    struct gl_shader_program *shProg
       = _mesa_lookup_shader_program(ctx, program);
 
@@ -886,6 +886,11 @@ _mesa_get_programiv(GLcontext *ctx, GLuint program,
       return;
    }
 
+   if (shProg->VertexProgram)
+      attribs = shProg->VertexProgram->Base.Attributes;
+   else
+      attribs = NULL;
+
    switch (pname) {
    case GL_DELETE_STATUS:
       *params = shProg->DeletePending;
@@ -903,11 +908,10 @@ _mesa_get_programiv(GLcontext *ctx, GLuint program,
       *params = shProg->NumShaders;
       break;
    case GL_ACTIVE_ATTRIBUTES:
-      *params = shProg->Attributes ? shProg->Attributes->NumParameters : 0;
+      *params = attribs ? attribs->NumParameters : 0;
       break;
    case GL_ACTIVE_ATTRIBUTE_MAX_LENGTH:
-      *params = _mesa_longest_parameter_name(shProg->Attributes,
-                                             PROGRAM_INPUT) + 1;
+      *params = _mesa_longest_parameter_name(attribs, PROGRAM_INPUT) + 1;
       break;
    case GL_ACTIVE_UNIFORMS:
       *params = shProg->Uniforms ? shProg->Uniforms->NumUniforms : 0;

src/mesa/shader/slang/slang_link.c

@@ -1,8 +1,8 @@
 /*
  * Mesa 3-D graphics library
- * Version:  6.5.3
+ * Version:  7.2
  *
- * Copyright (C) 2007  Brian Paul   All Rights Reserved.
+ * Copyright (C) 2008  Brian Paul   All Rights Reserved.
  *
  * Permission is hereby granted, free of charge, to any person obtaining a
  * copy of this software and associated documentation files (the "Software"),
@@ -215,74 +215,110 @@ link_uniform_vars(struct gl_shader_program *shProg,
  * For example, if the vertex shader declared "attribute vec4 foobar" we'll
  * allocate a generic vertex attribute for "foobar" and plug that value into
  * the vertex program instructions.
+ * But if the user called glBindAttributeLocation(), those bindings will
+ * have priority.
  */
 static GLboolean
 _slang_resolve_attributes(struct gl_shader_program *shProg,
-                          struct gl_program *prog)
+                          const struct gl_program *origProg,
+                          struct gl_program *linkedProg)
 {
+   GLint attribMap[MAX_VERTEX_ATTRIBS];
    GLuint i, j;
    GLbitfield usedAttributes;
 
-   assert(prog->Target == GL_VERTEX_PROGRAM_ARB);
+   assert(origProg != linkedProg);
+   assert(origProg->Target == GL_VERTEX_PROGRAM_ARB);
+   assert(linkedProg->Target == GL_VERTEX_PROGRAM_ARB);
 
    if (!shProg->Attributes)
       shProg->Attributes = _mesa_new_parameter_list();
 
+   if (linkedProg->Attributes) {
+      _mesa_free_parameter_list(linkedProg->Attributes);
+   }
+   linkedProg->Attributes = _mesa_new_parameter_list();
+
+
    /* Build a bitmask indicating which attribute indexes have been
     * explicitly bound by the user with glBindAttributeLocation().
     */
    usedAttributes = 0x0;
    for (i = 0; i < shProg->Attributes->NumParameters; i++) {
       GLint attr = shProg->Attributes->Parameters[i].StateIndexes[0];
-      usedAttributes |= attr;
+      usedAttributes |= (1 << attr);
+   }
+
+   /* initialize the generic attribute map entries to -1 */
+   for (i = 0; i < MAX_VERTEX_ATTRIBS; i++) {
+      attribMap[i] = -1;
    }
 
    /*
     * Scan program for generic attribute references
     */
-   for (i = 0; i < prog->NumInstructions; i++) {
-      struct prog_instruction *inst = prog->Instructions + i;
+   for (i = 0; i < linkedProg->NumInstructions; i++) {
+      struct prog_instruction *inst = linkedProg->Instructions + i;
       for (j = 0; j < 3; j++) {
          if (inst->SrcReg[j].File == PROGRAM_INPUT &&
              inst->SrcReg[j].Index >= VERT_ATTRIB_GENERIC0) {
-            /* this is a generic attrib */
-            const GLint k = inst->SrcReg[j].Index - VERT_ATTRIB_GENERIC0;
-            const char *name = prog->Attributes->Parameters[k].Name;
-            /* See if this attrib name is in the program's attribute list
-             * (i.e. was bound by the user).
+            /*
+             * OK, we've found a generic vertex attribute reference.
              */
-            GLint index = _mesa_lookup_parameter_index(shProg->Attributes,
-                                                          -1, name);
-            GLint attr;
-            if (index >= 0) {
-               /* found, user must have specified a binding */
-               attr = shProg->Attributes->Parameters[index].StateIndexes[0];
-            }
-            else {
-               /* Not found, choose our own attribute number.
-                * Start at 1 since generic attribute 0 always aliases
-                * glVertex/position.
+            const GLint k = inst->SrcReg[j].Index - VERT_ATTRIB_GENERIC0;
+
+            GLint attr = attribMap[k];
+
+            if (attr < 0) {
+               /* Need to figure out attribute mapping now.
+                */
+               const char *name = origProg->Attributes->Parameters[k].Name;
+               const GLint size = origProg->Attributes->Parameters[k].Size;
+               const GLenum type =origProg->Attributes->Parameters[k].DataType;
+               GLint index, attr;
+
+               /* See if there's a user-defined attribute binding for
+                * this name.
                 */
-               GLint size = prog->Attributes->Parameters[k].Size;
-               GLenum datatype = prog->Attributes->Parameters[k].DataType;
-               for (attr = 1; attr < MAX_VERTEX_ATTRIBS; attr++) {
-                  if (((1 << attr) & usedAttributes) == 0)
-                     break;
+               index = _mesa_lookup_parameter_index(shProg->Attributes,
+                                                    -1, name);
+               if (index >= 0) {
+                  /* Found a user-defined binding */
+                  attr = shProg->Attributes->Parameters[index].StateIndexes[0];
                }
-               if (attr == MAX_VERTEX_ATTRIBS) {
-                  /* too many!  XXX record error log */
-                  return GL_FALSE;
+               else {
+                  /* No user-defined binding, choose our own attribute number.
+                   * Start at 1 since generic attribute 0 always aliases
+                   * glVertex/position.
+                   */
+                  for (attr = 1; attr < MAX_VERTEX_ATTRIBS; attr++) {
+                     if (((1 << attr) & usedAttributes) == 0)
+                        break;
+                  }
+                  if (attr == MAX_VERTEX_ATTRIBS) {
+                     link_error(shProg, "Too many vertex attributes");
+                     return GL_FALSE;
+                  }
+
+                  /* mark this attribute as used */
+                  usedAttributes |= (1 << attr);
                }
-               _mesa_add_attribute(shProg->Attributes, name, size, datatype,attr);
 
-	       /* set the attribute as used */
-	       usedAttributes |= 1<<attr;
+               attribMap[k] = attr;
+
+               /* Save the final name->attrib binding so it can be queried
+                * with glGetAttributeLocation().
+                */
+               _mesa_add_attribute(linkedProg->Attributes, name,
+                                   size, type, attr);
             }
 
+            /* update the instruction's src reg */
             inst->SrcReg[j].Index = VERT_ATTRIB_GENERIC0 + attr;
          }
       }
    }
+
    return GL_TRUE;
 }
 
@@ -344,36 +380,6 @@ _slang_update_inputs_outputs(struct gl_program *prog)
 }
 
 
-/**
- * Scan a vertex program looking for instances of
- * (PROGRAM_INPUT, VERT_ATTRIB_GENERIC0 + oldAttrib) and replace with
- * (PROGRAM_INPUT, VERT_ATTRIB_GENERIC0 + newAttrib).
- * This is used when the user calls glBindAttribLocation on an already linked
- * shader program.
- */
-void
-_slang_remap_attribute(struct gl_program *prog, GLuint oldAttrib, GLuint newAttrib)
-{
-   GLuint i, j;
-
-   assert(prog->Target == GL_VERTEX_PROGRAM_ARB);
-
-   for (i = 0; i < prog->NumInstructions; i++) {
-      struct prog_instruction *inst = prog->Instructions + i;
-      for (j = 0; j < 3; j++) {
-         if (inst->SrcReg[j].File == PROGRAM_INPUT) {
-            if (inst->SrcReg[j].Index == VERT_ATTRIB_GENERIC0 + oldAttrib) {
-               inst->SrcReg[j].Index = VERT_ATTRIB_GENERIC0 + newAttrib;
-            }
-         }
-      }
-   }
-
-   _slang_update_inputs_outputs(prog);
-}
-
-
-
 /** cast wrapper */
 static struct gl_vertex_program *
 vertex_program(struct gl_program *prog)
@@ -492,9 +498,8 @@ _slang_link(GLcontext *ctx,
    /*_mesa_print_uniforms(shProg->Uniforms);*/
 
    if (shProg->VertexProgram) {
-      if (!_slang_resolve_attributes(shProg, &shProg->VertexProgram->Base)) {
-         /*goto cleanup;*/
-         _mesa_problem(ctx, "_slang_resolve_attributes() failed");
+      if (!_slang_resolve_attributes(shProg, &vertProg->Base,
+                                     &shProg->VertexProgram->Base)) {
          return;
       }
    }

src/mesa/shader/slang/slang_link.h

@@ -1,8 +1,8 @@
 /*
  * Mesa 3-D graphics library
- * Version:  6.5.3
+ * Version:  7.2
  *
- * Copyright (C) 2007  Brian Paul   All Rights Reserved.
+ * Copyright (C) 2008  Brian Paul   All Rights Reserved.
  *
  * Permission is hereby granted, free of charge, to any person obtaining a
  * copy of this software and associated documentation files (the "Software"),
@@ -32,10 +32,6 @@ extern void
 _slang_link(GLcontext *ctx, GLhandleARB h,
             struct gl_shader_program *shProg);
 
-extern void
-_slang_remap_attribute(struct gl_program *prog, GLuint oldAttrib,
-                       GLuint newAttrib);
-
 
 #endif