[cogl] Use the same number for n_tex_coord_attribs in all linked shaders

[Neil Roberts <nroberts src gnome org>]

commit 00f0a66ec949a641db744f56230c2bcfc98c1883
Author: Neil Roberts <neil linux intel com>
Date:   Wed Oct 19 12:47:22 2011 +0100

    Use the same number for n_tex_coord_attribs in all linked shaders
    
    On GLES2, we need to specify an array size for the texture coord
    varying array. Previously this size would be decided in one of the
    following ways:
    
    - For generated vertex shaders it is always the number of layers in
      the pipeline.
    
    - For generated fragment shaders it is the highest sampled texture
      unit in the pipeline or the number of attributes supplied by the
      primitive, whichever is higher.
    
    - For user shaders it is usually the number of attributes supplied by
      the primitive. However, if the application tries to compile the
      shader and query the result before using it, it will always be at
      least 4.
    
    These shaders can quite easily end up with different values for the
    declaration which makes it fail to link. This patch changes it so that
    all of the shaders are generated with the maximum of the number of
    texture attributes supplied by the primitive and the number of layers
    in the pipeline. If this value changes then the shaders are
    regenerated, including user shaders. That way all of the shaders will
    always have the same value.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=662184
    
    Reviewed-by: Robert Bragg <robert linux intel com>

 cogl/cogl-pipeline-fragend-glsl.c  |   30 +++++++++++++++++-------------
 cogl/cogl-pipeline-opengl.c        |   13 ++++++++++++-
 cogl/cogl-pipeline-private.h       |    3 ++-
 cogl/cogl-pipeline-progend-glsl.c  |   28 ++++++----------------------
 cogl/cogl-pipeline-vertend-fixed.c |    3 ++-
 cogl/cogl-pipeline-vertend-glsl.c  |   27 +++++++++++++++++++--------
 cogl/cogl-shader.c                 |   10 ++++++----
 7 files changed, 64 insertions(+), 50 deletions(-)
---
diff --git a/cogl/cogl-pipeline-fragend-glsl.c b/cogl/cogl-pipeline-fragend-glsl.c
index 4728236..488070b 100644
--- a/cogl/cogl-pipeline-fragend-glsl.c
+++ b/cogl/cogl-pipeline-fragend-glsl.c
@@ -103,6 +103,11 @@ typedef struct
      program changes then we may need to redecide whether to generate
      a shader at all */
   unsigned int user_program_age;
+
+  /* The number of tex coord attributes that the shader was generated
+     for. If this changes on GLES2 then we need to regenerate the
+     shader */
+  int n_tex_coord_attribs;
 } CoglPipelineShaderState;
 
 static CoglUserDataKey shader_state_key;
