Adds back tex_coord array for CoglShader compatibility

This adds back compatibility for CoglShaders that reference the
cogl_tex_coord_in[] or cogl_tex_coord_out[] varyings. Unlike the
previous way this was done this patch maintains the use of layer numbers
for attributes and maintains forwards compatibility by letting shaders
alternatively access the per-layer tex_coord varyings via
cogl_tex_coord%i_in/out defines that index into the array.

cogl/cogl-context-private.h

@@ -140,6 +140,7 @@ struct _CoglContext
   CoglPipeline     *texture_pipeline; /* used for set_source_texture */
   GString          *codegen_header_buffer;
   GString          *codegen_source_buffer;
+  GString          *codegen_boilerplate_buffer;
   GList            *source_stack;
 
   int               legacy_state_set;

cogl/cogl-context.c

@@ -293,6 +293,7 @@ cogl_context_new (CoglDisplay *display,
   context->texture_pipeline = cogl_pipeline_new (context);
   context->codegen_header_buffer = g_string_new ("");
   context->codegen_source_buffer = g_string_new ("");
+  context->codegen_boilerplate_buffer = g_string_new ("");
   context->source_stack = NULL;
 
   context->legacy_state_set = 0;

cogl/cogl-glsl-shader-boilerplate.h

@@ -44,6 +44,7 @@
   _COGL_COMMON_SHADER_BOILERPLATE \
   "#define cogl_color_out _cogl_color\n" \
   "varying vec4 _cogl_color;\n" \
+  "#define cogl_tex_coord_out _cogl_tex_coord\n" \
   "#define cogl_position_out gl_Position\n" \
   "#define cogl_point_size_out gl_PointSize\n" \
   "\n" \
@@ -61,6 +62,7 @@
   "varying vec4 _cogl_color;\n" \
   "\n" \
   "#define cogl_color_in _cogl_color\n" \
+  "#define cogl_tex_coord_in _cogl_tex_coord\n" \
   "\n" \
   "#define cogl_color_out gl_FragColor\n" \
   "#define cogl_depth_out gl_FragDepth\n" \

cogl/cogl-glsl-shader-private.h

@@ -28,6 +28,7 @@ _cogl_glsl_shader_set_source_with_boilerplate (CoglContext *ctx,
                                                const char *version_string,
                                                GLuint shader_gl_handle,
                                                GLenum shader_gl_type,
+                                               CoglPipeline *pipeline,
                                                GLsizei count_in,
                                                const char **strings_in,
                                                const GLint *lengths_in);

cogl/cogl-glsl-shader.c

@@ -39,11 +39,42 @@
 
 #include <glib.h>
 
+static CoglBool
+add_layer_vertex_boilerplate_cb (CoglPipelineLayer *layer,
+                                 void *user_data)
+{
+  GString *layer_declarations = user_data;
+  int unit_index = _cogl_pipeline_layer_get_unit_index (layer);
+  g_string_append_printf (layer_declarations,
+                          "attribute vec4 cogl_tex_coord%d_in;\n"
+                          "#define cogl_texture_matrix%i cogl_texture_matrix[%i]\n"
+                          "#define cogl_tex_coord%i_out _cogl_tex_coord[%i]\n",
+                          layer->index,
+                          layer->index,
+                          unit_index,
+                          layer->index,
+                          unit_index);
+  return TRUE;
+}
+
+static CoglBool
+add_layer_fragment_boilerplate_cb (CoglPipelineLayer *layer,
+                                   void *user_data)
+{
+  GString *layer_declarations = user_data;
+  g_string_append_printf (layer_declarations,
+                          "#define cogl_tex_coord%i_in _cogl_tex_coord[%i]\n",
+                          layer->index,
+                          _cogl_pipeline_layer_get_unit_index (layer));
+  return TRUE;
+}
+
 void
 _cogl_glsl_shader_set_source_with_boilerplate (CoglContext *ctx,
                                                const char *version_string,
                                                GLuint shader_gl_handle,
                                                GLenum shader_gl_type,
+                                               CoglPipeline *pipeline,
                                                GLsizei count_in,
                                                const char **strings_in,
                                                const GLint *lengths_in)
@@ -54,7 +85,8 @@ _cogl_glsl_shader_set_source_with_boilerplate (CoglContext *ctx,
   const char **strings = g_alloca (sizeof (char *) * (count_in + 4));
   GLint *lengths = g_alloca (sizeof (GLint) * (count_in + 4));
   int count = 0;
-  char *tex_coord_declarations = NULL;
+
+  int n_layers;
 
   vertex_boilerplate = _COGL_VERTEX_SHADER_BOILERPLATE;
   fragment_boilerplate = _COGL_FRAGMENT_SHADER_BOILERPLATE;
@@ -85,6 +117,37 @@ _cogl_glsl_shader_set_source_with_boilerplate (CoglContext *ctx,
       lengths[count++] = strlen (fragment_boilerplate);
     }
 
+  n_layers = cogl_pipeline_get_n_layers (pipeline);
+  if (n_layers)
+    {
+      GString *layer_declarations = ctx->codegen_boilerplate_buffer;
+      g_string_set_size (layer_declarations, 0);
+
+      g_string_append_printf (layer_declarations,
+                              "varying vec4 _cogl_tex_coord[%d];\n",
+                              n_layers);
+
+      if (shader_gl_type == GL_VERTEX_SHADER)
+        {
+          g_string_append_printf (layer_declarations,
+                                  "uniform mat4 cogl_texture_matrix[%d];\n",
+                                  n_layers);
+
+          _cogl_pipeline_foreach_layer_internal (pipeline,
+                                                 add_layer_vertex_boilerplate_cb,
+                                                 layer_declarations);
+        }
+      else if (shader_gl_type == GL_FRAGMENT_SHADER)
+        {
+          _cogl_pipeline_foreach_layer_internal (pipeline,
+                                                 add_layer_fragment_boilerplate_cb,
+                                                 layer_declarations);
+        }
+
+      strings[count] = layer_declarations->str;
+      lengths[count++] = -1; /* null terminated */
+    }
+
   memcpy (strings + count, strings_in, sizeof (char *) * count_in);
   if (lengths_in)
     memcpy (lengths + count, lengths_in, sizeof (GLint) * count_in);
@@ -119,6 +182,4 @@ _cogl_glsl_shader_set_source_with_boilerplate (CoglContext *ctx,
 
   GE( ctx, glShaderSource (shader_gl_handle, count,
                            (const char **) strings, lengths) );
-
-  g_free (tex_coord_declarations);
 }

cogl/cogl-pipeline-private.h

@@ -929,6 +929,10 @@ CoglBool
 _cogl_pipeline_layer_numbers_equal (CoglPipeline *pipeline0,
                                     CoglPipeline *pipeline1);
 
+CoglBool
+_cogl_pipeline_layer_and_unit_numbers_equal (CoglPipeline *pipeline0,
+                                             CoglPipeline *pipeline1);
+
 CoglBool
 _cogl_pipeline_need_texture_combine_separate
                                     (CoglPipelineLayer *combine_authority);

cogl/cogl-pipeline.c

@@ -646,6 +646,41 @@ _cogl_pipeline_layer_numbers_equal (CoglPipeline *pipeline0,
   return TRUE;
 }
 
+CoglBool
+_cogl_pipeline_layer_and_unit_numbers_equal (CoglPipeline *pipeline0,
+                                             CoglPipeline *pipeline1)
+{
+  CoglPipeline *authority0 =
+    _cogl_pipeline_get_authority (pipeline0, COGL_PIPELINE_STATE_LAYERS);
+  CoglPipeline *authority1 =
+    _cogl_pipeline_get_authority (pipeline1, COGL_PIPELINE_STATE_LAYERS);
+  int n_layers = authority0->n_layers;
+  int i;
+
+  if (authority1->n_layers != n_layers)
+    return FALSE;
+
+  _cogl_pipeline_update_layers_cache (authority0);
+  _cogl_pipeline_update_layers_cache (authority1);
+
+  for (i = 0; i < n_layers; i++)
+    {
+      CoglPipelineLayer *layer0 = authority0->layers_cache[i];
+      CoglPipelineLayer *layer1 = authority1->layers_cache[i];
+      int unit0, unit1;
+
+      if (layer0->index != layer1->index)
+        return FALSE;
+
+      unit0 = _cogl_pipeline_layer_get_unit_index (layer0);
+      unit1 = _cogl_pipeline_layer_get_unit_index (layer1);
+      if (unit0 != unit1)
+        return FALSE;
+    }
+
+  return TRUE;
+}
+
 typedef struct
 {
   int i;

cogl/cogl-shader-private.h

@@ -27,6 +27,7 @@
 #include "cogl-object-private.h"
 #include "cogl-shader.h"
 #include "cogl-gl-header.h"
+#include "cogl-pipeline.h"
 
 typedef struct _CoglShader CoglShader;
 
@@ -40,14 +41,15 @@ struct _CoglShader
 {
   CoglHandleObject _parent;
   GLuint gl_handle;
-  int                n_tex_coord_attribs;
+  CoglPipeline *compilation_pipeline;
   CoglShaderType type;
   CoglShaderLanguage language;
   char *source;
 };
 
 void
-_cogl_shader_compile_real (CoglHandle handle, int n_tex_coord_attribs);
+_cogl_shader_compile_real (CoglHandle handle,
+                           CoglPipeline *pipeline);
 
 CoglShaderLanguage
 _cogl_program_get_language (CoglHandle handle);

cogl/cogl-shader.c

@@ -90,7 +90,7 @@ cogl_create_shader (CoglShaderType type)
   shader = g_slice_new (CoglShader);
   shader->language = COGL_SHADER_LANGUAGE_GLSL;
   shader->gl_handle = 0;
-  shader->n_tex_coord_attribs = 0;
+  shader->compilation_pipeline = NULL;
   shader->type = type;
 
   return _cogl_shader_handle_new (shader);
@@ -115,6 +115,12 @@ delete_shader (CoglShader *shader)
     }
 
   shader->gl_handle = 0;
+
+  if (shader->compilation_pipeline)
+    {
+      cogl_object_unref (shader->compilation_pipeline);
+      shader->compilation_pipeline = NULL;
+    }
 }
 
 void
@@ -151,14 +157,22 @@ cogl_shader_source (CoglHandle   handle,
 void
 cogl_shader_compile (CoglHandle handle)
 {
+  CoglShader *shader;
+
   _COGL_GET_CONTEXT (ctx, NO_RETVAL);
 
   if (!cogl_is_shader (handle))
     return;
 
-  /* XXX: We don't actually compile anything until the shader gets
+#ifdef HAVE_COGL_GL
-   * used so we have an opportunity to add some boilerplate to the
+  shader = handle;
-   * shader.
+  if (shader->language == COGL_SHADER_LANGUAGE_ARBFP)
+    _cogl_shader_compile_real (handle, NULL);
+#endif
+
+  /* XXX: For GLSL we don't actually compile anything until the shader
+   * gets used so we have an opportunity to add some boilerplate to
+   * the shader.
    *
    * At the end of the day this is obviously a badly designed API
    * given that we are having to lie to the user. It was a mistake to
@@ -168,7 +182,7 @@ cogl_shader_compile (CoglHandle handle)
 
 void
 _cogl_shader_compile_real (CoglHandle handle,
-                           int n_tex_coord_attribs)
+                           CoglPipeline *pipeline)
 {
   CoglShader *shader = handle;
   const char *version;
@@ -216,10 +230,20 @@ _cogl_shader_compile_real (CoglHandle handle,
 #endif
     {
       GLenum gl_type;
+      GLint status;
 
-      if (shader->gl_handle &&
+      if (shader->gl_handle)
-          shader->n_tex_coord_attribs == n_tex_coord_attribs)
+        {
+          CoglPipeline *prev = shader->compilation_pipeline;
+
+          /* XXX: currently the only things that will affect the
+           * boilerplate for user shaders, apart from driver features,
+           * are the pipeline layer-indices and texture-unit-indices
+           */
+          if (pipeline == prev ||
+              _cogl_pipeline_layer_and_unit_numbers_equal (prev, pipeline))
             return;
+        }
 
       if (shader->gl_handle)
         delete_shader (shader);
@@ -253,6 +277,7 @@ _cogl_shader_compile_real (CoglHandle handle,
                                                      version,
                                                      shader->gl_handle,
                                                      gl_type,
+                                                     pipeline,
                                                      1,
                                                      (const char **)
                                                       &shader->source,
@@ -260,67 +285,51 @@ _cogl_shader_compile_real (CoglHandle handle,
 
       GE (ctx, glCompileShader (shader->gl_handle));
 
-      shader->n_tex_coord_attribs = n_tex_coord_attribs;
+      shader->compilation_pipeline = cogl_object_ref (pipeline);
 
-#ifdef COGL_GL_DEBUG
+      GE (ctx, glGetShaderiv (shader->gl_handle, GL_COMPILE_STATUS, &status));
-      if (!cogl_shader_is_compiled (handle))
+      if (!status)
         {
-          char *log = cogl_shader_get_info_log (handle);
+          char buffer[512];
-          g_warning ("Failed to compile GLSL program:\nsrc:\n%s\nerror:\n%s\n",
+          int len = 0;
+
+          ctx->glGetShaderInfoLog (shader->gl_handle, 511, &len, buffer);
+          buffer[len] = '\0';
+
+          g_warning ("Failed to compile GLSL program:\n"
+                     "src:\n%s\n"
+                     "error:\n%s\n",
                      shader->source,
-                     log);
+                     buffer);
-          g_free (log);
         }
-#endif
     }
 }
 
 char *
 cogl_shader_get_info_log (CoglHandle handle)
 {
-  CoglShader *shader;
-
-  _COGL_GET_CONTEXT (ctx, NULL);
-
   if (!cogl_is_shader (handle))
     return NULL;
 
-  shader = handle;
+  /* XXX: This API doesn't really do anything!
-
-#ifdef HAVE_COGL_GL
-  if (shader->language == COGL_SHADER_LANGUAGE_ARBFP)
-    {
-      /* ARBfp exposes a program error string, but since cogl_program
-       * doesn't have any API to query an error log it is not currently
-       * exposed. */
-      return g_strdup ("");
-    }
-  else
-#endif
-    {
-      char buffer[512];
-      int len = 0;
-
-      /* We don't normally compile the shader when the user calls
-       * cogl_shader_compile() because we want to be able to add
-       * boilerplate code that depends on how it ends up finally being
-       * used.
    *
-       * Here we force an early compile if the user is interested in
+   * This API is purely for compatibility
-       * log information to increase the chance that the log will be
+   *
-       * useful! We have to guess the number of texture coordinate
+   * The reason we don't do anything is because a shader needs to
-       * attributes that may be used since that affects the
+   * be associated with a CoglPipeline for Cogl to be able to
-       * boilerplate. We use four so that the shader will still
+   * compile and link anything.
-       * compile if the user is using more than one
+   *
-       * layer. Unfortunately this is likely to end up causing it to
+   * The way this API was originally designed as a very thin wrapper
-       * be compiled again when we know the actual number of layers */
+   * over the GL api was a mistake and it's now very difficult to
-      if (!shader->gl_handle)
+   * make the API work in a meaningful way given how the rest of Cogl
-        _cogl_shader_compile_real (handle, 4);
+   * has evolved.
+   *
+   * The CoglShader API is mostly deprecated by CoglSnippets and so
+   * these days we do the bare minimum to support the existing users
+   * of it until they are able to migrate to the snippets api.
+   */
 
-      ctx->glGetShaderInfoLog (shader->gl_handle, 511, &len, buffer);
+  return g_strdup ("");
-      buffer[len] = '\0';
-      return g_strdup (buffer);
-    }
 }
 
 CoglShaderType
@@ -344,46 +353,29 @@ CoglBool
 cogl_shader_is_compiled (CoglHandle handle)
 {
 #if defined (HAVE_COGL_GL) || defined (HAVE_COGL_GLES2)
-  GLint status;
-  CoglShader *shader;
-
-  _COGL_GET_CONTEXT (ctx, FALSE);
-
   if (!cogl_is_shader (handle))
     return FALSE;
 
-  shader = handle;
+  /* XXX: This API doesn't really do anything!
-
-#ifdef HAVE_COGL_GL
-  if (shader->language == COGL_SHADER_LANGUAGE_ARBFP)
-    return TRUE;
-  else
-#endif
-    {
-      /* FIXME: We currently have an arbitrary limit of 4 texture
-       * coordinate attributes since our API means we have to add
-       * some boilerplate to the users GLSL program (for GLES2)
-       * before we actually know how many attributes are in use.
    *
-       * 4 will probably be enough (or at least that limitation should
+   * This API is purely for compatibility and blatantly lies to the
-       * be enough until we can replace this API with the pipeline
+   * user about whether their shader has been compiled.
-       * snippets API) but if it isn't then the shader won't compile,
-       * through no fault of the user.
    *
-       * To some extent this is just a symptom of bad API design; it
+   * I suppose we could say we're stretching the definition of
-       * was a mistake for Cogl to so thinly wrap the OpenGL shader
+   * "compile" and are deferring any related errors to be "linker"
-       * API. Eventually we plan for this whole API will be deprecated
+   * errors.
-       * by the pipeline snippets framework.
+   *
+   * The reason we don't do anything is because a shader needs to
+   * be associated with a CoglPipeline for Cogl to be able to
+   * compile and link anything.
+   *
+   * The CoglShader API is mostly deprecated by CoglSnippets and so
+   * these days we do the bare minimum to support the existing users
+   * of it until they are able to migrate to the snippets api.
    */
-      if (!shader->gl_handle)
-        _cogl_shader_compile_real (handle, 4);
 
-      GE (ctx, glGetShaderiv (shader->gl_handle, GL_COMPILE_STATUS, &status));
-      if (status == GL_TRUE)
   return TRUE;
-      else
+
-        return FALSE;
-    }
 #else
   return FALSE;
 #endif

cogl/driver/gl/cogl-pipeline-fragend-glsl.c

@@ -101,12 +101,6 @@ typedef struct
      again */
   LayerDataList layers;
 
-  /* Age of the user program that was current when the shader was
-     generated. We need to keep track of this because if the user
-     program changes then we may need to redecide whether to generate
-     a shader at all */
-  unsigned int user_program_age;
-
 } CoglPipelineShaderState;
 
 static CoglUserDataKey shader_state_key;
