On 08/23/2012 08:55 AM, Imre Deak wrote:
The glGet hash was initialized only once for a single GL API, even if the application later created a context for a different API. This resulted in glGet failing for otherwise valid parameters in a context if that parameter was invalid in another context created earlier.Amend this by also adding to the hash the parameters of a new API if the application starts to use one. With the change the hash collision statistics reported by GET_DEBUG for the combined API lookup tables aren't significantly different from the single API case. The worst case is still 6 collisions for a single parameter. Signed-off-by: Imre Deak<imre.d...@intel.com> --- src/mesa/main/context.c | 5 ++--- src/mesa/main/get.c | 41 ++++++++++++++++++++++++++++++++--------- 2 files changed, 34 insertions(+), 12 deletions(-) diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c index b78bcee..9831a5f 100644 --- a/src/mesa/main/context.c +++ b/src/mesa/main/context.c @@ -404,9 +404,6 @@ one_time_init( struct gl_context *ctx ) _mesa_get_cpu_features(); - /* context dependence is never a one-time thing... */ - _mesa_init_get_hash(ctx); - for (i = 0; i< 256; i++) { _mesa_ubyte_to_float_color_tab[i] = (float) i / 255.0F; } @@ -425,6 +422,8 @@ one_time_init( struct gl_context *ctx ) /* per-API one-time init */ if (!(api_init_mask& (1<< ctx->API))) { + _mesa_init_get_hash(ctx); + /* * This is fine as ES does not use the remap table, but it may not be * future-proof. We cannot always initialize the remap table because diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c index 38d6cc3..e4818ba 100644 --- a/src/mesa/main/get.c +++ b/src/mesa/main/get.c @@ -1380,7 +1380,14 @@ static const struct value_desc values[] = { * collisions for any enum (typical numbers). And the code is very * simple, even though it feels a little magic. */ -static unsigned short table[1024]; +#define TABLE_IDX_BITS 10 + +#define TABLE_IDX(v) ((v)& ((1<< TABLE_IDX_BITS) - 1)) +#define TABLE_API(v) ((v)>> TABLE_IDX_BITS) + +#define TABLE_VALUE(api, idx) (((api)<< TABLE_IDX_BITS) | (idx)) + +static unsigned short table[1<< TABLE_IDX_BITS]; static const int prime_factor = 89, prime_step = 281; #ifdef GET_DEBUG @@ -1398,14 +1405,14 @@ print_table_stats(void) if (!table[i]) continue; count++; - d =&values[table[i]]; + d =&values[TABLE_IDX(table[i])]; hash = (d->pname * prime_factor); j = 0; while (1) { - if (values[table[hash& mask]].pname == d->pname) - break; - hash += prime_step; - j++; + if (values[TABLE_IDX(table[hash& mask])].pname == d->pname) + break; + hash += prime_step; + j++; } if (j< 10) @@ -1435,6 +1442,8 @@ void _mesa_init_get_hash(struct gl_context *ctx) int i, hash, index, mask; int api_mask = 0, api_bit; + assert(Elements(values)<= 1<< TABLE_IDX_BITS); + mask = Elements(table) - 1; api_bit = 1<< ctx->API; @@ -1450,9 +1459,13 @@ void _mesa_init_get_hash(struct gl_context *ctx) while (1) { index = hash& mask; if (!table[index]) { - table[index] = i; + table[index] = TABLE_VALUE(api_mask, i); break; } + /* Did some other API install the entry already? */ + if (TABLE_IDX(table[index]) == i&& + TABLE_API(table[index]) == api_mask) + break; hash += prime_step; } } @@ -1981,11 +1994,16 @@ find_value(const char *func, GLenum pname, void **p, union value *v) struct gl_texture_unit *unit; int mask, hash; const struct value_desc *d; + int api_bit; + + api_bit = 1<< ctx->API; mask = Elements(table) - 1; hash = (pname * prime_factor); while (1) { - d =&values[table[hash& mask]]; + unsigned table_val = table[hash& mask]; + + d =&values[TABLE_IDX(table_val)]; /* If the enum isn't valid, the hash walk ends with index 0, * which is the API mask entry at the beginning of values[]. */ @@ -1995,7 +2013,12 @@ find_value(const char *func, GLenum pname, void **p, union value *v) return&error_value; } - if (likely(d->pname == pname)) + if (likely(d->pname == pname&& (TABLE_API(table_val)& api_bit))) + /* + * Don't bail out here if the API doesn't match, since the same pname + * can be present in the table for other APIs (with different + * semantics). + */ break; hash += prime_step;
I don't fully grok the hashing changes but this looks OK to me assuming you've run piglit, etc.
Is this a candidate for the 8.0 branch too? For the series: Reviewed-by: Brian Paul <bri...@vmware.com> -Brian _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
