pipeline: Use layer numbers not unit numbers in the combine strings

In a combine string the application can specify TEXTURE_? as a source
to sample from the texture attached to a particular unit. The number
specified here was being interpreted as a unit index. This is not
helpful to applications because theoretically the unit index is an
internal implementation detail so they can't reliably determine what
it is. This patch changes them to be interpreted as layer indices
instead.

To make this work the enums in CoglPipelineCombineSource are no longer
directly mapped to GLenums. Otherwise it implies a low limit on the
number of layer indices because there are only 32 reserved numbers
between GL_TEXTURE0 and GL_ACTIVE_TEXTURE.

This also fixes a bug in the ARBfp fragend where it was generating
code using the texture type of the layer doing the referencing rather
than the layer that was being referenced.

Reviewed-by: Robert Bragg <robert@linux.intel.com>

cogl/cogl-pipeline-fragend-arbfp.c

@@ -391,7 +391,7 @@ setup_arg (CoglPipeline *pipeline,
            CoglPipelineLayer *layer,
            CoglBlendStringChannelMask mask,
            int arg_index,
-           GLint src,
+           CoglPipelineCombineSource src,
            GLint op,
            CoglPipelineFragendARBfpArg *arg)
 {
@@ -433,16 +433,40 @@ setup_arg (CoglPipeline *pipeline,
       else
         arg->name = "output";
       break;
-    default: /* GL_TEXTURE0..N */
+    default: /* Sample the texture attached to a specific layer */
+      {
+        int layer_num = src - COGL_PIPELINE_COMBINE_SOURCE_TEXTURE0;
+        CoglPipelineGetLayerFlags flags = COGL_PIPELINE_GET_LAYER_NO_CREATE;
+        CoglPipelineLayer *other_layer =
+          _cogl_pipeline_get_layer_with_flags (pipeline, layer_num, flags);
+
+        if (other_layer == NULL)
+          {
+            static gboolean warning_seen = FALSE;
+            if (!warning_seen)
+              {
+                g_warning ("The application is trying to use a texture "
+                           "combine with a layer number that does not exist");
+                warning_seen = TRUE;
+              }
+            arg->type = COGL_PIPELINE_FRAGEND_ARBFP_ARG_TYPE_SIMPLE;
+            arg->name = "output";
+          }
+        else
+          {
+            CoglTextureType texture_type;
+
             arg->type = COGL_PIPELINE_FRAGEND_ARBFP_ARG_TYPE_TEXTURE;
             arg->name = "texture[%d]";
-      arg->texture_unit = src - GL_TEXTURE0;
+            arg->texture_unit =
-      /* FIXME: Is this right? Shouldn't it be using the texture type
+              _cogl_pipeline_layer_get_unit_index (other_layer);
-         of the layer for the given unit, not the type of the layer
+            texture_type = _cogl_pipeline_layer_get_texture_type (other_layer);
-         we're generating for? */
             setup_texture_source (shader_state,
                                   arg->texture_unit,
-                            _cogl_pipeline_layer_get_texture_type (layer));
+                                  texture_type);
+          }
+      }
+      break;
     }
 
   arg->swizzle = "";

cogl/cogl-pipeline-fragend-fixed.c

@@ -123,6 +123,61 @@ _cogl_pipeline_fragend_fixed_start (CoglPipeline *pipeline,
   return TRUE;
 }
 
