vbo/dlist: only use merged primitives when it's ok to do so

Merging primitives generates incorrect gl_PrimitiveID[In] values.
So make merged primitives construction non-destructive and fallback
to drawing with original primitives if a program reads gl_PrimitiveId.

This commit adds _mesa_update_primitive_id_is_unused modeled after
_mesa_update_allow_draw_out_of_order to update ctx->_PrimitiveIDIsUnused
each time shaders are updated.

Reviewed-by: Marek Olšák <marek.olsak@amd.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7078>

src/mesa/main/context.c

@@ -1294,6 +1294,7 @@ _mesa_initialize_context(struct gl_context *ctx,
    }
 
    ctx->FirstTimeCurrent = GL_TRUE;
+   ctx->_PrimitiveIDIsUnused = true;
 
    return GL_TRUE;
 

src/mesa/main/mtypes.h

@@ -5382,6 +5382,8 @@ struct gl_context
 
    GLboolean ViewportInitialized;  /**< has viewport size been initialized? */
    GLboolean _AllowDrawOutOfOrder;
+   /* Is gl_PrimitiveID unused by the current shaders? */
+   bool _PrimitiveIDIsUnused;
 
    GLbitfield varying_vp_inputs;  /**< mask of VERT_BIT_* flags */
 

src/mesa/main/pipelineobj.c

@@ -536,6 +536,7 @@ _mesa_bind_pipeline(struct gl_context *ctx,
 
       _mesa_update_vertex_processing_mode(ctx);
       _mesa_update_allow_draw_out_of_order(ctx);
+      _mesa_update_primitive_id_is_unused(ctx);
    }
 }
 

src/mesa/main/shaderapi.c

@@ -2581,6 +2581,7 @@ _mesa_use_program(struct gl_context *ctx, gl_shader_stage stage,
                                      shProg);
       _mesa_reference_program(ctx, target, prog);
       _mesa_update_allow_draw_out_of_order(ctx);
+      _mesa_update_primitive_id_is_unused(ctx);
       if (stage == MESA_SHADER_VERTEX)
          _mesa_update_vertex_processing_mode(ctx);
       return;

src/mesa/main/state.c

@@ -147,6 +147,44 @@ _mesa_update_allow_draw_out_of_order(struct gl_context *ctx)
 }
 
 
