From e6b3bfa41a18d7885ee6933d4333194b9976c0e4 Mon Sep 17 00:00:00 2001 From: Neil Roberts Date: Thu, 9 Feb 2012 20:39:27 +0000 Subject: [PATCH] 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 --- cogl/cogl-pipeline-fragend-arbfp.c | 46 +++++++++++---- cogl/cogl-pipeline-fragend-fixed.c | 78 +++++++++++++++++++++++-- cogl/cogl-pipeline-fragend-glsl.c | 94 +++++++++++++----------------- cogl/cogl-pipeline-layer-private.h | 14 +++-- 4 files changed, 157 insertions(+), 75 deletions(-) diff --git a/cogl/cogl-pipeline-fragend-arbfp.c b/cogl/cogl-pipeline-fragend-arbfp.c index a3d99b166..31727a539 100644 --- a/cogl/cogl-pipeline-fragend-arbfp.c +++ b/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 */ - arg->type = COGL_PIPELINE_FRAGEND_ARBFP_ARG_TYPE_TEXTURE; - arg->name = "texture[%d]"; - arg->texture_unit = src - GL_TEXTURE0; - /* FIXME: Is this right? Shouldn't it be using the texture type - of the layer for the given unit, not the type of the layer - we're generating for? */ - setup_texture_source (shader_state, - arg->texture_unit, - _cogl_pipeline_layer_get_texture_type (layer)); + 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 = + _cogl_pipeline_layer_get_unit_index (other_layer); + texture_type = _cogl_pipeline_layer_get_texture_type (other_layer); + setup_texture_source (shader_state, + arg->texture_unit, + texture_type); + } + } + break; } arg->swizzle = ""; diff --git a/cogl/cogl-pipeline-fragend-fixed.c b/cogl/cogl-pipeline-fragend-fixed.c index 44edfa167..c47917a66 100644 --- a/cogl/cogl-pipeline-fragend-fixed.c +++ b/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])); } diff --git a/cogl/cogl-pipeline-fragend-glsl.c b/cogl/cogl-pipeline-fragend-glsl.c index 91a3e6bba..faaa26c7f 100644 --- a/cogl/cogl-pipeline-fragend-glsl.c +++ b/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, - unit_index); + layer->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) - g_string_append_printf (shader_source, - "cogl_texel%i.%s", - src - COGL_PIPELINE_COMBINE_SOURCE_TEXTURE0, - swizzle); + { + 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", + 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 && - src < COGL_PIPELINE_COMBINE_SOURCE_TEXTURE0 + 32) + if (src >= COGL_PIPELINE_COMBINE_SOURCE_TEXTURE0) { - FindPipelineLayerData data; + 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); - data.unit_index = src - COGL_PIPELINE_COMBINE_SOURCE_TEXTURE0; - data.layer = layer; - - _cogl_pipeline_foreach_layer_internal (pipeline, - find_pipeline_layer_cb, - &data); - - ensure_texture_lookup_generated (shader_state, - pipeline, - data.layer); + if (other_layer) + ensure_texture_lookup_generated (shader_state, + pipeline, + other_layer); } break; } diff --git a/cogl/cogl-pipeline-layer-private.h b/cogl/cogl-pipeline-layer-private.h index 6911b2bef..4446224ba 100644 --- a/cogl/cogl-pipeline-layer-private.h +++ b/cogl/cogl-pipeline-layer-private.h @@ -170,12 +170,14 @@ typedef enum typedef enum { - /* These are the same values as GL */ - COGL_PIPELINE_COMBINE_SOURCE_TEXTURE = 0x1702, - COGL_PIPELINE_COMBINE_SOURCE_CONSTANT = 0x8576, - COGL_PIPELINE_COMBINE_SOURCE_PRIMARY_COLOR = 0x8577, - COGL_PIPELINE_COMBINE_SOURCE_PREVIOUS = 0x8578, - COGL_PIPELINE_COMBINE_SOURCE_TEXTURE0 = 0x84C0 + /* Note that these numbers are deliberately not the same as the GL + numbers so that we can reserve all numbers > TEXTURE0 to store + very large layer numbers */ + COGL_PIPELINE_COMBINE_SOURCE_TEXTURE, + COGL_PIPELINE_COMBINE_SOURCE_CONSTANT, + COGL_PIPELINE_COMBINE_SOURCE_PRIMARY_COLOR, + COGL_PIPELINE_COMBINE_SOURCE_PREVIOUS, + COGL_PIPELINE_COMBINE_SOURCE_TEXTURE0 } CoglPipelineCombineSource; typedef enum