+static void
+translate_sources (CoglPipeline *pipeline,
+                   int n_sources,
+                   CoglPipelineCombineSource *source_in,
+                   GLenum *source_out)
+{
+  int i;
+
+  /* The texture source numbers specified in the layer combine are the
+     layer numbers so we need to map these to unit indices */
+
+  for (i = 0; i < n_sources; i++)
+    switch (source_in[i])
+      {
+      case COGL_PIPELINE_COMBINE_SOURCE_TEXTURE:
+        source_out[i] = GL_TEXTURE;
+        break;
+
+      case COGL_PIPELINE_COMBINE_SOURCE_CONSTANT:
+        source_out[i] = GL_CONSTANT;
+        break;
+
+      case COGL_PIPELINE_COMBINE_SOURCE_PRIMARY_COLOR:
+        source_out[i] = GL_PRIMARY_COLOR;
+        break;
+
+      case COGL_PIPELINE_COMBINE_SOURCE_PREVIOUS:
+        source_out[i] = GL_PREVIOUS;
+        break;
+
+      default:
+        {
+          int layer_num = source_in[i] - COGL_PIPELINE_COMBINE_SOURCE_TEXTURE0;
+          CoglPipelineGetLayerFlags flags = COGL_PIPELINE_GET_LAYER_NO_CREATE;
+          CoglPipelineLayer *layer =
+            _cogl_pipeline_get_layer_with_flags (pipeline, layer_num, flags);
+
+          if (layer == NULL)
+            {
+              static gboolean warning_seen = FALSE;
+              if (!warning_seen)
+                {
+                  g_warning ("The application is trying to use a texture "
+                             "combine with a layer number that does not exist");
+                  warning_seen = TRUE;
+                }
+              source_out[i] = GL_PREVIOUS;
+            }
+          else
+            source_out[i] = (_cogl_pipeline_layer_get_unit_index (layer) +
+                             GL_TEXTURE0);
+        }
+      }
+}
+
 static gboolean
 _cogl_pipeline_fragend_fixed_add_layer (CoglPipeline *pipeline,
                                         CoglPipelineLayer *layer,
@@ -216,6 +271,7 @@ _cogl_pipeline_fragend_fixed_add_layer (CoglPipeline *pipeline,
         _cogl_pipeline_layer_get_authority (layer,
                                             COGL_PIPELINE_LAYER_STATE_COMBINE);
       CoglPipelineLayerBigState *big_state = authority->big_state;
+      GLenum sources[3];
 
       GE (ctx, glTexEnvi (GL_TEXTURE_ENV, GL_TEXTURE_ENV_MODE, GL_COMBINE));
 
@@ -235,21 +291,26 @@ _cogl_pipeline_fragend_fixed_add_layer (CoglPipeline *pipeline,
       n_rgb_func_args =
         _cogl_get_n_args_for_combine_func (big_state->texture_combine_rgb_func);
 
+      translate_sources (pipeline,
+                         n_rgb_func_args,
+                         big_state->texture_combine_rgb_src,
+                         sources);
+
       GE (ctx, glTexEnvi (GL_TEXTURE_ENV, GL_SRC0_RGB,
-                          big_state->texture_combine_rgb_src[0]));
+                          sources[0]));
       GE (ctx, glTexEnvi (GL_TEXTURE_ENV, GL_OPERAND0_RGB,
                           big_state->texture_combine_rgb_op[0]));
       if (n_rgb_func_args > 1)
         {
           GE (ctx, glTexEnvi (GL_TEXTURE_ENV, GL_SRC1_RGB,
-                              big_state->texture_combine_rgb_src[1]));
+                              sources[1]));
           GE (ctx, glTexEnvi (GL_TEXTURE_ENV, GL_OPERAND1_RGB,
                               big_state->texture_combine_rgb_op[1]));
         }
       if (n_rgb_func_args > 2)
         {
           GE (ctx, glTexEnvi (GL_TEXTURE_ENV, GL_SRC2_RGB,
-                              big_state->texture_combine_rgb_src[2]));
+                              sources[2]));
           GE (ctx, glTexEnvi (GL_TEXTURE_ENV, GL_OPERAND2_RGB,
                               big_state->texture_combine_rgb_op[2]));
         }
