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

[Eric Anholt]

Paul Berry <stereotype441 at gmail.com> writes:

> 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.

> 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);

Or
src_reg primitive_id_offset(UNIFORM, this->uniforms, glsl_type::uint_type);

That'll get you a reasonable swizzle that won't be pulling in other
weird stuff when the uniform gets repacked.

> +   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);
> +   }

In other cases param components beyond uniform_vector_size have had a
pointer to a static zero, but I don't see a harm from doing this,
either.

(I would have used a normal C cast, but oh well)

> +   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)));

Note: This overwrites .yzw of g1, but it looks like those are delivered
as zero, and I suspect they're not used afterwards.  You may want to use
dst(ATTR, VARYING_SLOT_PRIMITIVE_ID, glsl_type::uint_type, WRITEMASK_X)
for sanity.

> +   this->current_annotation = NULL;
> +}

I don't think any of this feedback has an actual runtime effect.  Either
way, these two patches are:

Reviewed-by: Eric Anholt <eric at anholt.net>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20131122/4561b62f/attachment-0001.pgp>