[cogl/cogl-1.14] pipeline-cache: Use a special trimmed down pipeline for the key

[Neil Roberts <nroberts src gnome org>]

commit 20657d62451c65c27ab441dc5140e2a4e2c75674
Author: Neil Roberts <neil linux intel com>
Date:   Tue Mar 26 11:17:55 2013 +0000

    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)

 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 125b967..7577559 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 d9590c8..9bc26ef 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 56700b5..acb5653 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 c029f45..a91ad25 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 63791a4..1d1447e 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);