[cogl] fragend-arbfp: Move the pipeline cache to a separate file

[Neil Roberts <nroberts src gnome org>]

commit 461bff18671e582efacae6a24e5a508e81cb4af9
Author: Neil Roberts <neil linux intel com>
Date:   Thu Jun 30 15:55:56 2011 +0100

    fragend-arbfp: Move the pipeline cache to a separate file
    
    The pipeline cache is now handled in CoglPipelineCache instead of
    directly in the ARBfp fragend. The flags needed to hash a pipeline
    should be exactly the same for the ARBfp and GLSL fragends so it's
    convenient to share the code. The hash table now stores the actual
    pipeline as the value instead of the private data so that the two
    fragends can attach their data to it. That way it's possible to use
    the same pipeline key with ancestors that are using different
    fragends.
    
    The hash table is created with g_hash_table_new_full to set a
    destructor for the key and value and there is a destructor for
    CoglPipelineCache that gets called when the CoglContext is
    destroyed. That way we no longer leak the pipelines and shader state
    when the context is desroyed.

 cogl/Makefile.am                           |    2 +
 cogl/cogl-context-private.h                |    5 +-
 cogl/cogl-context.c                        |    9 +-
 cogl/cogl-pipeline-cache.c                 |  146 ++++++++++++++++++++++
 cogl/cogl-pipeline-cache.h                 |   50 ++++++++
 cogl/cogl-pipeline-fragend-arbfp-private.h |    7 -
 cogl/cogl-pipeline-fragend-arbfp.c         |  180 +++++++---------------------
 7 files changed, 248 insertions(+), 151 deletions(-)
---
diff --git a/cogl/Makefile.am b/cogl/Makefile.am
index f50a27a..7a9e0ea 100644
--- a/cogl/Makefile.am
+++ b/cogl/Makefile.am
@@ -256,6 +256,8 @@ cogl_sources_c = \
 	$(srcdir)/cogl-pipeline-vertend-fixed-private.h	\
 	$(srcdir)/cogl-pipeline-progend-glsl.c		\
 	$(srcdir)/cogl-pipeline-progend-glsl-private.h	\
+	$(srcdir)/cogl-pipeline-cache.h			\
+	$(srcdir)/cogl-pipeline-cache.c			\
 	$(srcdir)/cogl-material-compat.c		\
 	$(srcdir)/cogl-program.c			\
 	$(srcdir)/cogl-program-private.h		\
diff --git a/cogl/cogl-context-private.h b/cogl/cogl-context-private.h
index 8ffcba1..b4adebd 100644
--- a/cogl/cogl-context-private.h
+++ b/cogl/cogl-context-private.h
@@ -42,6 +42,7 @@
 #include "cogl-bitmask.h"
 #include "cogl-atlas.h"
 #include "cogl-texture-driver.h"
