cogl: Remove the generated array size for cogl_tex_coord_in

Under GLES2 we were defining the cogl_tex_coord_in varying as an array
with a size determined by the number of texture coordinate arrays
enabled whenever the program is used. This meant that we may have to
regenerate the shader with a different size if the shader is used with
more texture coord arrays later. However in OpenGL the equivalent
builtin varying gl_TexCoord is simply defined as:

varying vec4 gl_TexCoord[]; /* <-- no size */

GLSL is documented that if you declare an array with no size then you
can only access it with a constant index and the size of the array
will be determined by the highest index used. If you want to access it
with a non-constant expression you need to redeclare the array
yourself with a size.

We can replicate the same behaviour in our Cogl shaders by instead
declaring the cogl_tex_coord_in with no size. That way we don't have
to pass around the number of tex coord attributes enabled when we
flush a material. It also means that CoglShader can go back to
directly uploading the source string to GL when cogl_shader_source is
called so that we don't have to keep a copy of it around.

If the user wants to access cogl_tex_coord_in with a non-constant
index then they can simply redeclare the array themself. Hopefully
developers will expect to have to do this if they are accustomed to
the gl_TexCoord array.

cogl/cogl-context.c

@@ -262,7 +262,7 @@ cogl_create_context (void)
                                 default_texture_data);
 
   cogl_push_source (_context->opaque_color_pipeline);
-  _cogl_pipeline_flush_gl_state (_context->opaque_color_pipeline, FALSE, 0);
+  _cogl_pipeline_flush_gl_state (_context->opaque_color_pipeline, FALSE);
   _cogl_enable (enable_flags);
   _cogl_flush_face_winding ();
 

cogl/cogl-pipeline-arbfp.c

@@ -184,8 +184,7 @@ get_arbfp_program_state (CoglPipeline *pipeline)
 static gboolean
 _cogl_pipeline_backend_arbfp_start (CoglPipeline *pipeline,
                                     int n_layers,
-                                    unsigned long pipelines_difference,
+                                    unsigned long pipelines_difference)
-                                    int n_tex_coord_attribs)
 {
   CoglPipelineBackendARBfpPrivate *priv;
   CoglPipeline *authority;

cogl/cogl-pipeline-fixed.c

@@ -69,8 +69,7 @@ _cogl_pipeline_backend_fixed_get_max_texture_units (void)
 static gboolean
 _cogl_pipeline_backend_fixed_start (CoglPipeline *pipeline,
                                     int n_layers,
-                                    unsigned long pipelines_difference,
+                                    unsigned long pipelines_difference)
-                                    int n_tex_coord_attribs)
 {
   CoglHandle user_program;
 

cogl/cogl-pipeline-glsl.c

@@ -102,15 +102,6 @@ typedef struct _GlslProgramState
   GString *header, *source;
   UnitState *unit_state;
 
-  /* To allow writing shaders that are portable between GLES 2 and
-   * OpenGL Cogl prepends a number of boilerplate #defines and
-   * declarations to user shaders. One of those declarations is an
-   * 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. */
-  int n_tex_coord_attribs;
-
 #ifdef HAVE_COGL_GLES2
   /* The GLES2 generated program that was generated from the user
      program. This is used to detect when the GLES2 backend generates
@@ -286,8 +277,7 @@ link_program (GLint gl_program)
 static gboolean
 _cogl_pipeline_backend_glsl_start (CoglPipeline *pipeline,
                                    int n_layers,
-                                   unsigned long pipelines_difference,
+                                   unsigned long pipelines_difference)
-                                   int n_tex_coord_attribs)
 {
   CoglPipelineBackendGlslPrivate *priv;
   CoglPipeline *authority;
@@ -320,18 +310,9 @@ _cogl_pipeline_backend_glsl_start (CoglPipeline *pipeline,
   if (priv->glsl_program_state)
     {
       /* However if the program has changed since the last link then we do
-       * need to relink
+       * 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
-       * _cogl_tex_coord[] varying array declaration. */
       if (user_program == NULL ||
-          (priv->glsl_program_state->user_program_age == user_program->age
+          (priv->glsl_program_state->user_program_age == user_program->age))
-#ifdef HAVE_COGL_GLES2
-           && (priv->glsl_program_state->n_tex_coord_attribs >=
-               n_tex_coord_attribs)
-#endif
-           ))
         return TRUE;
 
       /* Destroy the existing program. We can't just dirty the whole
@@ -382,21 +363,6 @@ _cogl_pipeline_backend_glsl_start (CoglPipeline *pipeline,
      encountered it or because the user program has changed since it
      was last linked */
 
