[Mesa-dev] [PATCH 3/3] mesa: glGet: fix parameter lookup for apps using multiple APIs

[Imre Deak]

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.deak at 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;
-- 
1.7.9.5