[Mesa-dev] [PATCH 2/2] i965/gs: Fix gl_PrimitiveIDIn when using SW primitive restart.

Paul Berry stereotype441 at gmail.com
Fri Oct 11 22:56:27 CEST 2013


Ivy Bridge hardware doesn't support primitve restart under all
circumstances--when it doesn't, we emulate it in software by splitting
up each logical draw operation into multiple 3DPRIMITIVE commands.
This causes the hardware's primitive ID counter to be reset to 0,
producing incorrect values for gl_PrimitiveIDIn.

To work around that problem, we create a hidden uniform which is added
to the hardware's primitive ID counter at the top of the geometry
shader; the software primitive restart mechanism ensures that this
uniform always contains the number of primitives that were sent down
the pipeline by previous 3DPRIMITIVE commands within the same logical
draw operation.

To reduce the performance impact of the workaround, we only compile it
into the shader when the hardware is Ivy Bridge and the shader
accesses gl_PrimitiveIDIn.  To reduce unnecessary state updates, we
only flag _NEW_PROGRAM_CONSTANTS when when the workaround is compiled
in *and* software primitive restart is active.

On Ivy Bridge, fixes all variants of
"glsl-1.50-geometry-primitive-id-restart" except the GL_LINE_LOOP
variants, which are broken due to an unrelated hardware limitation.
---
 src/mesa/drivers/dri/i965/brw_context.c           |  1 +
 src/mesa/drivers/dri/i965/brw_context.h           |  2 ++
 src/mesa/drivers/dri/i965/brw_draw.c              | 20 +++++++++++++++
 src/mesa/drivers/dri/i965/brw_primitive_restart.c |  4 ++-
 src/mesa/drivers/dri/i965/brw_vec4_gs.c           | 11 +++++++++
 src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp | 30 +++++++++++++++++++++++
 src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.h   |  1 +
 7 files changed, 68 insertions(+), 1 deletion(-)

diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
index 776d221..0717398 100644
--- a/src/mesa/drivers/dri/i965/brw_context.c
+++ b/src/mesa/drivers/dri/i965/brw_context.c
@@ -453,6 +453,7 @@ brwCreateContext(int api,
 
    brw->prim_restart.in_progress = false;
    brw->prim_restart.enable_cut_index = false;
+   brw->prim_restart.sw_prim_counter = 0;
 
    brw_init_state( brw );
 
diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
index c6e6655..8625145 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -591,6 +591,7 @@ struct brw_gs_prog_data
    unsigned control_data_format;
 
    bool include_primitive_id;
+   bool need_primitive_id_workaround;
 };
 
 /** Number of texture sampler units */
@@ -1365,6 +1366,7 @@ struct brw_context
    struct {
       bool in_progress;
       bool enable_cut_index;
+      GLuint sw_prim_counter;
    } prim_restart;
 
    /** Computed depth/stencil/hiz state from the current attached
diff --git a/src/mesa/drivers/dri/i965/brw_draw.c b/src/mesa/drivers/dri/i965/brw_draw.c
index 549f9d0a..cf283d3 100644
--- a/src/mesa/drivers/dri/i965/brw_draw.c
+++ b/src/mesa/drivers/dri/i965/brw_draw.c
@@ -445,6 +445,26 @@ retry:
       }
    }
 
+   /* When using software primitive restart, Ivy Bridge resets the primitive
+    * ID on each restart.  We work around this by creating a hidden uniform
+    * whose value will be added to the incoming primitive ID.
+    *
+    * To ensure that the uniform value gets updated, we need to flag
+    * _NEW_PROGRAM_CONSTANTS.
+    *
+    * We do this after the draw call because that's when the uniform value
+    * changes (vbo_sw_primitive_restart() increments it after each draw, and
+    * brw_handle_primitive_restart() resets it to 0 after the final draw).  A
+    * side benefit of doing this after the draw is that we can safely examine
+    * brw->geometry_program and brw->gs.prog_data to see if the workaround is
+    * necessary.
+    */
+   if (brw->gen < 8 && !brw->is_haswell && brw->prim_restart.in_progress &&
+       !brw->prim_restart.enable_cut_index && brw->geometry_program != NULL &&
+       brw->gs.prog_data->need_primitive_id_workaround) {
+      ctx->NewState |= _NEW_PROGRAM_CONSTANTS;
+   }
+
    if (brw->always_flush_batch)
       intel_batchbuffer_flush(brw);
 