@@ -268,9 +273,14 @@ _cogl_pipeline_fragend_glsl_start (CoglPipeline *pipeline,
     {
       /* If we already have a valid GLSL shader then we don't need to
          generate a new one. However if there's a user program and it
-         has changed since the last link then we do need a new shader */
-      if (user_program == NULL ||
-          shader_state->user_program_age == user_program->age)
+         has changed since the last link then we do need a new
+         shader. If the number of tex coord attribs changes on GLES2
+         then we need to regenerate the shader with a different boiler
+         plate */
+      if ((user_program == NULL ||
+           shader_state->user_program_age == user_program->age)
+          && (ctx->driver != COGL_DRIVER_GLES2 ||
+              shader_state->n_tex_coord_attribs == n_tex_coord_attribs))
         return TRUE;
 
       /* We need to recreate the shader so destroy the existing one */
@@ -285,6 +295,8 @@ _cogl_pipeline_fragend_glsl_start (CoglPipeline *pipeline,
   if (user_program)
     shader_state->user_program_age = user_program->age;
 
+  shader_state->n_tex_coord_attribs = n_tex_coord_attribs;
+
   /* If the user program contains a fragment shader then we don't need
      to generate one */
   if (user_program &&
@@ -873,8 +885,6 @@ _cogl_pipeline_fragend_glsl_end (CoglPipeline *pipeline,
       GLint lengths[2];
       GLint compile_status;
       GLuint shader;
-      int n_tex_coord_attribs = 0;
-      int i, n_layers;
 
       COGL_STATIC_COUNTER (fragend_glsl_compile_counter,
                            "glsl fragment compile counter",
@@ -921,15 +931,9 @@ _cogl_pipeline_fragend_glsl_end (CoglPipeline *pipeline,
       lengths[1] = shader_state->source->len;
       source_strings[1] = shader_state->source->str;
 
-      /* Find the highest texture unit that is sampled to pass as the
-         number of texture coordinate attributes */
-      n_layers = cogl_pipeline_get_n_layers (pipeline);
-      for (i = 0; i < n_layers; i++)
-        if (shader_state->unit_state[i].sampled)
-          n_tex_coord_attribs = i + 1;
-
       _cogl_shader_set_source_with_boilerplate (shader, GL_FRAGMENT_SHADER,
-                                                n_tex_coord_attribs,
+                                                shader_state
+                                                ->n_tex_coord_attribs,
                                                 2, /* count */
                                                 source_strings, lengths);
 
diff --git a/cogl/cogl-pipeline-opengl.c b/cogl/cogl-pipeline-opengl.c
index cc9ca1b..92d75ea 100644
--- a/cogl/cogl-pipeline-opengl.c
+++ b/cogl/cogl-pipeline-opengl.c
@@ -1200,6 +1200,16 @@ _cogl_pipeline_flush_gl_state (CoglPipeline *pipeline,
   else
     layer_differences = NULL;
 
+  /* Make sure we generate the texture coordinate array to be at least
+     the number of layers. This is important because the vertend will
+     try to pass along the corresponding varying for each layer
+     regardless of whether the fragment shader is actually using
+     it. Also it is possible that the application is assuming that if
+     the attribute isn't passed then it will default to 0,0. This is
+     what test-cogl-primitive does */
+  if (n_layers > n_tex_coord_attribs)
+    n_tex_coord_attribs = n_layers;
+
   /* First flush everything that's the same regardless of which
    * pipeline backend is being used...
    *
@@ -1300,7 +1310,8 @@ _cogl_pipeline_flush_gl_state (CoglPipeline *pipeline,
        * scratch buffers here... */
       if (G_UNLIKELY (!vertend->start (pipeline,
                                        n_layers,
-                                       pipelines_difference)))
+                                       pipelines_difference,
+                                       n_tex_coord_attribs)))
         continue;
 
       state.vertend = vertend;
diff --git a/cogl/cogl-pipeline-private.h b/cogl/cogl-pipeline-private.h
index b8965c2..57f3503 100644
--- a/cogl/cogl-pipeline-private.h
+++ b/cogl/cogl-pipeline-private.h
@@ -542,7 +542,8 @@ typedef struct _CoglPipelineVertend
 {
   gboolean (*start) (CoglPipeline *pipeline,
                      int n_layers,
-                     unsigned long pipelines_difference);
+                     unsigned long pipelines_difference,
+                     int n_tex_coord_attribs);
   gboolean (*add_layer) (CoglPipeline *pipeline,
                          CoglPipelineLayer *layer,
                          unsigned long layers_difference);
diff --git a/cogl/cogl-pipeline-progend-glsl.c b/cogl/cogl-pipeline-progend-glsl.c
index c82bc76..0d83ed6 100644
--- a/cogl/cogl-pipeline-progend-glsl.c
+++ b/cogl/cogl-pipeline-progend-glsl.c
@@ -106,7 +106,7 @@ typedef struct
    * array of texture coordinate varyings, but to know how to emit the
    * declaration we need to know how many texture coordinate
    * attributes are in use.  The boilerplate also needs to be changed
-   * if this increases. */
+   * if this changes. */
   int n_tex_coord_attribs;
 
 #ifdef HAVE_COGL_GLES2
@@ -620,11 +620,12 @@ _cogl_pipeline_progend_glsl_end (CoglPipeline *pipeline,
    * need to relink
    *
    * Also if the number of texture coordinate attributes in use has
-   * increased, then delete the program so we can prepend a new
+   * changed, then delete the program so we can prepend a new
    * _cogl_tex_coord[] varying array declaration. */
-  if (program_state->program && user_program &&
-      (user_program->age != program_state->user_program_age ||
-       n_tex_coord_attribs > program_state->n_tex_coord_attribs))
+  if ((program_state->program && user_program &&
+       user_program->age != program_state->user_program_age) ||
+      (ctx->driver == COGL_DRIVER_GLES2 &&
+       n_tex_coord_attribs != program_state->n_tex_coord_attribs))
     {
       GE( ctx, glDeleteProgram (program_state->program) );
       program_state->program = 0;
@@ -640,23 +641,6 @@ _cogl_pipeline_progend_glsl_end (CoglPipeline *pipeline,
       /* Attach all of the shader from the user program */
       if (user_program)
         {
-          if (program_state->n_tex_coord_attribs > n_tex_coord_attribs)
-            n_tex_coord_attribs = program_state->n_tex_coord_attribs;
-
-#ifdef HAVE_COGL_GLES2
-          /* Find the largest count of texture coordinate attributes
-           * associated with each of the shaders so we can ensure a consistent
-           * _cogl_tex_coord[] array declaration across all of the shaders.*/
-          if (ctx->driver == COGL_DRIVER_GLES2 &&
-              user_program)
-            for (l = user_program->attached_shaders; l; l = l->next)
-              {
-                CoglShader *shader = l->data;
-                n_tex_coord_attribs = MAX (shader->n_tex_coord_attribs,
-                                           n_tex_coord_attribs);
-              }
-#endif
-
           for (l = user_program->attached_shaders; l; l = l->next)
             {
               CoglShader *shader = l->data;
diff --git a/cogl/cogl-pipeline-vertend-fixed.c b/cogl/cogl-pipeline-vertend-fixed.c
index 3275a06..477b2af 100644
--- a/cogl/cogl-pipeline-vertend-fixed.c
+++ b/cogl/cogl-pipeline-vertend-fixed.c
@@ -46,7 +46,8 @@ const CoglPipelineVertend _cogl_pipeline_fixed_vertend;
 static gboolean
 _cogl_pipeline_vertend_fixed_start (CoglPipeline *pipeline,
                                     int n_layers,
-                                    unsigned long pipelines_difference)
+                                    unsigned long pipelines_difference,
+                                    int n_tex_coord_attribs)
 {
   CoglProgram *user_program;
 
diff --git a/cogl/cogl-pipeline-vertend-glsl.c b/cogl/cogl-pipeline-vertend-glsl.c
index a3a460d..3465573 100644
--- a/cogl/cogl-pipeline-vertend-glsl.c
+++ b/cogl/cogl-pipeline-vertend-glsl.c
@@ -56,6 +56,11 @@ typedef struct
      program changes then we may need to redecide whether to generate
      a shader at all */
   unsigned int user_program_age;
+
+  /* The number of tex coord attributes that the shader was generated
+     for. If this changes on GLES2 then we need to regenerate the
+     shader */
+  int n_tex_coord_attribs;
 } CoglPipelineShaderState;
 
 static CoglUserDataKey shader_state_key;
@@ -126,7 +131,8 @@ _cogl_pipeline_vertend_glsl_get_shader (CoglPipeline *pipeline)
 static gboolean
 _cogl_pipeline_vertend_glsl_start (CoglPipeline *pipeline,
                                    int n_layers,
-                                   unsigned long pipelines_difference)
+                                   unsigned long pipelines_difference,
+                                   int n_tex_coord_attribs)
 {
   CoglPipelineShaderState *shader_state;
   CoglPipeline *template_pipeline = NULL;
@@ -203,9 +209,14 @@ _cogl_pipeline_vertend_glsl_start (CoglPipeline *pipeline,
     {
       /* If we already have a valid GLSL shader then we don't need to
          generate a new one. However if there's a user program and it
-         has changed since the last link then we do need a new shader */
-      if (user_program == NULL ||
-          shader_state->user_program_age == user_program->age)
+         has changed since the last link then we do need a new
+         shader. If the number of tex coord attribs changes on GLES2
+         then we need to regenerate the shader with a different boiler
+         plate */
+      if ((user_program == NULL ||
+           shader_state->user_program_age == user_program->age)
+          && (ctx->driver != COGL_DRIVER_GLES2 ||
+              shader_state->n_tex_coord_attribs == n_tex_coord_attribs))
         return TRUE;
 
       /* We need to recreate the shader so destroy the existing one */
@@ -220,6 +231,8 @@ _cogl_pipeline_vertend_glsl_start (CoglPipeline *pipeline,
   if (user_program)
     shader_state->user_program_age = user_program->age;
 
+  shader_state->n_tex_coord_attribs = n_tex_coord_attribs;
+
   /* If the user program contains a vertex shader then we don't need
      to generate one */
   if (user_program &&
@@ -336,7 +349,6 @@ _cogl_pipeline_vertend_glsl_end (CoglPipeline *pipeline,
       GLint lengths[2];
       GLint compile_status;
       GLuint shader;
-      int n_layers;
 
       COGL_STATIC_COUNTER (vertend_glsl_compile_counter,
                            "glsl vertex compile counter",
@@ -359,10 +371,9 @@ _cogl_pipeline_vertend_glsl_end (CoglPipeline *pipeline,
       lengths[1] = shader_state->source->len;
       source_strings[1] = shader_state->source->str;
 
-      n_layers = cogl_pipeline_get_n_layers (pipeline);
-
       _cogl_shader_set_source_with_boilerplate (shader, GL_VERTEX_SHADER,
-                                                n_layers,
+                                                shader_state
+                                                ->n_tex_coord_attribs,
                                                 2, /* count */
                                                 source_strings, lengths);
 
diff --git a/cogl/cogl-shader.c b/cogl/cogl-shader.c
index b94f23d..72d9541 100644
--- a/cogl/cogl-shader.c
+++ b/cogl/cogl-shader.c
@@ -342,7 +342,7 @@ _cogl_shader_compile_real (CoglHandle handle,
 #ifdef HAVE_COGL_GLES2
           &&
           (ctx->driver != COGL_DRIVER_GLES2 ||
-           shader->n_tex_coord_attribs >= n_tex_coord_attribs)
+           shader->n_tex_coord_attribs == n_tex_coord_attribs)
 #endif
          )
         return;
@@ -424,9 +424,11 @@ cogl_shader_get_info_log (CoglHandle handle)
        * Here we force an early compile if the user is interested in
        * log information to increase the chance that the log will be
        * useful! We have to guess the number of texture coordinate
-       * attributes that may be used (normally less than 4) since that
-       * affects the boilerplate.
-       */
+       * attributes that may be used since that affects the
+       * boilerplate. We use four so that the shader will still
+       * compile if the user is using more than one
+       * layer. Unfortunately this is likely to end up causing it to
+       * be compiled again when we know the actual number of layers */
       if (!shader->gl_handle)
         _cogl_shader_compile_real (handle, 4);