+void
+_mesa_update_primitive_id_is_unused(struct gl_context *ctx)
+{
+   /* Only the compatibility profile with display lists needs this. */
+   if (ctx->API != API_OPENGL_COMPAT)
+      return;
+
+   /* If all of these are NULL, GLSL is disabled. */
+   struct gl_program *tcs =
+      ctx->_Shader->CurrentProgram[MESA_SHADER_TESS_CTRL];
+   struct gl_program *tes =
+      ctx->_Shader->CurrentProgram[MESA_SHADER_TESS_EVAL];
+   struct gl_program *gs =
+      ctx->_Shader->CurrentProgram[MESA_SHADER_GEOMETRY];
+   struct gl_program *fs =
+      ctx->_Shader->CurrentProgram[MESA_SHADER_FRAGMENT];
+
+   /* Update ctx->_PrimitiveIDIsUnused for display list if
+    * allow_incorrect_primitive_id isn't enabled.
+    * We can use merged primitives (see vbo_save) for drawing unless
+    * one program expects a correct primitive-ID value.
+    */
+   /* TODO: it may be possible to relax the restriction in some cases. If the current
+    * geometry shader doesn't read gl_PrimitiveIDIn but does write gl_PrimitiveID,
+    * then the restriction on fragment shaders reading gl_PrimitiveID can be lifted.
+    */
+   ctx->_PrimitiveIDIsUnused = !(
+         (tcs && (tcs->info.system_values_read & BITFIELD64_BIT(SYSTEM_VALUE_PRIMITIVE_ID) ||
+                  tcs->info.inputs_read & VARYING_BIT_PRIMITIVE_ID)) ||
+         (tes && (tes->info.system_values_read & BITFIELD64_BIT(SYSTEM_VALUE_PRIMITIVE_ID) ||
+                  tes->info.inputs_read & VARYING_BIT_PRIMITIVE_ID)) ||
+         (gs && (gs->info.system_values_read & BITFIELD64_BIT(SYSTEM_VALUE_PRIMITIVE_ID) ||
+                 gs->info.inputs_read & VARYING_BIT_PRIMITIVE_ID)) ||
+         (fs && (fs->info.system_values_read & BITFIELD64_BIT(SYSTEM_VALUE_PRIMITIVE_ID) ||
+                 fs->info.inputs_read & VARYING_BIT_PRIMITIVE_ID)));
+}
+
+
 /**
  * Update the ctx->*Program._Current pointers to point to the
  * current/active programs.

src/mesa/main/state.h

@@ -31,6 +31,9 @@
 extern void
 _mesa_update_allow_draw_out_of_order(struct gl_context *ctx);
 
+extern void
+_mesa_update_primitive_id_is_unused(struct gl_context *ctx);
+
 extern void
 _mesa_update_state(struct gl_context *ctx);
 

src/mesa/vbo/vbo_save.h

@@ -66,10 +66,15 @@ struct vbo_save_vertex_list {
 
    struct _mesa_prim *prims;
    GLuint prim_count;
-
-   struct _mesa_index_buffer ib;
    GLuint min_index, max_index;
 
+   struct {
+      struct _mesa_prim *prims;
+      struct _mesa_index_buffer ib;
+      GLuint prim_count;
+      GLuint min_index, max_index;
+   } merged;
+
    struct vbo_save_primitive_store *prim_store;
 };
 

src/mesa/vbo/vbo_save_api.c

@@ -567,6 +567,9 @@ compile_vertex_list(struct gl_context *ctx)
    node->vertex_count = save->vert_count;
    node->wrap_count = save->copied.nr;
    node->prims = save->prims;
+   node->merged.prims = NULL;
+   node->merged.ib.obj = NULL;
+   node->merged.prim_count = 0;
    node->prim_count = save->prim_count;
    node->prim_store = save->prim_store;
 
@@ -640,9 +643,17 @@ compile_vertex_list(struct gl_context *ctx)
    /* Create an index buffer. */
    node->min_index = node->max_index = 0;
    if (save->vert_count) {
-      int end = node->prims[node->prim_count - 1].start +
+      /* We won't modify node->prims, so use a const alias to avoid unintended
-                node->prims[node->prim_count - 1].count;
+       * writes to it. */
-      int total_vert_count = end - node->prims[0].start;
+      const struct _mesa_prim *original_prims = node->prims;
+
+      int end = original_prims[node->prim_count - 1].start +
+                original_prims[node->prim_count - 1].count;
+      int total_vert_count = end - original_prims[0].start;
+
+      node->min_index = node->prims[0].start;
+      node->max_index = end - 1;
+
       /* Estimate for the worst case: all prims are line strips (the +1 is because
        * wrap_buffers may call use but the last primitive may not be complete) */
       int max_indices_count = MAX2(total_vert_count * 2 - (node->prim_count * 2) + 1,
@@ -656,10 +667,10 @@ compile_vertex_list(struct gl_context *ctx)
       int last_valid_prim = -1;
       /* Construct indices array. */
       for (unsigned i = 0; i < node->prim_count; i++) {
-         assert(node->prims[i].basevertex == 0);
+         assert(original_prims[i].basevertex == 0);
-         GLubyte mode = node->prims[i].mode;
+         GLubyte mode = original_prims[i].mode;
 
-         int vertex_count = node->prims[i].count;
+         int vertex_count = original_prims[i].count;
          if (!vertex_count) {
             continue;
          }
@@ -670,7 +681,7 @@ compile_vertex_list(struct gl_context *ctx)
 
          /* If 2 consecutive prims use the same mode => merge them. */
          bool merge_prims = last_valid_prim >= 0 &&
-                            mode == node->prims[last_valid_prim].mode &&
+                            mode == node->merged.prims[last_valid_prim].mode &&
                             mode != GL_LINE_LOOP && mode != GL_TRIANGLE_FAN &&
                             mode != GL_QUAD_STRIP && mode != GL_POLYGON &&
                             mode != GL_PATCHES;
@@ -681,18 +692,18 @@ compile_vertex_list(struct gl_context *ctx)
          if (merge_prims &&
              mode == GL_TRIANGLE_STRIP) {
             /* Insert a degenerate triangle */
-            assert(node->prims[last_valid_prim].mode == GL_TRIANGLE_STRIP);
+            assert(node->merged.prims[last_valid_prim].mode == GL_TRIANGLE_STRIP);
-            unsigned tri_count = node->prims[last_valid_prim].count - 2;
+            unsigned tri_count = node->merged.prims[last_valid_prim].count - 2;
 
             indices[idx] = indices[idx - 1];
-            indices[idx + 1] = node->prims[i].start;
+            indices[idx + 1] = original_prims[i].start;
             idx += 2;
-            node->prims[last_valid_prim].count += 2;
+            node->merged.prims[last_valid_prim].count += 2;
 
             if (tri_count % 2) {
                /* Add another index to preserve winding order */
-               indices[idx++] = node->prims[i].start;
+               indices[idx++] = original_prims[i].start;
-               node->prims[last_valid_prim].count++;
+               node->merged.prims[last_valid_prim].count++;
             }
          }
 
@@ -702,23 +713,21 @@ compile_vertex_list(struct gl_context *ctx)
           * prim mode is GL_LINES (so merge_prims is true) or if the next
           * primitive mode is GL_LINES or GL_LINE_LOOP.
           */
-         if (node->prims[i].mode == GL_LINE_STRIP &&
+         if (original_prims[i].mode == GL_LINE_STRIP &&
              (merge_prims ||
               (i < node->prim_count - 1 &&
-               (node->prims[i + 1].mode == GL_LINE_STRIP ||
+               (original_prims[i + 1].mode == GL_LINE_STRIP ||
-                node->prims[i + 1].mode == GL_LINES)))) {
+                original_prims[i + 1].mode == GL_LINES)))) {
             for (unsigned j = 0; j < vertex_count; j++) {
-               indices[idx++] = node->prims[i].start + j;
+               indices[idx++] = original_prims[i].start + j;
                /* Repeat all but the first/last indices. */
                if (j && j != vertex_count - 1) {
-                  indices[idx++] = node->prims[i].start + j;
+                  indices[idx++] = original_prims[i].start + j;
-                  node->prims[i].count++;
                }
             }
-            node->prims[i].mode = mode;
          } else {
             for (unsigned j = 0; j < vertex_count; j++) {
-               indices[idx++] = node->prims[i].start + j;
+               indices[idx++] = original_prims[i].start + j;
             }
          }
 
@@ -727,14 +736,17 @@ compile_vertex_list(struct gl_context *ctx)
 
          if (merge_prims) {
             /* Update vertex count. */
-            node->prims[last_valid_prim].count += idx - start;
+            node->merged.prims[last_valid_prim].count += idx - start;
          } else {
             /* Keep this primitive */
             last_valid_prim += 1;
             assert(last_valid_prim <= i);
-            node->prims[i].start = start;
+            node->merged.prims = realloc(node->merged.prims, (1 + last_valid_prim) * sizeof(struct _mesa_prim));
-            node->prims[last_valid_prim] = node->prims[i];
+            node->merged.prims[last_valid_prim] = original_prims[i];
+            node->merged.prims[last_valid_prim].start = start;
+            node->merged.prims[last_valid_prim].count = idx - start;
          }
+         node->merged.prims[last_valid_prim].mode = mode;
       }
 
       if (idx == 0)
@@ -742,35 +754,33 @@ compile_vertex_list(struct gl_context *ctx)
 
       assert(idx <= max_indices_count);
 
-      node->prim_count = last_valid_prim + 1;
+      node->merged.prim_count = last_valid_prim + 1;
-      node->ib.ptr = NULL;
+      node->merged.ib.ptr = NULL;
-      node->ib.count = idx;
+      node->merged.ib.count = idx;
-      node->ib.index_size_shift = (GL_UNSIGNED_INT - GL_UNSIGNED_BYTE) >> 1;
+      node->merged.ib.index_size_shift = (GL_UNSIGNED_INT - GL_UNSIGNED_BYTE) >> 1;
-      node->min_index = min_index;
+      node->merged.min_index = min_index;
-      node->max_index = max_index;
+      node->merged.max_index = max_index;
 
-      node->ib.obj = ctx->Driver.NewBufferObject(ctx, VBO_BUF_ID + 1);
+      node->merged.ib.obj = ctx->Driver.NewBufferObject(ctx, VBO_BUF_ID + 1);
       bool success = ctx->Driver.BufferData(ctx,
                                             GL_ELEMENT_ARRAY_BUFFER_ARB,
                                             idx * sizeof(uint32_t), indices,
                                             GL_STATIC_DRAW_ARB, GL_MAP_WRITE_BIT,
-                                            node->ib.obj);
+                                            node->merged.ib.obj);
 
       if (success)
          goto out;
 