-#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 (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
-
-  priv->glsl_program_state->n_tex_coord_attribs = n_tex_coord_attribs;
-
   /* Check whether the user program contains a fragment
      shader. Otherwise we need to generate one */
   if (user_program)
@@ -1027,9 +993,7 @@ _cogl_pipeline_backend_glsl_end (CoglPipeline *pipeline,
 
               g_assert (shader->language == COGL_SHADER_LANGUAGE_GLSL);
 
-              _cogl_shader_compile_real (shader,
+              cogl_shader_compile (shader);
-                                         glsl_program_state->
-                                         n_tex_coord_attribs);
 
               GE( glAttachShader (gl_program, shader->gl_handle) );
             }
@@ -1070,8 +1034,6 @@ _cogl_pipeline_backend_glsl_end (CoglPipeline *pipeline,
           source_strings[1] = glsl_program_state->source->str;
 
           _cogl_shader_set_source_with_boilerplate (shader, GL_FRAGMENT_SHADER,
-                                                    glsl_program_state->
-                                                    n_tex_coord_attribs,
                                                     2, /* count */
                                                     source_strings, lengths);
 

cogl/cogl-pipeline-opengl-private.h

@@ -150,8 +150,7 @@ _cogl_gl_use_program_wrapper (CoglHandle program);
 
 void
 _cogl_pipeline_flush_gl_state (CoglPipeline *pipeline,
-                               gboolean skip_gl_state,
+                               gboolean skip_gl_state);
-                               int n_tex_coord_attribs);
 
 #endif /* __COGL_PIPELINE_OPENGL_PRIVATE_H */
 

cogl/cogl-pipeline-opengl.c

@@ -1031,8 +1031,7 @@ backend_add_layer_cb (CoglPipelineLayer *layer,
  */
 void
 _cogl_pipeline_flush_gl_state (CoglPipeline *pipeline,
-                               gboolean skip_gl_color,
+                               gboolean skip_gl_color)
-                               int n_tex_coord_attribs)
 {
   unsigned long    pipelines_difference;
   int              n_layers;
@@ -1135,8 +1134,7 @@ _cogl_pipeline_flush_gl_state (CoglPipeline *pipeline,
        * scratch buffers here... */
       if (G_UNLIKELY (!backend->start (pipeline,
                                        n_layers,
-                                       pipelines_difference,
+                                       pipelines_difference)))
-                                       n_tex_coord_attribs)))
         continue;
 
       state.backend = backend;

cogl/cogl-pipeline-private.h

@@ -567,8 +567,7 @@ typedef struct _CoglPipelineBackend
 
   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);

cogl/cogl-shader-boilerplate.h

@@ -92,6 +92,7 @@
   "#endif\n" \
   "\n" \
   "varying vec4 _cogl_color;\n" \
+  "varying vec4 _cogl_tex_coord[];\n" \
   "\n" \
   "#define cogl_color_in _cogl_color\n" \
   "#define cogl_tex_coord_in _cogl_tex_coord\n" \

cogl/cogl-shader-private.h