@@ -288,34 +282,28 @@ _cogl_pipeline_fragend_glsl_start (CoglPipeline *pipeline,
         }
     }
 
+  if (user_program)
+    {
+      /* If the user program contains a fragment shader then we don't need
+         to generate one */
+      if (_cogl_program_has_fragment_shader (user_program))
+        {
           if (shader_state->gl_shader)
             {
-      /* 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)
-        return;
-
-      /* We need to recreate the shader so destroy the existing one */
               GE( ctx, glDeleteShader (shader_state->gl_shader) );
               shader_state->gl_shader = 0;
             }
+          return;
+        }
+    }
+
+  if (shader_state->gl_shader)
+    return;
 
   /* If we make it here then we have a glsl_shader_state struct
      without a gl_shader either because this is the first time we've
      encountered it or because the user program has changed */
 
-  if (user_program)
-    {
-      shader_state->user_program_age = user_program->age;
-
-      /* If the user program contains a fragment shader then we don't need
-         to generate one */
-      if (_cogl_program_has_fragment_shader (user_program))
-        return;
-    }
-
   /* We reuse two grow-only GStrings for code-gen. One string
      contains the uniform and attribute declarations while the
      other contains the main function. We need two strings
@@ -1017,12 +1005,7 @@ _cogl_pipeline_fragend_glsl_end (CoglPipeline *pipeline,
               get_texture_target_string (texture_type, &target_string, NULL);
 
               g_string_append_printf (shader_state->header,
-                                      "varying vec4 _cogl_tex_coord%i;\n"
-                                      "#define cogl_tex_coord%i_in _cogl_tex_coord%i\n"
                                       "uniform sampler%s cogl_sampler%i;\n",
-                                      layer->index,
-                                      layer->index,
-                                      layer->index,
                                       target_string,
                                       layer->index);
             }
@@ -1077,6 +1060,7 @@ _cogl_pipeline_fragend_glsl_end (CoglPipeline *pipeline,
       _cogl_glsl_shader_set_source_with_boilerplate (ctx,
                                                      version_string,
                                                      shader, GL_FRAGMENT_SHADER,
+                                                     pipeline,
                                                      2, /* count */
                                                      source_strings, lengths);
 