@@ -258,21 +319,26 @@ _cogl_pipeline_fragend_fixed_add_layer (CoglPipeline *pipeline,
       n_alpha_func_args =
         _cogl_get_n_args_for_combine_func (big_state->texture_combine_alpha_func);
 
+      translate_sources (pipeline,
+                         n_alpha_func_args,
+                         big_state->texture_combine_alpha_src,
+                         sources);
+
       GE (ctx, glTexEnvi (GL_TEXTURE_ENV, GL_SRC0_ALPHA,
-                          big_state->texture_combine_alpha_src[0]));
+                          sources[0]));
       GE (ctx, glTexEnvi (GL_TEXTURE_ENV, GL_OPERAND0_ALPHA,
                           big_state->texture_combine_alpha_op[0]));
       if (n_alpha_func_args > 1)
         {
           GE (ctx, glTexEnvi (GL_TEXTURE_ENV, GL_SRC1_ALPHA,
-                              big_state->texture_combine_alpha_src[1]));
+                              sources[1]));
           GE (ctx, glTexEnvi (GL_TEXTURE_ENV, GL_OPERAND1_ALPHA,
                               big_state->texture_combine_alpha_op[1]));
         }
       if (n_alpha_func_args > 2)
         {
           GE (ctx, glTexEnvi (GL_TEXTURE_ENV, GL_SRC2_ALPHA,
-                              big_state->texture_combine_alpha_src[2]));
+                              sources[2]));
           GE (ctx, glTexEnvi (GL_TEXTURE_ENV, GL_OPERAND2_ALPHA,
                               big_state->texture_combine_alpha_op[2]));
         }

cogl/cogl-pipeline-fragend-glsl.c

