[Mesa-dev] [RFC] [PATCH] mesa/st: set max_int instance_divisor for constant elements

[Keith Whitwell]

My preference for these constant elements would be to actually turn them
into constants, ie. bind the buffer as a constant buffer (rather than a
vertex buffer) and have the shader refer to them as CONST[], rather than
INPUT[].

This is based on a few considerations, but primarily a recognition that
none of the main APIs support zero-stride vertex arrays, and these
"arrays" are really state (ctx->Current) masquerading as vertex data.

That said, I've looked at changing this a few times and have backed away
due to the large effort involved for a relatively minor cleanup...

In terms of your change, presumably the driver wants this information
because the hardware also doesn't really support zero-stride arrays &
you have to jump through hoops to make it work?

In any case, it looks like a nice way to side-step my earlier concerns
with zero-stride attributes -- with instancing there is now effectively
a way of specifying this which presumably GL and D3D are also capable of
expressing.

So basically, it looks like an improvement on the current situation and
maybe makes me feel happier about these zero-stride arrays...

Keith

On Wed, 2010-08-11 at 07:38 -0700, Luca Barbieri wrote:
> Currently constant elements are specified only with stride = 0.
> 
> However, drivers often need to know whether an element is constant
> or not when setting up the vertex element CSO instead of when setting
> the buffers.
> 
> Hence, in addition to stride = 0, also set instance_divisor = ~0 in
> the vertex element, in all places where Mesa sets up both at the same time.
> 
> Drivers that ignore instancing will behave as before since stride = 0 is
> set too.
> 
> Drivers that support instancing should continue to behave correctly, and
> if not can just ignore this divisor.
> 
> Drivers that take advantage of this hint will however perform best.
> ---
>  src/mesa/state_tracker/st_draw.c          |    4 ++--
>  src/mesa/state_tracker/st_draw_feedback.c |    2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/mesa/state_tracker/st_draw.c b/src/mesa/state_tracker/st_draw.c
> index 5b05489..ee69572 100644
> --- a/src/mesa/state_tracker/st_draw.c
> +++ b/src/mesa/state_tracker/st_draw.c
> @@ -388,7 +388,7 @@ setup_interleaved_attribs(GLcontext *ctx,
>  
>        velements[attr].src_offset =
>           (unsigned) (arrays[mesaAttr]->Ptr - offset0);
> -      velements[attr].instance_divisor = 0;
> +      velements[attr].instance_divisor = stride ? 0 : ~0;
>        velements[attr].vertex_buffer_index = 0;
>        velements[attr].src_format =
>           st_pipe_vertex_format(arrays[mesaAttr]->Type,
> @@ -483,7 +483,7 @@ setup_non_interleaved_attribs(GLcontext *ctx,
>        /* common-case setup */
>        vbuffer[attr].stride = stride; /* in bytes */
>        vbuffer[attr].max_index = max_index;
> -      velements[attr].instance_divisor = 0;
> +      velements[attr].instance_divisor = stride ? 0 : ~0;
>        velements[attr].vertex_buffer_index = attr;
>        velements[attr].src_format
>           = st_pipe_vertex_format(arrays[mesaAttr]->Type,
> diff --git a/src/mesa/state_tracker/st_draw_feedback.c b/src/mesa/state_tracker/st_draw_feedback.c
> index 5cf2666..41c9ecc 100644
> --- a/src/mesa/state_tracker/st_draw_feedback.c
> +++ b/src/mesa/state_tracker/st_draw_feedback.c
> @@ -180,7 +180,7 @@ st_feedback_draw_vbo(GLcontext *ctx,
>        /* common-case setup */
>        vbuffers[attr].stride = arrays[mesaAttr]->StrideB; /* in bytes */
>        vbuffers[attr].max_index = max_index;
> -      velements[attr].instance_divisor = 0;
> +      velements[attr].instance_divisor = vbuffers[attr].stride ? 0 : ~0;
>        velements[attr].vertex_buffer_index = attr;
>        velements[attr].src_format = 
>           st_pipe_vertex_format(arrays[mesaAttr]->Type,