[Mesa-dev] [PATCH 00/13] Fix gl_VertexID on i965

[Kenneth Graunke]

On Monday, August 04, 2014 08:17:13 PM Ian Romanick wrote:
> On 08/03/2014 11:07 PM, Kenneth Graunke wrote:
> > On Sunday, June 22, 2014 03:59:01 AM Marek Olšák wrote:
> >> That's right. A uniform won't work with ARB_draw_indirect unless
> >> you lower it to direct draws, which would be very bad if it was
> >> applied to all drivers.
> >> 
> >> Radeonsi indeed supports BaseVertex and BaseInstance as system
> >> values in the vertex shader. Well, vertex fetching has to be done
> >> in the vertex shader on that hardware, so the system values are
> >> kinda required there.
> >> 
> >> Marek
> > 
> > FWIW, I've thrown together some patches for i965 which make
> > gl_BaseVertex available as a system value, which I think is the right
> > thing to do.  I've tested the indirect draw case, and it seems to
> > work fine.
> 
> Yeah, I like your approach.  Patch 12 in my original series causes
> regressions in a couple piglit tests... which I somehow did not see
> before.  It looks like your series regressions some geometry shader
> tests.  glsl-1.50-geometry-primitive-types looks like the geometry
> shader is getting a floating point value for the vertex ID:
> 
> Testing GL_LINES(1 vertices)
> Testing GL_LINES(2 vertices)
>   Expected vertex IDs: 1 2
>   Actual vertex IDs:   -1082130431 1065353218
> Testing GL_LINES(3 vertices)
>   Expected vertex IDs: 1 2
>   Actual vertex IDs:   -1082130431 1065353218
> Testing GL_LINES(4 vertices)
>   Expected vertex IDs: 1 2 3 4
>   Actual vertex IDs:   -1082130431 1065353218 1065353219 -1082130428
> 
> -1082130431 is 0xbf800001, which is the bit encoding of -1.0.
> 1065353218 is the bit encoding of 1.0.  So... there's something fishy
> there. :)

Whoops.  I believe I've fixed this in my 'basevertex-v6.1' branch.  I wasn't emitting the 3DSTATE_VERTEX_BUFFERS for the buffer containing gl_BaseVertex unless there were other vertex buffers as well.  Somehow, you ended up with -1.0...I got more rubbish looking values.

> I did some clean up on the branch and pushed it to basevertex-v6.  I
> squashed your HAX commit into the original commit for the lowering pass,
> and I removed the commit that added the fake uniform.

Looks good!  One nit...I noticed that the lowering flag is still called gl_VertexID_using_uniform_gl_BaseVertex.

Personally, I would just drop the enum and use a bool for whether to add gl_BaseVertex to gl_VertexID.  It sounds like the system value is what most people want, and if someone wants a uniform path, they can extend it in the future.

> I think your approach will allow us to implement most of
> GL_ARB_shader_draw_parameters without much difficulty.  We'll get
> gl_BaseVertex and gl_BaseInstance... gl_DrawID seems a bit harder.

Yeah :) Perhaps I should call it draw_parameters_bo instead of basevertex_bo.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140805/6bbca59f/attachment.sig>