+#include "cogl-pipeline-cache.h"
 
 typedef struct
 {
@@ -106,9 +107,7 @@ struct _CoglContext
 
   int               legacy_state_set;
 
-#ifdef HAVE_COGL_GL
-  GHashTable       *arbfp_cache;
-#endif
+  CoglPipelineCache *pipeline_cache;
 
   /* Textures */
   CoglHandle        default_gl_texture_2d_tex;
diff --git a/cogl/cogl-context.c b/cogl/cogl-context.c
index 757bbcd..4ab37e2 100644
--- a/cogl/cogl-context.c
+++ b/cogl/cogl-context.c
@@ -288,10 +288,7 @@ cogl_context_new (CoglDisplay *display,
 
   context->legacy_depth_test_enabled = FALSE;
 
-#ifdef HAVE_COGL_GL
-  _context->arbfp_cache = g_hash_table_new (_cogl_pipeline_fragend_arbfp_hash,
-                                            _cogl_pipeline_fragend_arbfp_equal);
-#endif
+  context->pipeline_cache = cogl_pipeline_cache_new ();
 
   for (i = 0; i < COGL_BUFFER_BIND_TARGET_COUNT; i++)
     context->current_buffer[i] = NULL;
@@ -468,9 +465,7 @@ _cogl_context_free (CoglContext *context)
     cogl_object_unref (_context->flushed_projection_stack);
 #endif
 
-#ifdef HAVE_COGL_GL
-  g_hash_table_unref (context->arbfp_cache);
-#endif
+  cogl_pipeline_cache_free (context->pipeline_cache);
 
   g_byte_array_free (context->buffer_map_fallback_array, TRUE);
 
diff --git a/cogl/cogl-pipeline-cache.c b/cogl/cogl-pipeline-cache.c
new file mode 100644
index 0000000..f5e32a7
--- /dev/null
+++ b/cogl/cogl-pipeline-cache.c
@@ -0,0 +1,146 @@
+/*
+ * Cogl
+ *
+ * An object oriented GL/GLES Abstraction/Utility Layer
+ *
+ * Copyright (C) 2011 Intel Corporation.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * <http://www.gnu.org/licenses/>.
+ *
+ *
+ * Authors:
+ *   Neil Roberts <neil linux intel com>
+ *   Robert Bragg <robert linux intel com>
+ */
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include "cogl-pipeline-private.h"
+#include "cogl-pipeline-cache.h"
+#include "cogl-context-private.h"
+
+struct _CoglPipelineCache
+{
+  GHashTable *fragment_hash;
+};
+
+static unsigned int
+pipeline_fragment_hash (const void *data)
+{
+  unsigned int fragment_state;
+  unsigned int layer_fragment_state;
+
+  _COGL_GET_CONTEXT (ctx, 0);
+
+  fragment_state =
+    _cogl_pipeline_get_state_for_fragment_codegen (ctx);
+  layer_fragment_state =
+    _cogl_pipeline_get_layer_state_for_fragment_codegen (ctx);
+
+  return _cogl_pipeline_hash ((CoglPipeline *)data,
+                              fragment_state, layer_fragment_state,
+                              0);
+}
+
+static gboolean
+pipeline_fragment_equal (const void *a, const void *b)
+{
+  unsigned int fragment_state;
+  unsigned int layer_fragment_state;
+
+  _COGL_GET_CONTEXT (ctx, 0);
+
+  fragment_state =
+    _cogl_pipeline_get_state_for_fragment_codegen (ctx);
+  layer_fragment_state =
+    _cogl_pipeline_get_layer_state_for_fragment_codegen (ctx);
+
+  return _cogl_pipeline_equal ((CoglPipeline *)a, (CoglPipeline *)b,
+                               fragment_state, layer_fragment_state,
+                               0);
+}
+
+CoglPipelineCache *
+cogl_pipeline_cache_new (void)
+{
+  CoglPipelineCache *cache = g_new (CoglPipelineCache, 1);
+
+  cache->fragment_hash = g_hash_table_new_full (pipeline_fragment_hash,
+                                                pipeline_fragment_equal,
+                                                cogl_object_unref,
+                                                cogl_object_unref);
+
+  return cache;
+}
+
+void
+cogl_pipeline_cache_free (CoglPipelineCache *cache)
+{
+  g_hash_table_destroy (cache->fragment_hash);
+  g_free (cache);
+}
+
+CoglPipeline *
+_cogl_pipeline_cache_get_fragment_template (CoglPipelineCache *cache,
+                                            CoglPipeline *key_pipeline)
+{
+  CoglPipeline *template =
+    g_hash_table_lookup (cache->fragment_hash, key_pipeline);
+
+  if (template == NULL)
+    {
+      /* XXX: I wish there was a way to insert into a GHashTable with
+       * a pre-calculated hash value since there is a cost to
+       * calculating the hash of a CoglPipeline and in this case we
+       * know we have already called _cogl_pipeline_hash during the
+       * lookup so we could pass the value through to here to avoid
+       * hashing it again.
+       */
+
+      /* 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. */
+      template = cogl_pipeline_copy (key_pipeline);
+
+      g_hash_table_insert (cache->fragment_hash,
+                           template,
+                           cogl_object_ref (template));
+
+      if (G_UNLIKELY (g_hash_table_size (cache->fragment_hash) > 50))
+        {
+          static gboolean seen = FALSE;
+          if (!seen)
+            g_warning ("Over 50 separate fragment shaders have been "
+                       "generated which is very unusual, so something "
+                       "is probably wrong!\n");
+          seen = TRUE;
+        }
+    }
+
+  return template;
+}
diff --git a/cogl/cogl-pipeline-cache.h b/cogl/cogl-pipeline-cache.h
new file mode 100644
index 0000000..57f80ae
--- /dev/null
+++ b/cogl/cogl-pipeline-cache.h
@@ -0,0 +1,50 @@
+/*
+ * Cogl
+ *
+ * An object oriented GL/GLES Abstraction/Utility Layer
+ *
+ * Copyright (C) 2011 Intel Corporation.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see <http://www.gnu.org/licenses/>.
+ *
+ *
+ */
+
+#ifndef __COGL_PIPELINE_CACHE_H__
+#define __COGL_PIPELINE_CACHE_H__
+
+#include "cogl-pipeline.h"
+
+typedef struct _CoglPipelineCache CoglPipelineCache;
+
+CoglPipelineCache *
+cogl_pipeline_cache_new (void);
+
+void
+cogl_pipeline_cache_free (CoglPipelineCache *cache);
+
+/*
+ * Gets a pipeline from the cache that has the same state as
+ * @key_pipeline for the state in
+ * COGL_PIPELINE_STATE_AFFECTS_FRAGMENT_CODEGEN. If there is no
+ * matching pipline already then a copy of key_pipeline is stored in
+ * the cache so that it will be used next time the function is called
+ * with a similar pipeline. In that case the copy itself will be
+ * returned
+ */
+CoglPipeline *
+_cogl_pipeline_cache_get_fragment_template (CoglPipelineCache *cache,
+                                            CoglPipeline *key_pipeline);
+
+#endif /* __COGL_PIPELINE_CACHE_H__ */
diff --git a/cogl/cogl-pipeline-fragend-arbfp-private.h b/cogl/cogl-pipeline-fragend-arbfp-private.h
index 168bbe9..06c625c 100644
--- a/cogl/cogl-pipeline-fragend-arbfp-private.h
+++ b/cogl/cogl-pipeline-fragend-arbfp-private.h
@@ -32,12 +32,5 @@
 
 extern const CoglPipelineFragend _cogl_pipeline_arbfp_fragend;
 
-unsigned int
-_cogl_pipeline_fragend_arbfp_hash (const void *pipeline);
-
-gboolean
-_cogl_pipeline_fragend_arbfp_equal (const void *pipeline0,
-                                    const void *pipeline1);
-
 #endif /* __COGL_PIPELINE_ARBFP_PRIVATE_H */
 
diff --git a/cogl/cogl-pipeline-fragend-arbfp.c b/cogl/cogl-pipeline-fragend-arbfp.c
index da0907f..aed5d96 100644
--- a/cogl/cogl-pipeline-fragend-arbfp.c
+++ b/cogl/cogl-pipeline-fragend-arbfp.c
@@ -69,9 +69,6 @@ typedef struct
 {
   int ref_count;
 
-  /* XXX: only valid during codegen */
-  CoglPipeline *arbfp_authority;
-
   CoglHandle user_program;
   /* XXX: only valid during codegen */
   GString *source;
@@ -155,6 +152,7 @@ _cogl_pipeline_fragend_arbfp_start (CoglPipeline *pipeline,
 {
   CoglPipelineShaderState *shader_state;
   CoglPipeline *authority;
+  CoglPipeline *template_pipeline;
   CoglHandle user_program;
 
   _COGL_GET_CONTEXT (ctx, FALSE);
@@ -184,8 +182,7 @@ _cogl_pipeline_fragend_arbfp_start (CoglPipeline *pipeline,
         return FALSE;
     }
 
-  /* Now lookup our ARBfp backend private state (allocating if
-   * necessary) */
+  /* Now lookup our ARBfp backend private state */
   shader_state = get_shader_state (pipeline);
 
   /* If we have a valid shader_state then we are all set and don't
@@ -219,113 +216,73 @@ _cogl_pipeline_fragend_arbfp_start (CoglPipeline *pipeline,
 
   /* If we haven't yet found an existing program then before we resort to
    * generating a new arbfp program we see if we can find a suitable
-   * program in the arbfp_cache. */
+   * program in the pipeline_cache. */
   if (G_LIKELY (!(COGL_DEBUG_ENABLED (COGL_DEBUG_DISABLE_PROGRAM_CACHES))))
     {
-      shader_state = g_hash_table_lookup (ctx->arbfp_cache, authority);
+      template_pipeline =
+        _cogl_pipeline_cache_get_fragment_template (ctx->pipeline_cache,
+                                                    authority);
+
+      shader_state = get_shader_state (template_pipeline);
+
       if (shader_state)
-        {
-          shader_state->ref_count++;
-          set_shader_state (pipeline, shader_state);
+        shader_state->ref_count++;
+    }
+
+  /* If we still haven't got a shader state then we'll have to create
+     a new one */
+  if (shader_state == NULL)
+    {
+      shader_state = shader_state_new (n_layers);
 
-          /* Since we have already resolved the arbfp-authority at this point
-           * we might as well also associate any program we find from the cache
-           * with the authority too... */
-          if (authority != pipeline)
+      shader_state->user_program = user_program;
+      if (user_program == COGL_INVALID_HANDLE)
+        {
+          int i;
+
+          /* We reuse a single grow-only GString for code-gen */
+          g_string_set_size (ctx->codegen_source_buffer, 0);
+          shader_state->source = ctx->codegen_source_buffer;
+          g_string_append (shader_state->source,
+                           "!!ARBfp1.0\n"
+                           "TEMP output;\n"
+                           "TEMP tmp0, tmp1, tmp2, tmp3, tmp4;\n"
+                           "PARAM half = {.5, .5, .5, .5};\n"
+                           "PARAM one = {1, 1, 1, 1};\n"
+                           "PARAM two = {2, 2, 2, 2};\n"
+                           "PARAM minus_one = {-1, -1, -1, -1};\n");
+
+          for (i = 0; i < n_layers; i++)
             {
-              shader_state->ref_count++;
-              set_shader_state (authority, shader_state);
+              shader_state->unit_state[i].sampled = FALSE;
+              shader_state->unit_state[i].dirty_combine_constant = FALSE;
             }
-          return TRUE;
+          shader_state->next_constant_id = 0;
         }
     }
 
-  /* If we still haven't found an existing program then start
-   * generating code for a new program...
-   */
-
-  shader_state = shader_state_new (n_layers);
   set_shader_state (pipeline, shader_state);
 
-  /* Since we have already resolved the arbfp-authority at this point we might
-   * as well also associate any program we generate with the authority too...
-   */
+  /* Since we have already resolved the arbfp-authority at this point
+   * we might as well also associate any program we find from the cache
+   * with the authority too... */
   if (authority != pipeline)
     {
       shader_state->ref_count++;
       set_shader_state (authority, shader_state);
     }
 
-  shader_state->user_program = user_program;
-  if (user_program == COGL_INVALID_HANDLE)
+  /* If we found a template then we'll attach it to that too so that
+     next time a similar pipeline is used it can use the same state */
+  if (template_pipeline)
     {
-      int i;
-
-      /* We reuse a single grow-only GString for code-gen */
-      g_string_set_size (ctx->codegen_source_buffer, 0);
-      shader_state->source = ctx->codegen_source_buffer;
-      g_string_append (shader_state->source,
-                       "!!ARBfp1.0\n"
-                       "TEMP output;\n"
-                       "TEMP tmp0, tmp1, tmp2, tmp3, tmp4;\n"
-                       "PARAM half = {.5, .5, .5, .5};\n"
-                       "PARAM one = {1, 1, 1, 1};\n"
-                       "PARAM two = {2, 2, 2, 2};\n"
-                       "PARAM minus_one = {-1, -1, -1, -1};\n");
-
-      /* At the end of code-gen we'll add the program to a cache and
-       * we'll use the authority pipeline as the basis for key into
-       * that cache... */
-      shader_state->arbfp_authority = authority;
-
-      for (i = 0; i < n_layers; i++)
-        {
-          shader_state->unit_state[i].sampled = FALSE;
-          shader_state->unit_state[i].dirty_combine_constant = FALSE;
-        }
-      shader_state->next_constant_id = 0;
+      shader_state->ref_count++;
+      set_shader_state (template_pipeline, shader_state);
     }
 
   return TRUE;
 }
 
-unsigned int
-_cogl_pipeline_fragend_arbfp_hash (const void *data)
-{
-  unsigned int fragment_state;
-  unsigned int layer_fragment_state;
-
-  _COGL_GET_CONTEXT (ctx, 0);
-
-  fragment_state =
-    _cogl_pipeline_get_state_for_fragment_codegen (ctx);
-  layer_fragment_state =
-    _cogl_pipeline_get_layer_state_for_fragment_codegen (ctx);
-
-  return _cogl_pipeline_hash ((CoglPipeline *)data,
-
-                              fragment_state, layer_fragment_state,
-                              0);
-}
-
-gboolean
-_cogl_pipeline_fragend_arbfp_equal (const void *a, const void *b)
-{
-  unsigned int fragment_state;
-  unsigned int layer_fragment_state;
-
-  _COGL_GET_CONTEXT (ctx, 0);
-
-  fragment_state =
-    _cogl_pipeline_get_state_for_fragment_codegen (ctx);
-  layer_fragment_state =
-    _cogl_pipeline_get_layer_state_for_fragment_codegen (ctx);
-
-  return _cogl_pipeline_equal ((CoglPipeline *)a, (CoglPipeline *)b,
-                               fragment_state, layer_fragment_state,
-                               0);
-}
-
 static const char *
 gl_target_to_arbfp_string (GLenum gl_target)
 {
@@ -889,51 +846,6 @@ _cogl_pipeline_fragend_arbfp_end (CoglPipeline *pipeline,
         }
 
       shader_state->source = NULL;
-
-      if (G_LIKELY (!(COGL_DEBUG_ENABLED (COGL_DEBUG_DISABLE_PROGRAM_CACHES))))
-        {
-          CoglPipeline *key;
-
-          /* XXX: I wish there was a way to insert into a GHashTable
-           * with a pre-calculated hash value since there is a cost to
-           * calculating the hash of a CoglPipeline and in this case
-           * we know we have already called _cogl_pipeline_hash during
-           * _cogl_pipeline_fragend_arbfp_backend_start so we could pass the
-           * value through to here to avoid hashing it again.
-           */
-
-          /* 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 arbfp program and references small dummy
-           * textures instead of potentially large user textures. */
-          key = cogl_pipeline_copy (shader_state->arbfp_authority);
-          shader_state->ref_count++;
-          g_hash_table_insert (ctx->arbfp_cache, key, shader_state);
-          if (G_UNLIKELY (g_hash_table_size (ctx->arbfp_cache) > 50))
-            {
-              static gboolean seen = FALSE;
-              if (!seen)
-                g_warning ("Over 50 separate ARBfp programs have been "
-                           "generated which is very unusual, so something "
-                           "is probably wrong!\n");
-              seen = TRUE;
-            }
-        }
-
-      /* The authority is only valid during codegen since the program
-       * state may have a longer lifetime than the original authority
-       * it is created for. */
-      shader_state->arbfp_authority = NULL;
     }
 
   if (shader_state->user_program != COGL_INVALID_HANDLE)