[Mesa-dev] [PATCH] i965: Mask the cut index based on the index buffer type in 3DSTATE_VF.

[Ian Romanick]

On 05/23/2013 03:46 PM, Kenneth Graunke wrote:
> According to the documentation: "The Cut Index is compared to the
> fetched (and possibly-sign-extended) vertex index, and if these values

Which documentation is this?  The only types that are valid for index 
buffers are unsigned, so what does "possibly-sign-extended" even mean?

> are equal, the current primitive topology is terminated.  Note that,
> for index buffers <32bpp, it is possible to set the Cut Index to a
> (large) value that will never match a sign-extended vertex index."
>
> This suggests that we should not set the value to 0xFFFFFFFF for
> unsigned byte or short index buffers, but rather 0xFF or 0xFFFF.

For GL_PRIMITIVE_RESTART_FIXED_INDEX (ES and desktop 4.something), where 
the setting of the restart value is out of application control, this is 
absolutely correct.  The OpenGL 4.3 spec says:

     "Primitive restart can also be enabled or disabled with a target
     of PRIMITIVE_RESTART_FIXED_INDEX. In this case, the primitive
     restart index is equal to 2^N − 1, where N is 8, 16 or 32 if the
     type is UNSIGNED_BYTE, UNSIGNED_SHORT, or UNSIGNED_INT,
     respectively, and the index value specified by
     PrimitiveRestartIndex is ignored."

For GL_PRIMITIVE_RESTART, I'm not so sure.  I couldn't find anything 
conclusive any of the specs.  The only thing I found was a hint in the 
NV_primitive_restart extension spec:

     *   What should the default primitive restart index be?

         RESOLVED: Zero.  It's tough to pick another number that is
         meaningful for all three element data types.  In practice, apps
         are likely to set it to 0xFFFF or 0xFFFFFFFF.

You can infer from this that applications are expected to set 0xFFFF for 
GL_UNSIGNED_SHORT and 0xFFFFFFFF for GL_UNSIGNED_LONG.  Experimentation 
is the only way to know for sure. :(

> Fixes sporadic failures in the ES 3 instanced_arrays_primitive_restart
> conformance test when run in combination with other tests.  No Piglit
> regressions.
>
> Cc: Ian Romanick <idr at freedesktop.org
> Cc: Paul Berry <stereotype441 at gmail.com>
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>   src/mesa/drivers/dri/i965/brw_primitive_restart.c | 27 ++++++++++++++++-------
>   1 file changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_primitive_restart.c b/src/mesa/drivers/dri/i965/brw_primitive_restart.c
> index f824915..cf4a1ea 100644
> --- a/src/mesa/drivers/dri/i965/brw_primitive_restart.c
> +++ b/src/mesa/drivers/dri/i965/brw_primitive_restart.c
> @@ -183,19 +183,30 @@ haswell_upload_cut_index(struct brw_context *brw)
>      if (!intel->is_haswell)
>         return;
>
> -   const unsigned cut_index_setting =
> -      ctx->Array._PrimitiveRestart ? HSW_CUT_INDEX_ENABLE : 0;
> -
> -   BEGIN_BATCH(2);
> -   OUT_BATCH(_3DSTATE_VF << 16 | cut_index_setting | (2 - 2));
> -   OUT_BATCH(ctx->Array._RestartIndex);
> -   ADVANCE_BATCH();
> +   if (ctx->Array._PrimitiveRestart) {
> +      int cut_index = ctx->Array._RestartIndex;
> +
> +      if (brw->ib.type == GL_UNSIGNED_BYTE)
> +         cut_index &= 0xff;
> +      else if (brw->ib.type == GL_UNSIGNED_SHORT)
> +         cut_index &= 0xffff;
> +
> +      BEGIN_BATCH(2);
> +      OUT_BATCH(_3DSTATE_VF << 16 | HSW_CUT_INDEX_ENABLE | (2 - 2));
> +      OUT_BATCH(cut_index);
> +      ADVANCE_BATCH();
> +   } else {
> +      BEGIN_BATCH(2);
> +      OUT_BATCH(_3DSTATE_VF << 16 | (2 - 2));
> +      OUT_BATCH(0);
> +      ADVANCE_BATCH();
> +   }
>   }
>
>   const struct brw_tracked_state haswell_cut_index = {
>      .dirty = {
>         .mesa  = _NEW_TRANSFORM,
> -      .brw   = 0,
> +      .brw   = BRW_NEW_INDEX_BUFFER,
>         .cache = 0,
>      },
>      .emit = haswell_upload_cut_index,
>