cogl-pipeline: Move texture enabling/disabling to fixed fragend

Previously enabling and disabling textures was done whatever the
backend in cogl-pipeline-opengl. However enabling and disabling
texture targets only has any meaning if no fragment shaders are being
used so this patch moves the code to cogl-pipeline-fragend-fixed.

The GLES2 wrapper has also been changed to ignore enabledness when
deciding whether to update texture coordinate attribute pointers.

clutter/cogl/cogl/cogl-pipeline-fragend-arbfp.c

@@ -173,12 +173,6 @@ arbfp_program_state_unref (ArbfpProgramState *state)
     }
 }
 
-static int
-_cogl_pipeline_fragend_arbfp_get_max_texture_units (void)
-{
-  return _cogl_get_max_texture_image_units ();
-}
-
 static CoglPipelineFragendARBfpPrivate *
 get_arbfp_priv (CoglPipeline *pipeline)
 {
@@ -1118,7 +1112,6 @@ _cogl_pipeline_fragend_arbfp_free_priv (CoglPipeline *pipeline)
 
 const CoglPipelineFragend _cogl_pipeline_arbfp_fragend =
 {
-  _cogl_pipeline_fragend_arbfp_get_max_texture_units,
   _cogl_pipeline_fragend_arbfp_start,
   _cogl_pipeline_fragend_arbfp_add_layer,
   _cogl_pipeline_fragend_arbfp_passthrough,

clutter/cogl/cogl/cogl-pipeline-fragend-fixed.c

@@ -49,8 +49,25 @@
 
 const CoglPipelineFragend _cogl_pipeline_fixed_fragend;
 
+static void
+_cogl_disable_texture_unit (int unit_index)
+{
+  CoglTextureUnit *unit;
+
+  _COGL_GET_CONTEXT (ctx, NO_RETVAL);
+
+  unit = &g_array_index (ctx->texture_units, CoglTextureUnit, unit_index);
+
+  if (unit->enabled_gl_target)
+    {
+      _cogl_set_active_texture_unit (unit_index);
+      GE (glDisable (unit->enabled_gl_target));
+      unit->enabled_gl_target = 0;
+    }
+}
+
 static int
-_cogl_pipeline_fragend_fixed_get_max_texture_units (void)
+get_max_texture_units (void)
 {
   _COGL_GET_CONTEXT (ctx, 0);
 
@@ -106,6 +123,68 @@ _cogl_pipeline_fragend_fixed_add_layer (CoglPipeline *pipeline,
    */
   _cogl_set_active_texture_unit (unit_index);
 
+  if (G_UNLIKELY (unit_index >= get_max_texture_units ()))
+    {
+      _cogl_disable_texture_unit (unit_index);
+      /* TODO: although this isn't considered an error that
+       * warrants falling back to a different backend we
+       * should print a warning here. */
+      return TRUE;
+    }
+
+  /* Handle enabling or disabling the right texture target */
+  if (layers_difference & COGL_PIPELINE_LAYER_STATE_TEXTURE)
+    {
+      CoglPipelineLayer *authority =
+        _cogl_pipeline_layer_get_authority (layer,
+                                            COGL_PIPELINE_LAYER_STATE_TEXTURE);
+      CoglHandle texture;
+      GLuint     gl_texture;
+      GLenum     gl_target;
+
+      texture = (authority->texture == COGL_INVALID_HANDLE ?
+                 ctx->default_gl_texture_2d_tex :
+                 authority->texture);
+
+      cogl_texture_get_gl_texture (texture,
+                                   &gl_texture,
+                                   &gl_target);
+
+      _cogl_set_active_texture_unit (unit_index);
+
+      /* The common GL code handles binding the right texture so we
+         just need to handle enabling and disabling it */
+
+      if (unit->enabled_gl_target != gl_target)
+        {
+          /* Disable the previous target if it's still enabled */
+          if (unit->enabled_gl_target)
+            GE (glDisable (unit->enabled_gl_target));
+
+          /* Enable the new target */
+          if (!G_UNLIKELY (cogl_debug_flags & COGL_DEBUG_DISABLE_TEXTURING))
+            {
+              GE (glEnable (gl_target));
+              unit->enabled_gl_target = gl_target;
+            }
+        }
+    }
+  else
+    {
+      /* Even though there may be no difference between the last flushed
+       * texture state and the current layers texture state it may be that the
+       * texture unit has been disabled for some time so we need to assert that
+       * it's enabled now.
+       */
+      if (!G_UNLIKELY (cogl_debug_flags & COGL_DEBUG_DISABLE_TEXTURING) &&
+          !unit->enabled_gl_target == 0)
+        {
+          _cogl_set_active_texture_unit (unit_index);
+          GE (glEnable (unit->gl_target));
+          unit->enabled_gl_target = unit->gl_target;
+        }
+    }
+
   if (layers_difference & COGL_PIPELINE_LAYER_STATE_COMBINE)
     {
       CoglPipelineLayer *authority =
@@ -188,10 +267,35 @@ _cogl_pipeline_fragend_fixed_add_layer (CoglPipeline *pipeline,
   return TRUE;
 }
 
+static gboolean
+get_highest_unit_index_cb (CoglPipelineLayer *layer,
+                           void *user_data)
+{
+  int unit_index = _cogl_pipeline_layer_get_unit_index (layer);
+  int *highest_index = user_data;
+
+  *highest_index = unit_index;
+
+  return TRUE;
+}
+
 static gboolean
 _cogl_pipeline_fragend_fixed_end (CoglPipeline *pipeline,
                                   unsigned long pipelines_difference)
 {
+  int highest_unit_index = -1;
+  int i;
+
+  _COGL_GET_CONTEXT (ctx, FALSE);
+
+  _cogl_pipeline_foreach_layer_internal (pipeline,
+                                         get_highest_unit_index_cb,
+                                         &highest_unit_index);
+
+  /* Disable additional texture units that may have previously been in use.. */
+  for (i = highest_unit_index + 1; i < ctx->texture_units->len; i++)
+    _cogl_disable_texture_unit (i);
+
   if (pipelines_difference & COGL_PIPELINE_STATE_FOG)
     {
       CoglPipeline *authority =
@@ -245,7 +349,6 @@ _cogl_pipeline_fragend_fixed_end (CoglPipeline *pipeline,
 
 const CoglPipelineFragend _cogl_pipeline_fixed_fragend =
 {
-  _cogl_pipeline_fragend_fixed_get_max_texture_units,
   _cogl_pipeline_fragend_fixed_start,
   _cogl_pipeline_fragend_fixed_add_layer,
   NULL, /* passthrough */

clutter/cogl/cogl/cogl-pipeline-fragend-glsl.c

@@ -138,12 +138,6 @@ typedef struct _CoglPipelineFragendGlslPrivate
 
 const CoglPipelineFragend _cogl_pipeline_glsl_backend;
 
-static int
-_cogl_pipeline_fragend_glsl_get_max_texture_units (void)
-{
-  return _cogl_get_max_texture_image_units ();
-}
-
 static GlslProgramState *
 glsl_program_state_new (int n_layers)
 {
@@ -1248,7 +1242,6 @@ _cogl_pipeline_fragend_glsl_free_priv (CoglPipeline *pipeline)
 
 const CoglPipelineFragend _cogl_pipeline_glsl_fragend =
 {
-  _cogl_pipeline_fragend_glsl_get_max_texture_units,
   _cogl_pipeline_fragend_glsl_start,
   _cogl_pipeline_fragend_glsl_add_layer,
   _cogl_pipeline_fragend_glsl_passthrough,

clutter/cogl/cogl/cogl-pipeline-opengl-private.h

@@ -54,17 +54,18 @@ typedef struct _CoglTextureUnit
    * glActiveTexture () */
   int                index;
 
-  /* Whether or not the corresponding gl_target has been glEnabled */
+  /* The GL target currently glEnabled or 0 if nothing is
-  gboolean           enabled;
+   * enabled. This is only used by the fixed pipeline fragend */
-
+  GLenum             enabled_gl_target;
-  /* The GL target currently glEnabled or the target last enabled
-   * if .enabled == FALSE */
-  GLenum             current_gl_target;
 
   /* The raw GL texture object name for which we called glBindTexture when
    * we flushed the last layer. (NB: The CoglTexture associated
    * with a layer may represent more than one GL texture) */
   GLuint             gl_texture;
+  /* The target of the GL texture object. This is just used so that we
+   * can quickly determine the intended target to flush when
+   * dirty_gl_texture == TRUE */
+  GLenum             gl_target;
 
   /* Foreign textures are those not created or deleted by Cogl. If we ever
    * call glBindTexture for a foreign texture then the next time we are
@@ -131,9 +132,6 @@ _cogl_get_texture_unit (int index_);
 void
 _cogl_destroy_texture_units (void);
 
-void
-_cogl_disable_texture_unit (int unit_index);
-
 void
 _cogl_set_active_texture_unit (int unit_index);
 

clutter/cogl/cogl/cogl-pipeline-opengl.c

@@ -80,9 +80,9 @@ static void
 texture_unit_init (CoglTextureUnit *unit, int index_)
 {
   unit->index = index_;
-  unit->enabled = FALSE;
+  unit->enabled_gl_target = 0;
-  unit->current_gl_target = 0;
   unit->gl_texture = 0;
+  unit->gl_target = 0;
   unit->is_foreign = FALSE;
   unit->dirty_gl_texture = FALSE;
   unit->matrix_stack = _cogl_matrix_stack_new ();
@@ -150,23 +150,6 @@ _cogl_set_active_texture_unit (int unit_index)
     }
 }
 
-void
-_cogl_disable_texture_unit (int unit_index)
-{
-  CoglTextureUnit *unit;
-
-  _COGL_GET_CONTEXT (ctx, NO_RETVAL);
-
-  unit = &g_array_index (ctx->texture_units, CoglTextureUnit, unit_index);
-
-  if (unit->enabled)
-    {
-      _cogl_set_active_texture_unit (unit_index);
-      GE (glDisable (unit->current_gl_target));
-      unit->enabled = FALSE;
-    }
-}
-
 /* Note: _cogl_bind_gl_texture_transient conceptually has slightly
  * different semantics to OpenGL's glBindTexture because Cogl never
  * cares about tracking multiple textures bound to different targets
@@ -235,6 +218,7 @@ _cogl_delete_gl_texture (GLuint gl_texture)
       if (unit->gl_texture == gl_texture)
         {
           unit->gl_texture = 0;
+          unit->gl_target = 0;
           unit->dirty_gl_texture = FALSE;
         }
     }
@@ -682,44 +666,17 @@ flush_layers_common_gl_state_cb (CoglPipelineLayer *layer, void *user_data)
           else
             GE (glBindTexture (gl_target, gl_texture));
           unit->gl_texture = gl_texture;
+          unit->gl_target = gl_target;
         }
 
       unit->is_foreign = _cogl_texture_is_foreign (texture);
 
-      /* Disable the previous target if it was different and it's
-       * still enabled */
-      if (unit->enabled && unit->current_gl_target != gl_target)
-        GE (glDisable (unit->current_gl_target));
-
-      if (!G_UNLIKELY (cogl_debug_flags & COGL_DEBUG_DISABLE_TEXTURING) &&
-          (!unit->enabled || unit->current_gl_target != gl_target))
-        {
-          GE (glEnable (gl_target));
-          unit->enabled = TRUE;
-          unit->current_gl_target = gl_target;
-        }
-
       /* The texture_storage_changed boolean indicates if the
        * CoglTexture's underlying GL texture storage has changed since
        * it was flushed to the texture unit. We've just flushed the
        * latest state so we can reset this. */
       unit->texture_storage_changed = FALSE;
     }
-  else
-    {
-      /* Even though there may be no difference between the last flushed
-       * texture state and the current layers texture state it may be that the
-       * texture unit has been disabled for some time so we need to assert that
-       * it's enabled now.
-       */
-      if (!G_UNLIKELY (cogl_debug_flags & COGL_DEBUG_DISABLE_TEXTURING) &&
-          !unit->enabled)
-        {
-          _cogl_set_active_texture_unit (unit_index);
-          GE (glEnable (unit->current_gl_target));
-          unit->enabled = TRUE;
-        }
-    }
 
   if (layers_difference & COGL_PIPELINE_LAYER_STATE_USER_MATRIX)
     {
@@ -781,10 +738,6 @@ _cogl_pipeline_flush_common_gl_state (CoglPipeline  *pipeline,
   _cogl_pipeline_foreach_layer_internal (pipeline,
                                          flush_layers_common_gl_state_cb,
                                          &state);
-
-  /* Disable additional texture units that may have previously been in use.. */
-  for (; state.i < ctx->texture_units->len; state.i++)
-    _cogl_disable_texture_unit (state.i);
 }
 
 /* Re-assert the layer's wrap modes on the given CoglTexture.
@@ -859,19 +812,20 @@ foreach_texture_unit_update_filter_and_wrap_modes (void)
       CoglTextureUnit *unit =
         &g_array_index (ctx->texture_units, CoglTextureUnit, i);
 
-      if (!unit->enabled)
-        break;
-
       if (unit->layer)
         {
           CoglHandle texture = _cogl_pipeline_layer_get_texture (unit->layer);
-          CoglPipelineFilter min;
-          CoglPipelineFilter mag;
 
-          _cogl_pipeline_layer_get_filters (unit->layer, &min, &mag);
+          if (texture != COGL_INVALID_HANDLE)
-          _cogl_texture_set_filters (texture, min, mag);
+            {
+              CoglPipelineFilter min;
+              CoglPipelineFilter mag;
 
-          _cogl_pipeline_layer_forward_wrap_modes (unit->layer, texture);
+              _cogl_pipeline_layer_get_filters (unit->layer, &min, &mag);
+              _cogl_texture_set_filters (texture, min, mag);
+
+              _cogl_pipeline_layer_forward_wrap_modes (unit->layer, texture);
+            }
         }
     }
 }
@@ -933,27 +887,9 @@ fragend_add_layer_cb (CoglPipelineLayer *layer,
   const CoglPipelineFragend *fragend = state->fragend;
   CoglPipeline *pipeline = state->pipeline;
   int unit_index = _cogl_pipeline_layer_get_unit_index (layer);
-  CoglTextureUnit *unit = _cogl_get_texture_unit (unit_index);
 
   _COGL_GET_CONTEXT (ctx, FALSE);
 
-  /* NB: We don't support the random disabling of texture
-   * units, so as soon as we hit a disabled unit we know all
-   * subsequent units are also disabled */
-  if (!unit->enabled)
-    return FALSE;
-
-  if (G_UNLIKELY (unit_index >= fragend->get_max_texture_units ()))
-    {
-      int j;
-      for (j = unit_index; j < ctx->texture_units->len; j++)
-        _cogl_disable_texture_unit (j);
-      /* TODO: although this isn't considered an error that
-       * warrants falling back to a different backend we
-       * should print a warning here. */
-      return FALSE;
-    }
-
   /* Either generate per layer code snippets or setup the
    * fixed function glTexEnv for each layer... */
   if (G_LIKELY (fragend->add_layer (pipeline,
@@ -1086,7 +1022,6 @@ _cogl_pipeline_flush_gl_state (CoglPipeline *pipeline,
    * 2) then foreach layer:
    *  determine gl_target/gl_texture
    *  bind texture
-   *  enable/disable target
    *  flush user matrix
    *
    *  Note: After _cogl_pipeline_flush_common_gl_state you can expect
@@ -1190,10 +1125,10 @@ done:
    * _cogl_bind_gl_texture_transient)
    */
   unit1 = _cogl_get_texture_unit (1);
-  if (unit1->enabled && unit1->dirty_gl_texture)
+  if (cogl_pipeline_get_n_layers (pipeline) > 1 && unit1->dirty_gl_texture)
     {
       _cogl_set_active_texture_unit (1);
-      GE (glBindTexture (unit1->current_gl_target, unit1->gl_texture));
+      GE (glBindTexture (unit1->gl_target, unit1->gl_texture));
       unit1->dirty_gl_texture = FALSE;
     }
 

clutter/cogl/cogl/cogl-pipeline-private.h

@@ -611,8 +611,6 @@ struct _CoglPipeline
 
 typedef struct _CoglPipelineFragend
 {
-  int (*get_max_texture_units) (void);
-
   gboolean (*start) (CoglPipeline *pipeline,
                      int n_layers,
                      unsigned long pipelines_difference,

clutter/cogl/cogl/driver/gles/cogl-gles2-wrapper.c

@@ -269,25 +269,19 @@ cogl_gles2_wrapper_get_locations (GLuint program,
     = glGetUniformLocation (program, "cogl_modelview_matrix");
 
   for (i = 0; i < COGL_GLES2_MAX_TEXTURE_UNITS; i++)
-    if (COGL_GLES2_TEXTURE_UNIT_IS_ENABLED (settings->texture_units, i))
+    {
-      {
+      char *matrix_var_name = g_strdup_printf ("cogl_texture_matrix[%d]", i);
-        char *matrix_var_name = g_strdup_printf ("cogl_texture_matrix[%d]", i);
+      char *tex_coord_var_name =
-        char *tex_coord_var_name =
+        g_strdup_printf ("cogl_tex_coord%d_in", i);
-          g_strdup_printf ("cogl_tex_coord%d_in", i);
 
-        uniforms->texture_matrix_uniforms[i]
+      uniforms->texture_matrix_uniforms[i]
-          = glGetUniformLocation (program, matrix_var_name);
+        = glGetUniformLocation (program, matrix_var_name);
-        attribs->multi_texture_coords[i]
+      attribs->multi_texture_coords[i]
-          = glGetAttribLocation (program, tex_coord_var_name);
+        = glGetAttribLocation (program, tex_coord_var_name);
 
-        g_free (tex_coord_var_name);
+      g_free (tex_coord_var_name);
-        g_free (matrix_var_name);
+      g_free (matrix_var_name);
-      }
+    }
-    else
-      {
-        uniforms->texture_matrix_uniforms[i] = -1;
-        attribs->multi_texture_coords[i] = -1;
-      }
 
   uniforms->point_size_uniform
     = glGetUniformLocation (program, "cogl_point_size_in");
@@ -648,26 +642,25 @@ _cogl_wrap_prepare_for_draw (void)
 
       /* TODO - coverage test */
       for (i = 0; i < COGL_GLES2_MAX_TEXTURE_UNITS; i++)
-        if (COGL_GLES2_TEXTURE_UNIT_IS_ENABLED (w->settings.texture_units, i))
+        {
-          {
+          GLint tex_coord_var_index;
-            GLint tex_coord_var_index;
+          CoglGles2WrapperTextureUnit *texture_unit;
-            CoglGles2WrapperTextureUnit *texture_unit;
 
-            texture_unit = w->texture_units + i;
+          texture_unit = w->texture_units + i;
-            if (!texture_unit->texture_coords_enabled)
+          if (!texture_unit->texture_coords_enabled)
-              continue;
+            continue;
 
-            /* TODO - we should probably have a per unit dirty flag too */
+          /* TODO - we should probably have a per unit dirty flag too */
 
-            /* TODO - coverage test */
+          /* TODO - coverage test */
-            tex_coord_var_index = program->attributes.multi_texture_coords[i];
+          tex_coord_var_index = program->attributes.multi_texture_coords[i];
-            glVertexAttribPointer (tex_coord_var_index,
+          glVertexAttribPointer (tex_coord_var_index,
-                                   texture_unit->texture_coords_size,
+                                 texture_unit->texture_coords_size,
-                                   texture_unit->texture_coords_type,
+                                 texture_unit->texture_coords_type,
-                                   GL_FALSE,
+                                 GL_FALSE,
-                                   texture_unit->texture_coords_stride,
+                                 texture_unit->texture_coords_stride,
-                                   texture_unit->texture_coords_pointer);
+                                 texture_unit->texture_coords_pointer);
-          }
+        }
     }
 
   if (w->dirty_vertex_attrib_enables)