@@ -391,12 +391,12 @@ ensure_texture_lookup_generated (CoglPipelineShaderState *shader_state,
 
   g_string_append_printf (shader_state->header,
                           "vec4 cogl_texel%i;\n",
-                          unit_index);
+                          layer->index);
 
   g_string_append_printf (shader_state->source,
                           "  cogl_texel%i = cogl_texture_lookup%i (",
-                          unit_index,
+                          layer->index,
-                          unit_index);
+                          layer->index);
 
   /* If point sprite coord generation is being used then divert to the
      built-in varying var for that instead of the texture
@@ -463,7 +463,7 @@ ensure_texture_lookup_generated (CoglPipelineShaderState *shader_state,
                               "cogl_real_texture_lookup%i (vec4 coords)\n"
                               "{\n"
                               "  return ",
-                              unit_index);
+                              layer->index);
 
       if (G_UNLIKELY (COGL_DEBUG_ENABLED (COGL_DEBUG_DISABLE_TEXTURING)))
         g_string_append (shader_state->header,
@@ -481,11 +481,11 @@ ensure_texture_lookup_generated (CoglPipelineShaderState *shader_state,
   snippet_data.snippets = get_layer_fragment_snippets (layer);
   snippet_data.hook = COGL_SNIPPET_HOOK_TEXTURE_LOOKUP;
   snippet_data.chain_function = g_strdup_printf ("cogl_real_texture_lookup%i",
-                                                 unit_index);
+                                                 layer->index);
   snippet_data.final_name = g_strdup_printf ("cogl_texture_lookup%i",
-                                             unit_index);
+                                             layer->index);
   snippet_data.function_prefix = g_strdup_printf ("cogl_texture_lookup_hook%i",
-                                                  unit_index);
+                                                  layer->index);
   snippet_data.return_type = "vec4";
   snippet_data.return_variable = "cogl_texel";
   snippet_data.arguments = "cogl_tex_coord";
@@ -533,7 +533,7 @@ add_arg (CoglPipelineShaderState *shader_state,
     case COGL_PIPELINE_COMBINE_SOURCE_TEXTURE:
       g_string_append_printf (shader_source,
                               "cogl_texel%i.%s",
-                              _cogl_pipeline_layer_get_unit_index (layer),
+                              layer->index,
                               swizzle);
       break;
 
@@ -559,43 +559,37 @@ add_arg (CoglPipelineShaderState *shader_state,
       break;
 
     default:
-      if (src >= COGL_PIPELINE_COMBINE_SOURCE_TEXTURE0 &&
+      {
-          src < COGL_PIPELINE_COMBINE_SOURCE_TEXTURE0 + 32)
+        int layer_num = src - COGL_PIPELINE_COMBINE_SOURCE_TEXTURE0;
+        CoglPipelineGetLayerFlags flags = COGL_PIPELINE_GET_LAYER_NO_CREATE;
+        CoglPipelineLayer *other_layer =
+          _cogl_pipeline_get_layer_with_flags (pipeline, layer_num, flags);
+
+        if (other_layer == NULL)
+          {
+            static gboolean warning_seen = FALSE;
+            if (!warning_seen)
+              {
+                g_warning ("The application is trying to use a texture "
+                           "combine with a layer number that does not exist");
+                warning_seen = TRUE;
+              }
+            g_string_append_printf (shader_source,
+                                    "vec4 (1.0, 1.0, 1.0, 1.0).%s",
+                                    swizzle);
+          }
+        else
           g_string_append_printf (shader_source,
                                   "cogl_texel%i.%s",
-                                src - COGL_PIPELINE_COMBINE_SOURCE_TEXTURE0,
+                                  other_layer->index,
                                   swizzle);
+      }
       break;
     }
 
   g_string_append_c (shader_source, ')');
 }
 
-
-typedef struct
-{
-  int unit_index;
-  CoglPipelineLayer *layer;
-} FindPipelineLayerData;
-
-static gboolean
-find_pipeline_layer_cb (CoglPipelineLayer *layer,
-                        void *user_data)
-{
-  FindPipelineLayerData *data = user_data;
-  int unit_index;
-
-  unit_index = _cogl_pipeline_layer_get_unit_index (layer);
-
-  if (unit_index == data->unit_index)
-    {
-      data->layer = layer;
-      return FALSE;
-    }
-
-  return TRUE;
-}
-
 static void
 ensure_arg_generated (CoglPipeline *pipeline,
                       CoglPipelineLayer *layer,
@@ -636,21 +630,17 @@ ensure_arg_generated (CoglPipeline *pipeline,
       break;
 
     default:
-      if (src >= COGL_PIPELINE_COMBINE_SOURCE_TEXTURE0 &&
+      if (src >= COGL_PIPELINE_COMBINE_SOURCE_TEXTURE0)
-          src < COGL_PIPELINE_COMBINE_SOURCE_TEXTURE0 + 32)
         {
-          FindPipelineLayerData data;
+          int layer_num = src - COGL_PIPELINE_COMBINE_SOURCE_TEXTURE0;
-
+          CoglPipelineGetLayerFlags flags = COGL_PIPELINE_GET_LAYER_NO_CREATE;
-          data.unit_index = src - COGL_PIPELINE_COMBINE_SOURCE_TEXTURE0;
+          CoglPipelineLayer *other_layer =
-          data.layer = layer;
+            _cogl_pipeline_get_layer_with_flags (pipeline, layer_num, flags);
-
-          _cogl_pipeline_foreach_layer_internal (pipeline,
-                                                 find_pipeline_layer_cb,
-                                                 &data);
 
+          if (other_layer)
             ensure_texture_lookup_generated (shader_state,
                                              pipeline,
-                                           data.layer);
+                                             other_layer);
         }
       break;
     }

cogl/cogl-pipeline-layer-private.h

@@ -170,12 +170,14 @@ typedef enum
 
 typedef enum
 {
-  /* These are the same values as GL */
+  /* Note that these numbers are deliberately not the same as the GL
-  COGL_PIPELINE_COMBINE_SOURCE_TEXTURE       = 0x1702,
+     numbers so that we can reserve all numbers > TEXTURE0 to store
-  COGL_PIPELINE_COMBINE_SOURCE_CONSTANT      = 0x8576,
+     very large layer numbers */
-  COGL_PIPELINE_COMBINE_SOURCE_PRIMARY_COLOR = 0x8577,
+  COGL_PIPELINE_COMBINE_SOURCE_TEXTURE,
-  COGL_PIPELINE_COMBINE_SOURCE_PREVIOUS      = 0x8578,
+  COGL_PIPELINE_COMBINE_SOURCE_CONSTANT,
-  COGL_PIPELINE_COMBINE_SOURCE_TEXTURE0      = 0x84C0
+  COGL_PIPELINE_COMBINE_SOURCE_PRIMARY_COLOR,
+  COGL_PIPELINE_COMBINE_SOURCE_PREVIOUS,
+  COGL_PIPELINE_COMBINE_SOURCE_TEXTURE0
 } CoglPipelineCombineSource;
 
 typedef enum