@@ -40,24 +40,18 @@ struct _CoglShader
 {
   CoglHandleObject   _parent;
   GLuint             gl_handle;
-  int                n_tex_coord_attribs;
   CoglShaderType     type;
   CoglShaderLanguage language;
-  char              *source;
 };
 
 CoglShader *_cogl_shader_pointer_from_handle (CoglHandle handle);
 
-void
-_cogl_shader_compile_real (CoglHandle handle, int n_tex_coord_attribs);
-
 CoglShaderLanguage
 _cogl_program_get_language (CoglHandle handle);
 
 void
 _cogl_shader_set_source_with_boilerplate (GLuint shader_gl_handle,
                                           GLenum shader_gl_type,
-                                          int n_tex_coord_attribs,
                                           GLsizei count_in,
                                           const char **strings_in,
                                           const GLint *lengths_in);

cogl/cogl-shader.c

@@ -101,33 +101,52 @@ cogl_create_shader (CoglShaderType type)
   shader = g_slice_new (CoglShader);
   shader->language = COGL_SHADER_LANGUAGE_GLSL;
   shader->gl_handle = 0;
-#ifdef HAVE_COGL_GLES2
-  shader->n_tex_coord_attribs = 0;
-#endif
   shader->type = type;
 
   return _cogl_shader_handle_new (shader);
 }
 
-static void
+void
-delete_shader (CoglShader *shader)
+_cogl_shader_set_source_with_boilerplate (GLuint shader_gl_handle,
+                                          GLenum shader_gl_type,
+                                          GLsizei count_in,
+                                          const char **strings_in,
+                                          const GLint *lengths_in)
 {
-  _COGL_GET_CONTEXT (ctx, NO_RETVAL);
+  static const char vertex_boilerplate[] = _COGL_VERTEX_SHADER_BOILERPLATE;
+  static const char fragment_boilerplate[] = _COGL_FRAGMENT_SHADER_BOILERPLATE;
 
-#ifdef HAVE_COGL_GL
+  const char **strings = g_alloca (sizeof (char *) * (count_in + 2));
-  if (shader->language == COGL_SHADER_LANGUAGE_ARBFP)
+  GLint *lengths = g_alloca (sizeof (GLint) * (count_in + 2));
+  int count = 0;
+
+  GET_CONTEXT (ctx, NO_RETVAL);
+
+  if (shader_gl_type == GL_VERTEX_SHADER)
     {
-      if (shader->gl_handle)
+      strings[count] = vertex_boilerplate;
-        GE (glDeletePrograms (1, &shader->gl_handle));
+      lengths[count++] = sizeof (vertex_boilerplate) - 1;
     }
+  else if (shader_gl_type == GL_FRAGMENT_SHADER)
+    {
+      strings[count] = fragment_boilerplate;
+      lengths[count++] = sizeof (fragment_boilerplate) - 1;
+    }
+
+  memcpy (strings + count, strings_in, sizeof (char *) * count_in);
+  if (lengths_in)
+    memcpy (lengths + count, lengths_in, sizeof (GLint) * count_in);
   else
-#endif
     {
-      if (shader->gl_handle)
+      int i;
-        GE (glDeleteShader (shader->gl_handle));
-    }
 
-  shader->gl_handle = 0;
+      for (i = 0; i < count_in; i++)
+        lengths[count + i] = -1; /* null terminated */
+    }
+  count += count_in;
+
+  GE( glShaderSource (shader_gl_handle, count,
+                      (const char **) strings, lengths) );
 }
 
 void