diff --git a/src/mesa/drivers/dri/i965/brw_primitive_restart.c b/src/mesa/drivers/dri/i965/brw_primitive_restart.c
index 2572c12..cbd8d21 100644
--- a/src/mesa/drivers/dri/i965/brw_primitive_restart.c
+++ b/src/mesa/drivers/dri/i965/brw_primitive_restart.c
@@ -175,7 +175,9 @@ brw_handle_primitive_restart(struct gl_context *ctx,
       /* Not all the primitive draw modes are supported by the cut index,
        * so take the software path
        */
-      vbo_sw_primitive_restart(ctx, prims, nr_prims, ib, NULL);
+      vbo_sw_primitive_restart(ctx, prims, nr_prims, ib,
+                               &brw->prim_restart.sw_prim_counter);
+      brw->prim_restart.sw_prim_counter = 0;
    }
 
    brw->prim_restart.in_progress = false;
diff --git a/src/mesa/drivers/dri/i965/brw_vec4_gs.c b/src/mesa/drivers/dri/i965/brw_vec4_gs.c
index 064e541..bfc9bc8 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_gs.c
+++ b/src/mesa/drivers/dri/i965/brw_vec4_gs.c
@@ -48,6 +48,13 @@ do_gs_prog(struct brw_context *brw,
    c.prog_data.include_primitive_id =
       (gp->program.Base.InputsRead & VARYING_BIT_PRIMITIVE_ID) != 0;
 
+   /* When using software primitive restart, Ivy Bridge resets the primitive
+    * ID on each restart.  We work around this by creating a hidden uniform
+    * whose value will be added to the incoming primitive ID.
+    */
+   if (brw->gen < 8 && !brw->is_haswell && c.prog_data.include_primitive_id)
+      c.prog_data.need_primitive_id_workaround = true;
+
    /* Allocate the references to the uniforms that will end up in the
     * prog_data associated with the compiled program, and which will be freed
     * by the state cache.
@@ -62,6 +69,10 @@ do_gs_prog(struct brw_context *brw,
    /* We also upload clip plane data as uniforms */
    param_count += MAX_CLIP_PLANES * 4;
 
+   /* If the primitive ID workaround is in use, that uses a uniform too. */
+   if (c.prog_data.need_primitive_id_workaround)
+      param_count++;
+
    c.prog_data.base.param = rzalloc_array(NULL, const float *, param_count);
    c.prog_data.base.pull_param = rzalloc_array(NULL, const float *, param_count);
 
diff --git a/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp
index 96636e8..50feb89 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp
@@ -116,9 +116,39 @@ vec4_gs_visitor::setup_payload()
 }
 
 
+/**
+ * When using software primitive restart, Ivy Bridge resets the primitive ID
+ * on each restart.  Work around this by creating a hidden uniform whose value
+ * will be added to the incoming primitive ID.
+ */
+void
+vec4_gs_visitor::primitive_id_workaround()
+{
+   /* Set up the uniform */
+   this->uniform_vector_size[this->uniforms] = 1;
+   dst_reg primitive_id_offset(UNIFORM, this->uniforms);
+   primitive_id_offset.type = BRW_REGISTER_TYPE_UD;
+   for (int i = 0; i < 4; i++) {
+      prog_data->param[this->uniforms * 4 + i] =
+         reinterpret_cast<const float *>(&brw->prim_restart.sw_prim_counter);
+   }
+   this->uniforms++;
+
+   /* Emit code to add the uniform to primitive ID */
+   this->current_annotation = "primitive ID workaround";
+   dst_reg dst(ATTR, VARYING_SLOT_PRIMITIVE_ID);
+   dst.type = BRW_REGISTER_TYPE_UD;
+   emit(ADD(dst, src_reg(dst), src_reg(primitive_id_offset)));
+   this->current_annotation = NULL;
+}
+
+
 void
 vec4_gs_visitor::emit_prolog()
 {
+   if (c->prog_data.need_primitive_id_workaround)
+      primitive_id_workaround();
+
    /* In vertex shaders, r0.2 is guaranteed to be initialized to zero.  In
     * geometry shaders, it isn't (it contains a bunch of information we don't
     * need, like the input primitive type).  We need r0.2 to be zero in order
diff --git a/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.h b/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.h
index e8da2e3..bdcb415 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.h
+++ b/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.h
@@ -98,6 +98,7 @@ protected:
 private:
    int setup_varying_inputs(int payload_reg, int *attribute_map);
    void emit_control_data_bits();
+   void primitive_id_workaround();
 
    src_reg vertex_count;
    src_reg control_data_bits;
-- 
1.8.4



More information about the mesa-dev mailing list