-      ctx->Driver.DeleteBuffer(ctx, node->ib.obj);
+      ctx->Driver.DeleteBuffer(ctx, node->merged.ib.obj);
       _mesa_error(ctx, GL_OUT_OF_MEMORY, "IB allocation");
 
    skip_node:
-      node->ib.obj = NULL;
+      node->merged.ib.obj = NULL;
       node->vertex_count = 0;
       node->prim_count = 0;
 
    out:
       free(indices);
-   } else {
-      node->ib.obj = NULL;
    }
 
    /* Deal with GL_COMPILE_AND_EXECUTE:
@@ -1904,7 +1914,9 @@ vbo_destroy_vertex_list(struct gl_context *ctx, void *data)
       free(node->prim_store);
    }
 
-   _mesa_reference_buffer_object(ctx, &node->ib.obj, NULL);
+   free(node->merged.prims);
+
+   _mesa_reference_buffer_object(ctx, &node->merged.ib.obj, NULL);
    free(node->current_data);
    node->current_data = NULL;
 }

src/mesa/vbo/vbo_save_draw.c

@@ -211,8 +211,17 @@ vbo_save_playback_vertex_list(struct gl_context *ctx, void *data)
       assert(ctx->NewState == 0);
 
       if (node->vertex_count > 0) {
-         ctx->Driver.Draw(ctx, node->prims, node->prim_count, &node->ib, true,
+         bool draw_using_merged_prim = ctx->_PrimitiveIDIsUnused &&
+                                       node->merged.prims;
+         if (!draw_using_merged_prim) {
+            ctx->Driver.Draw(ctx, node->prims, node->prim_count,
+                             NULL, true,
                              false, 0, node->min_index, node->max_index, 1, 0);
+         } else {
+            ctx->Driver.Draw(ctx, node->merged.prims, node->merged.prim_count,
+                             &node->merged.ib, true,
+                             false, 0, node->merged.min_index, node->merged.max_index, 1, 0);
+         }
       }
    }