[Cogl] [PATCH 4/5] pipeline-cache: Use a special trimmed down pipeline for the key
Neil Roberts
neil at linux.intel.com
Tue Mar 26 11:36:22 PDT 2013
When a pipeline is added to the cache, a normal copy would previously be
made to use as the key in the hash table. This copy keeps a reference
to the real pipeline which means all of the resources it contains are
retained forever, even if they aren't necessary to generate the hash.
This patch changes it to create a trimmed down copy that only has the
state necessary to generate the hash. A new function called
_cogl_pipeline_deep_copy is added which makes a new pipeline that is
directly a child of the root pipeline. It then copies over the
pertinent state from the original pipeline. The pipeline state is
copied using the existing _cogl_pipeline_copy_differences function.
There was no equivalent function for the layer state so I have added
one.
That way the pipeline key doesn't have the texture data state and it
doesn't hold a reference to the original pipeline so it should be much
cheaper to keep around.
---
cogl/cogl-pipeline-hash-table.c | 25 ++++-----
cogl/cogl-pipeline-layer-private.h | 5 ++
cogl/cogl-pipeline-layer.c | 103 +++++++++++++++++++++++++++++++++++++
cogl/cogl-pipeline-private.h | 11 ++++
cogl/cogl-pipeline.c | 91 ++++++++++++++++++++++++++++++++
tests/conform/test-conform-main.c | 2 +-
6 files changed, 222 insertions(+), 15 deletions(-)
diff --git a/cogl/cogl-pipeline-hash-table.c b/cogl/cogl-pipeline-hash-table.c
index 8df6371..8921efc 100644
--- a/cogl/cogl-pipeline-hash-table.c
+++ b/cogl/cogl-pipeline-hash-table.c
@@ -130,24 +130,21 @@ _cogl_pipeline_hash_table_get (CoglPipelineHashTable *hash,
"unusual, so something is probably wrong!\n",
hash->debug_string);
- /* XXX: Any keys referenced by the hash table need to remain valid
- * all the while that there are corresponding values, so for now we
- * simply make a copy of the current authority pipeline.
- *
- * FIXME: A problem with this is that our key into the cache may
- * hold references to some arbitrary user textures which will now be
- * kept alive indefinitly which is a shame. A better solution will
- * be to derive a special "key pipeline" from the authority which
- * derives from the base Cogl pipeline (to avoid affecting the
- * lifetime of any other pipelines) and only takes a copy of the
- * state that relates to the fragment shader and references small
- * dummy textures instead of potentially large user textures.
- */
entry = g_slice_new (CoglPipelineHashTableEntry);
- entry->pipeline = cogl_pipeline_copy (key_pipeline);
entry->hash = hash;
entry->hash_value = dummy_entry.hash_value;
+ copy_state = hash->main_state;
+ if (hash->layer_state)
+ copy_state |= COGL_PIPELINE_STATE_LAYERS;
+
+ /* Create a new pipeline that is a child of the root pipeline
+ * instead of a normal copy so that the template pipeline won't hold
+ * a reference to the original pipeline */
+ entry->pipeline = _cogl_pipeline_deep_copy (key_pipeline,
+ copy_state,
+ hash->layer_state);
+
g_hash_table_insert (hash->table, entry, entry);
hash->n_unique_pipelines++;
diff --git a/cogl/cogl-pipeline-layer-private.h b/cogl/cogl-pipeline-layer-private.h
index ca69168..7c493da 100644
--- a/cogl/cogl-pipeline-layer-private.h
+++ b/cogl/cogl-pipeline-layer-private.h
@@ -347,6 +347,11 @@ _cogl_pipeline_layer_get_wrap_mode_t (CoglPipelineLayer *layer);
CoglPipelineWrapMode
_cogl_pipeline_layer_get_wrap_mode_p (CoglPipelineLayer *layer);
+void
+_cogl_pipeline_layer_copy_differences (CoglPipelineLayer *dest,
+ CoglPipelineLayer *src,
+ unsigned long differences);
+
unsigned long
_cogl_pipeline_layer_compare_differences (CoglPipelineLayer *layer0,
CoglPipelineLayer *layer1);
diff --git a/cogl/cogl-pipeline-layer.c b/cogl/cogl-pipeline-layer.c
index a3a4545..b08d456 100644
--- a/cogl/cogl-pipeline-layer.c
+++ b/cogl/cogl-pipeline-layer.c
@@ -42,6 +42,8 @@
#include "cogl-context-private.h"
#include "cogl-texture-private.h"
+#include <string.h>
+
static void
_cogl_pipeline_layer_free (CoglPipelineLayer *layer);
@@ -146,6 +148,107 @@ _cogl_get_n_args_for_combine_func (CoglPipelineCombineFunc func)
return 0;
}
+void
+_cogl_pipeline_layer_copy_differences (CoglPipelineLayer *dest,
+ CoglPipelineLayer *src,
+ unsigned long differences)
+{
+ CoglPipelineLayerBigState *big_dest, *big_src;
+
+ if ((differences & COGL_PIPELINE_LAYER_STATE_NEEDS_BIG_STATE) &&
+ !dest->has_big_state)
+ {
+ dest->big_state = g_slice_new (CoglPipelineLayerBigState);
+ dest->has_big_state = TRUE;
+ }
+
+ big_dest = dest->big_state;
+ big_src = src->big_state;
+
+ dest->differences |= differences;
+
+ while (differences)
+ {
+ int index = _cogl_util_ffs (differences) - 1;
+
+ differences &= ~(1 << index);
+
+ /* This convoluted switch statement is just here so that we'll
+ * get a warning if a new state is added without handling it
+ * here */
+ switch (index)
+ {
+ case COGL_PIPELINE_LAYER_STATE_COUNT:
+ case COGL_PIPELINE_LAYER_STATE_UNIT_INDEX:
+ g_warn_if_reached ();
+ break;
+
+ case COGL_PIPELINE_LAYER_STATE_TEXTURE_TYPE_INDEX:
+ dest->texture_type = src->texture_type;
+ break;
+
+ case COGL_PIPELINE_LAYER_STATE_TEXTURE_DATA_INDEX:
+ dest->texture = src->texture;
+ if (dest->texture)
+ cogl_object_ref (dest->texture);
+ break;
+
+ case COGL_PIPELINE_LAYER_STATE_SAMPLER_INDEX:
+ dest->sampler_cache_entry = src->sampler_cache_entry;
+ break;
+
+ case COGL_PIPELINE_LAYER_STATE_COMBINE_INDEX:
+ {
+ CoglPipelineCombineFunc func;
+ int n_args, i;
+
+ func = big_src->texture_combine_rgb_func;
+ big_dest->texture_combine_rgb_func = func;
+ n_args = _cogl_get_n_args_for_combine_func (func);
+ for (i = 0; i < n_args; i++)
+ {
+ big_dest->texture_combine_rgb_src[i] =
+ big_src->texture_combine_rgb_src[i];
+ big_dest->texture_combine_rgb_op[i] =
+ big_src->texture_combine_rgb_op[i];
+ }
+
+ func = big_src->texture_combine_alpha_func;
+ big_dest->texture_combine_alpha_func = func;
+ n_args = _cogl_get_n_args_for_combine_func (func);
+ for (i = 0; i < n_args; i++)
+ {
+ big_dest->texture_combine_alpha_src[i] =
+ big_src->texture_combine_alpha_src[i];
+ big_dest->texture_combine_alpha_op[i] =
+ big_src->texture_combine_alpha_op[i];
+ }
+ }
+ break;
+
+ case COGL_PIPELINE_LAYER_STATE_COMBINE_CONSTANT_INDEX:
+ memcpy (big_dest->texture_combine_constant,
+ big_src->texture_combine_constant,
+ sizeof (big_dest->texture_combine_constant));
+ break;
+
+ case COGL_PIPELINE_LAYER_STATE_POINT_SPRITE_COORDS_INDEX:
+ big_dest->point_sprite_coords = big_src->point_sprite_coords;
+ break;
+
+ case COGL_PIPELINE_LAYER_STATE_VERTEX_SNIPPETS_INDEX:
+ _cogl_pipeline_snippet_list_copy (&big_dest->vertex_snippets,
+ &big_src->vertex_snippets);
+ break;
+
+ case COGL_PIPELINE_LAYER_STATE_FRAGMENT_SNIPPETS_INDEX:
+ _cogl_pipeline_snippet_list_copy (&big_dest->fragment_snippets,
+ &big_src->fragment_snippets);
+ break;
+ }
+ }
+}
+
static void
_cogl_pipeline_layer_init_multi_property_sparse_state (
CoglPipelineLayer *layer,
diff --git a/cogl/cogl-pipeline-private.h b/cogl/cogl-pipeline-private.h
index 32f6d51..67fe5e4 100644
--- a/cogl/cogl-pipeline-private.h
+++ b/cogl/cogl-pipeline-private.h
@@ -787,6 +787,17 @@ _cogl_pipeline_hash (CoglPipeline *pipeline,
unsigned long layer_differences,
CoglPipelineEvalFlags flags);
+/* Makes a copy of the given pipeline that is a child of the root
+ * pipeline rather than a child of the source pipeline. That way the
+ * new pipeline won't hold a reference to the source pipeline. The
+ * differences specified in @differences and @layer_differences are
+ * copied across and all other state is left with the default
+ * values. */
+CoglPipeline *
+_cogl_pipeline_deep_copy (CoglPipeline *pipeline,
+ unsigned long differences,
+ unsigned long layer_differences);
+
CoglPipeline *
_cogl_pipeline_journal_ref (CoglPipeline *pipeline);
diff --git a/cogl/cogl-pipeline.c b/cogl/cogl-pipeline.c
index 314cf44..c397eb5 100644
--- a/cogl/cogl-pipeline.c
+++ b/cogl/cogl-pipeline.c
@@ -2522,6 +2522,97 @@ _cogl_pipeline_hash (CoglPipeline *pipeline,
typedef struct
{
+ CoglContext *context;
+ CoglPipeline *src_pipeline;
+ CoglPipeline *dst_pipeline;
+ unsigned int layer_differences;
+} DeepCopyData;
+
+static CoglBool
+deep_copy_layer_cb (CoglPipelineLayer *src_layer,
+ void *user_data)
+{
+ DeepCopyData *data = user_data;
+ CoglPipelineLayer *dst_layer;
+ unsigned int differences = data->layer_differences;
+
+ dst_layer = _cogl_pipeline_get_layer (data->dst_pipeline, src_layer->index);
+
+ while (src_layer != data->context->default_layer_n &&
+ src_layer != data->context->default_layer_0 &&
+ differences)
+ {
+ unsigned long to_copy = differences & src_layer->differences;
+
+ if (to_copy)
+ {
+ _cogl_pipeline_layer_copy_differences (dst_layer, src_layer, to_copy);
+ differences ^= to_copy;
+ }
+
+ src_layer = COGL_PIPELINE_LAYER (COGL_NODE (src_layer)->parent);
+ }
+
+ return TRUE;
+}
+
+CoglPipeline *
+_cogl_pipeline_deep_copy (CoglPipeline *pipeline,
+ unsigned long differences,
+ unsigned long layer_differences)
+{
+ CoglPipeline *new, *authority;
+ CoglBool copy_layer_state;
+
+ _COGL_GET_CONTEXT (ctx, NULL);
+
+ if ((differences & COGL_PIPELINE_STATE_LAYERS))
+ {
+ copy_layer_state = TRUE;
+ differences &= ~COGL_PIPELINE_STATE_LAYERS;
+ }
+ else
+ copy_layer_state = FALSE;
+
+ new = cogl_pipeline_new (ctx);
+
+ for (authority = pipeline;
+ authority != ctx->default_pipeline && differences;
+ authority = COGL_PIPELINE (COGL_NODE (authority)->parent))
+ {
+ unsigned long to_copy = differences & authority->differences;
+
+ if (to_copy)
+ {
+ _cogl_pipeline_copy_differences (new, authority, to_copy);
+ differences ^= to_copy;
+ }
+ }
+
+ if (copy_layer_state)
+ {
+ DeepCopyData data;
+
+ /* The unit index doesn't need to be copied because it should
+ * end up with the same values anyway because the new pipeline
+ * will have the same indices as the source pipeline */
+ layer_differences &= ~COGL_PIPELINE_LAYER_STATE_UNIT;
+
+ data.context = ctx;
+ data.src_pipeline = pipeline;
+ data.dst_pipeline = new;
+ data.layer_differences = layer_differences;
+
+ _cogl_pipeline_foreach_layer_internal (pipeline,
+ deep_copy_layer_cb,
+ &data);
+ }
+
+ return new;
+}
+
+typedef struct
+{
int i;
CoglPipelineLayer **layers;
} AddLayersToArrayState;
diff --git a/tests/conform/test-conform-main.c b/tests/conform/test-conform-main.c
index 2dd5655..9345065 100644
--- a/tests/conform/test-conform-main.c
+++ b/tests/conform/test-conform-main.c
@@ -122,7 +122,7 @@ main (int argc, char **argv)
ADD_TEST (test_copy_replace_texture, 0, 0);
- ADD_TEST (test_pipeline_cache_unrefs_texture, 0, TEST_KNOWN_FAILURE);
+ ADD_TEST (test_pipeline_cache_unrefs_texture, 0, 0);
UNPORTED_TEST (test_viewport);
--
1.7.11.3.g3c3efa5
More information about the Cogl
mailing list