@@ -152,120 +171,31 @@ cogl_shader_source (CoglHandle   handle,
     language = COGL_SHADER_LANGUAGE_GLSL;
 
   /* Delete the old object if the language is changing... */
-  if (G_UNLIKELY (language != shader->language) &&
+  if (G_UNLIKELY (language != shader->language))
-      shader->gl_handle)
+    {
-    delete_shader (shader);
-
-  shader->source = g_strdup (source);
-
-  shader->language = language;
-}
-
-void
-cogl_shader_compile (CoglHandle handle)
-{
 #ifdef HAVE_COGL_GL
-  CoglShader *shader = handle;
+      if (shader->language == COGL_SHADER_LANGUAGE_ARBFP)
+        {
+          if (shader->gl_handle)
+            GE (glDeletePrograms (1, &shader->gl_handle));
+        }
+      else
 #endif
-
+        {
-  if (!cogl_is_shader (handle))
+          if (shader->gl_handle)
-    return;
+            GE (glDeleteShader (shader->gl_handle));
+        }
+    }
 
 #ifdef HAVE_COGL_GL
-  _cogl_shader_compile_real (shader, 0 /* ignored */);
+  if (language == COGL_SHADER_LANGUAGE_ARBFP)
-#endif
-
-  /* XXX: For GLES2 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
-   * so thinly wrap the OpenGL shader API and the current plan is to
-   * replace it with a pipeline snippets API. */
-}
-
-void
-_cogl_shader_set_source_with_boilerplate (GLuint shader_gl_handle,
-                                          GLenum shader_gl_type,
-                                          int n_tex_coord_attribs,
-                                          GLsizei count_in,
-                                          const char **strings_in,
-                                          const GLint *lengths_in)
-{
-  static const char vertex_boilerplate[] = _COGL_VERTEX_SHADER_BOILERPLATE;
-  static const char fragment_boilerplate[] = _COGL_FRAGMENT_SHADER_BOILERPLATE;
-
-  const char **strings = g_alloca (sizeof (char *) * (count_in + 2));
-  GLint *lengths = g_alloca (sizeof (GLint) * (count_in + 2));
-  int count = 0;
-#ifdef HAVE_COGL_GLES2
-  char *tex_coords_declaration = NULL;
-#endif
-
-  GET_CONTEXT (ctx, NO_RETVAL);
-
-  if (shader_gl_type == GL_VERTEX_SHADER)
-    {
-      strings[count] = vertex_boilerplate;
-      lengths[count++] = sizeof (vertex_boilerplate) - 1;
-    }
-  else if (shader_gl_type == GL_FRAGMENT_SHADER)
-    {
-      strings[count] = fragment_boilerplate;
-      lengths[count++] = sizeof (fragment_boilerplate) - 1;
-    }
-
-#ifdef HAVE_COGL_GLES2
-  if (n_tex_coord_attribs)
-    {
-      tex_coords_declaration =
-        g_strdup_printf ("varying vec2 _cogl_tex_coord[%d];\n",
-                         n_tex_coord_attribs);
-      strings[count] = tex_coords_declaration;
-      lengths[count++] = -1; /* null terminated */
-    }
-#endif
-
-  memcpy (strings + count, strings_in, sizeof (char *) * count_in);
-  if (lengths_in)
-    memcpy (lengths + count, lengths_in, sizeof (GLint) * count_in);
-  else
-    {
-      int i;
-
-      for (i = 0; i < count_in; i++)
-        lengths[count + i] = -1; /* null terminated */
-    }
-  count += count_in;
-
-  GE( glShaderSource (shader_gl_handle, count,
-                      (const char **) strings, lengths) );
-
-#ifdef HAVE_COGL_GLES2
-  g_free (tex_coords_declaration);
-#endif
-}
-
-void
-_cogl_shader_compile_real (CoglHandle handle,
-                           int n_tex_coord_attribs)
-{
-  CoglShader *shader = handle;
-
-  _COGL_GET_CONTEXT (ctx, NO_RETVAL);
-
-#ifdef HAVE_COGL_GL
-  if (shader->language == COGL_SHADER_LANGUAGE_ARBFP)
     {
 #ifdef COGL_GL_DEBUG
       GLenum gl_error;
 #endif
 
-      if (shader->gl_handle)
+      if (shader->gl_handle == 0)
-        return;
+        GE (glGenPrograms (1, &shader->gl_handle));
-
-      GE (glGenPrograms (1, &shader->gl_handle));
 
       GE (glBindProgram (GL_FRAGMENT_PROGRAM_ARB, shader->gl_handle));
 
@@ -275,8 +205,8 @@ _cogl_shader_compile_real (CoglHandle handle,
 #endif
       glProgramString (GL_FRAGMENT_PROGRAM_ARB,
                        GL_PROGRAM_FORMAT_ASCII_ARB,
-                       strlen (shader->source),
+                       strlen (source),
-                       shader->source);
+                       source);
 #ifdef COGL_GL_DEBUG
       gl_error = glGetError ();
       if (gl_error != GL_NO_ERROR)
@@ -284,7 +214,7 @@ _cogl_shader_compile_real (CoglHandle handle,
           g_warning ("%s: GL error (%d): Failed to compile ARBfp:\n%s\n%s",
                      G_STRLOC,
                      gl_error,
-                     shader->source,
+                     source,
                      glGetString (GL_PROGRAM_ERROR_STRING_ARB));
         }
 #endif
@@ -294,16 +224,6 @@ _cogl_shader_compile_real (CoglHandle handle,
     {
       GLenum gl_type;
 
-      if (shader->gl_handle
-#ifdef HAVE_COGL_GLES2
-          && shader->n_tex_coord_attribs >= n_tex_coord_attribs
-#endif
-         )
-        return;
-
-      if (shader->gl_handle)
-        delete_shader (shader);
-
       switch (shader->type)
         {
         case COGL_SHADER_TYPE_VERTEX:
@@ -317,31 +237,30 @@ _cogl_shader_compile_real (CoglHandle handle,
           break;
         }
 
-      shader->gl_handle = glCreateShader (gl_type);
+      if (!shader->gl_handle)
+        shader->gl_handle = glCreateShader (gl_type);
 
       _cogl_shader_set_source_with_boilerplate (shader->gl_handle,
                                                 gl_type,
-                                                n_tex_coord_attribs,
+                                                1, &source, NULL);
-                                                1,
-                                                (const char **) &shader->source,
-                                                NULL);
-
-      GE (glCompileShader (shader->gl_handle));
-
-#ifdef HAVE_COGL_GLES2
-      shader->n_tex_coord_attribs = n_tex_coord_attribs;
-#endif
-
-#ifdef COGL_GL_DEBUG
-      if (!cogl_shader_is_compiled (handle))
-        {
-          char *log = cogl_shader_get_info_log (handle);
-          g_warning ("Failed to compile GLSL program:\nsrc:\n%s\nerror:\n%s\n",
-                     shader->source,
-                     log);
-        }
-#endif
     }
+
+  shader->language = language;
+}
+
+void
+cogl_shader_compile (CoglHandle handle)
+{
+  CoglShader *shader;
+  _COGL_GET_CONTEXT (ctx, NO_RETVAL);
+
+  if (!cogl_is_shader (handle))
+    return;
+
+  shader = _cogl_shader_pointer_from_handle (handle);
+
+  if (shader->language == COGL_SHADER_LANGUAGE_GLSL)
+    GE (glCompileShader (shader->gl_handle));
 }
 
 char *
@@ -369,21 +288,6 @@ cogl_shader_get_info_log (CoglHandle handle)
     {
       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
-       * 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.
-       */
-      if (!shader->gl_handle)
-        _cogl_shader_compile_real (handle, 4);
-
       glGetShaderInfoLog (shader->gl_handle, 511, &len, buffer);
       buffer[len] = '\0';
       return g_strdup (buffer);
@@ -426,24 +330,6 @@ cogl_shader_is_compiled (CoglHandle handle)
   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
-       * be enough until we can replace this API with the pipeline
-       * 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
-       * was a mistake for Cogl to so thinly wrap the OpenGL shader
-       * API. Eventually we plan for this whole API will be deprecated
-       * by the pipeline snippets framework.
-       */
-      if (!shader->gl_handle)
-        _cogl_shader_compile_real (handle, 4);
-
       GE (glGetShaderiv (shader->gl_handle, GL_COMPILE_STATUS, &status));
       if (status == GL_TRUE)
         return TRUE;
@@ -509,4 +395,3 @@ cogl_shader_is_compiled (CoglHandle handle)
 }
 
 #endif /* HAVE_COGL_GLES */
-

cogl/cogl-vertex-attribute.c

@@ -435,7 +435,6 @@ enable_gl_state (CoglVertexAttribute **attributes,
   gboolean skip_gl_color = FALSE;
   CoglPipeline *source;
   CoglPipeline *copy = NULL;
-  int n_tex_coord_attribs = 0;
 
   _COGL_GET_CONTEXT (ctx, COGL_INVALID_HANDLE);
 
@@ -491,7 +490,6 @@ enable_gl_state (CoglVertexAttribute **attributes,
                                  base + attribute->offset));
           _cogl_bitmask_set (&ctx->temp_bitmask,
                              attribute->texture_unit, TRUE);
-          n_tex_coord_attribs++;
           break;
         case COGL_VERTEX_ATTRIBUTE_NAME_ID_POSITION_ARRAY:
           enable_flags |= COGL_ENABLE_VERTEX_ARRAY;
@@ -580,7 +578,7 @@ enable_gl_state (CoglVertexAttribute **attributes,
       _cogl_pipeline_apply_legacy_state (source);
     }
 
-  _cogl_pipeline_flush_gl_state (source, skip_gl_color, n_tex_coord_attribs);
+  _cogl_pipeline_flush_gl_state (source, skip_gl_color);
 
   if (ctx->enable_backface_culling)
     enable_flags |= COGL_ENABLE_BACKFACE_CULLING;

cogl/cogl.c

@@ -770,20 +770,13 @@ cogl_begin_gl (void)
    * A user should instead call cogl_set_source_color4ub() before
    * cogl_begin_gl() to simplify the state flushed.
    *
-   * XXX: note defining n_tex_coord_attribs using
-   * cogl_pipeline_get_n_layers is a hack, but the problem is that
-   * n_tex_coord_attribs is usually defined when drawing a primitive
-   * which isn't happening here.
-   *
    * Maybe it would be more useful if this code did flush the
    * opaque_color_pipeline and then call into cogl-pipeline-opengl.c to then
    * restore all state for the material's backend back to default OpenGL
    * values.
    */
   pipeline = cogl_get_source ();
-  _cogl_pipeline_flush_gl_state (pipeline,
+  _cogl_pipeline_flush_gl_state (pipeline, FALSE);
-                                 FALSE,
-                                 cogl_pipeline_get_n_layers (pipeline));
 
   if (ctx->enable_backface_culling)
     enable_flags |= COGL_ENABLE_BACKFACE_CULLING;

cogl/cogl2-path.c

@@ -221,7 +221,7 @@ _cogl_path_stroke_nodes (CoglPath *path)
 
   cogl_push_source (source);
 
-  _cogl_pipeline_flush_gl_state (source, FALSE, 0);
+  _cogl_pipeline_flush_gl_state (source, FALSE);
 
   /* Disable all client texture coordinate arrays */
   _cogl_bitmask_clear_all (&ctx->temp_bitmask);
@@ -365,7 +365,7 @@ _cogl_add_path_to_stencil_buffer (CoglPath *path,
   /* Just setup a simple pipeline that doesn't use texturing... */
   cogl_push_source (ctx->stencil_pipeline);
 
-  _cogl_pipeline_flush_gl_state (ctx->stencil_pipeline, FALSE, 0);
+  _cogl_pipeline_flush_gl_state (ctx->stencil_pipeline, FALSE);
 
   _cogl_enable (enable_flags);