cogl/driver/gl/cogl-pipeline-progend-glsl.c

@@ -377,7 +377,7 @@ get_uniform_cb (CoglPipeline *pipeline,
 
   g_string_set_size (ctx->codegen_source_buffer, 0);
   g_string_append_printf (ctx->codegen_source_buffer,
-                          "cogl_texture_matrix%i", layer_index);
+                          "cogl_texture_matrix[%i]", layer_index);
 
   GE_RET( uniform_location,
           ctx, glGetUniformLocation (state->gl_program,
@@ -725,7 +725,7 @@ _cogl_pipeline_progend_glsl_end (CoglPipeline *pipeline,
             {
               CoglShader *shader = l->data;
 
-              _cogl_shader_compile_real (shader, 4);
+              _cogl_shader_compile_real (shader, pipeline);
 
               g_assert (shader->language == COGL_SHADER_LANGUAGE_GLSL);
 

cogl/driver/gl/cogl-pipeline-vertend-glsl.c

@@ -54,12 +54,6 @@ typedef struct
   GLuint gl_shader;
   GString *header, *source;
 
-  /* Age of the user program that was current when the shader was
-     generated. We need to keep track of this because if the user
-     program changes then we may need to redecide whether to generate
-     a shader at all */
-  unsigned int user_program_age;
-
 } CoglPipelineShaderState;
 
 static CoglUserDataKey shader_state_key;
@@ -210,35 +204,28 @@ _cogl_pipeline_vertend_glsl_start (CoglPipeline *pipeline,
         }
     }
 
+  if (user_program)
+    {
+      /* If the user program contains a vertex shader then we don't need
+         to generate one */
+      if (_cogl_program_has_vertex_shader (user_program))
+        {
           if (shader_state->gl_shader)
             {
-      /* 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)
-        return;
-
-      /* We need to recreate the shader so destroy the existing one */
               GE( ctx, glDeleteShader (shader_state->gl_shader) );
               shader_state->gl_shader = 0;
             }
+          return;
+        }
+    }
+
+  if (shader_state->gl_shader)
+    return;
 
   /* If we make it here then we have a shader_state struct without a gl_shader
      either because this is the first time we've encountered it or
      because the user program has changed */
 
-  if (user_program)
-    {
-      shader_state->user_program_age = user_program->age;
-
-      /* If the user program contains a vertex shader then we don't need
-         to generate one */
-      if (_cogl_program_has_vertex_shader (user_program))
-        return;
-    }
-
   /* We reuse two grow-only GStrings for code-gen. One string
      contains the uniform and attribute declarations while the
      other contains the main function. We need two strings
@@ -279,17 +266,6 @@ _cogl_pipeline_vertend_glsl_add_layer (CoglPipeline *pipeline,
   if (shader_state->source == NULL)
     return TRUE;
 
-  g_string_append_printf (shader_state->header,
-                          "uniform mat4 cogl_texture_matrix%i;\n"
-                          "attribute vec4 cogl_tex_coord%i_in;\n"
-                          "varying vec4 _cogl_tex_coord%i;\n"
-                          "#define cogl_tex_coord%i_out _cogl_tex_coord%i\n",
-                          layer_index,
-                          layer_index,
-                          layer_index,
-                          layer_index,
-                          layer_index);
-
   /* Transform the texture coordinates by the layer's user matrix.
    *
    * FIXME: this should avoid doing the transform if there is no user
@@ -439,6 +415,7 @@ _cogl_pipeline_vertend_glsl_end (CoglPipeline *pipeline,
       _cogl_glsl_shader_set_source_with_boilerplate (ctx,
                                                      NULL,
                                                      shader, GL_VERTEX_SHADER,
+                                                     pipeline,
                                                      2, /* count */
                                                      source_strings, lengths);