arbfp: Copy pipelines used as cache keys

We are currently using a pipeline as a key into our arbfp program cache
but because we weren't making a copy of the pipelines used as keys there
were times when doing a lookup in the cache would end up trying to
compare a lookup key with an entry key that would point to invalid
memory.

Note: the current approach isn't ideal from the pov that that key
pipeline may reference some arbitrarily large user textures will now be
kept alive indefinitely. The plan to improve on this is that we will
have a mechanism to create a special "key pipeline" which will derive
from the default Cogl pipeline (to avoid affecting the lifetime of
other pipelines) and only copy state from the original pipeline that
affects the arbfp program and will reference small dummy textures
instead of potentially large user textures.
This commit is contained in:
Robert Bragg 2010-12-08 16:29:37 +00:00
parent 87b2300f98
commit 46295a0dd1

View File

@ -108,7 +108,11 @@ typedef struct _ArbfpProgramState
{ {
int ref_count; int ref_count;
/* XXX: only valid during codegen */
CoglPipeline *arbfp_authority;
CoglHandle user_program; CoglHandle user_program;
/* XXX: only valid during codegen */
GString *source; GString *source;
GLuint gl_program; GLuint gl_program;
UnitState *unit_state; UnitState *unit_state;
@ -328,6 +332,11 @@ _cogl_pipeline_backend_arbfp_start (CoglPipeline *pipeline,
"PARAM two = {2, 2, 2, 2};\n" "PARAM two = {2, 2, 2, 2};\n"
"PARAM minus_one = {-1, -1, -1, -1};\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... */
arbfp_program_state->arbfp_authority = authority;
for (i = 0; i < n_layers; i++) for (i = 0; i < n_layers; i++)
{ {
arbfp_program_state->unit_state[i].sampled = FALSE; arbfp_program_state->unit_state[i].sampled = FALSE;
@ -928,6 +937,8 @@ _cogl_pipeline_backend_arbfp_end (CoglPipeline *pipeline,
if (G_LIKELY (!(cogl_debug_flags & COGL_DEBUG_DISABLE_PROGRAM_CACHES))) if (G_LIKELY (!(cogl_debug_flags & COGL_DEBUG_DISABLE_PROGRAM_CACHES)))
{ {
CoglPipeline *key;
/* XXX: I wish there was a way to insert into a GHashTable /* XXX: I wish there was a way to insert into a GHashTable
* with a pre-calculated hash value since there is a cost to * with a pre-calculated hash value since there is a cost to
* calculating the hash of a CoglPipeline and in this case * calculating the hash of a CoglPipeline and in this case
@ -935,8 +946,24 @@ _cogl_pipeline_backend_arbfp_end (CoglPipeline *pipeline,
* _cogl_pipeline_arbfp_backend_start so we could pass the * _cogl_pipeline_arbfp_backend_start so we could pass the
* value through to here to avoid hashing it again. * value through to here to avoid hashing it again.
*/ */
g_hash_table_insert (ctx->arbfp_cache, pipeline, arbfp_program_state);
/* 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 (arbfp_program_state->arbfp_authority);
arbfp_program_state_ref (arbfp_program_state); arbfp_program_state_ref (arbfp_program_state);
g_hash_table_insert (ctx->arbfp_cache, key, arbfp_program_state);
if (G_UNLIKELY (g_hash_table_size (ctx->arbfp_cache) > 50)) if (G_UNLIKELY (g_hash_table_size (ctx->arbfp_cache) > 50))
{ {
static gboolean seen = FALSE; static gboolean seen = FALSE;
@ -947,6 +974,11 @@ _cogl_pipeline_backend_arbfp_end (CoglPipeline *pipeline,
seen = TRUE; 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. */
arbfp_program_state->arbfp_authority = NULL;
} }
if (arbfp_program_state->user_program != COGL_INVALID_HANDLE) if (arbfp_program_state->user_program != COGL_INVALID_HANDLE)