pipeline-cache: Use a special trimmed down pipeline for the key

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.

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

(cherry picked from commit e27e01c1215e7e7c7c0183ded11dd769bb112c5c)

6 files changed, 222 insertions, 15 deletions

diff --git a/cogl/cogl-pipeline-hash-table.c b/cogl/cogl-pipeline-hash-table.c
index 8df63714..8921efc5 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 125b9678..7577559f 100644
--- a/cogl/cogl-pipeline-layer-private.h
+++ b/cogl/cogl-pipeline-layer-private.h
@@ -358,6 +358,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 d9590c8b..9bc26ef6 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 56700b5b..acb5653c 100644
--- a/cogl/cogl-pipeline-private.h
+++ b/cogl/cogl-pipeline-private.h
@@ -845,6 +845,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 c029f45d..a91ad256 100644
--- a/cogl/cogl-pipeline.c
+++ b/cogl/cogl-pipeline.c
@@ -2771,6 +2771,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 63791a4c..1d1447e1 100644
--- a/tests/conform/test-conform-main.c
+++ b/tests/conform/test-conform-main.c
@@ -120,7 +120,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);