[Mesa-dev] [PATCH v03 35/38] i965: Port gen4+ emit vertices code to genxml.

[Kenneth Graunke]

On Saturday, May 6, 2017 12:34:33 AM PDT Pohjolainen, Topi wrote:
> On Sat, May 06, 2017 at 12:30:05AM -0700, Kenneth Graunke wrote:
> > On Thursday, May 4, 2017 10:38:53 PM PDT Pohjolainen, Topi wrote:
> > > On Thu, May 04, 2017 at 12:16:53PM -0700, Rafael Antognolli wrote:
> > > > On Thu, May 04, 2017 at 11:19:24AM +0300, Pohjolainen, Topi wrote:
> > > > > On Mon, May 01, 2017 at 06:43:23PM -0700, Rafael Antognolli wrote:
> > [snip]
> > > > > > +   if (vs_prog_data->uses_drawid) {
> > > > > > +      struct GENX(VERTEX_ELEMENT_STATE) elem_state = {
> > > > > > +         .Valid = true,
> > > > > > +         .VertexBufferIndex = brw->vb.nr_buffers + 1,
> > > > > > +         .SourceElementFormat = ISL_FORMAT_R32_UINT,
> > > > > > +         .Component0Control = VFCOMP_STORE_SRC,
> > > > > > +         .Component1Control = VFCOMP_STORE_0,
> > > > > > +         .Component2Control = VFCOMP_STORE_0,
> > > > > > +         .Component3Control = VFCOMP_STORE_0,
> > > > > > +#if GEN_GEN < 5
> > > > > > +         .DestinationElementOffset = i * 4,
> > > > > 
> > > > > Same comment as further up.
> > > > 
> > > > So, if I understand this field correctly, it should be
> > > > brw->vb.nr_enabled + 1, which would mean the old code was wrong?
> > > 
> > > Hmm, yeah, it looks odd. On gen < 6 you end up with two elements having
> > > indices set to "brw->vb.nr_buffers" and "brw->vb.nr_buffers + 1" but both
> > > having .DestinationElementOffset == i * 4 == brw->vb.nr_buffers * 4.
> > > 
> > > And like you said, this is already the case in the original. Ken, can you
> > > explain this?
> > 
> > Looks pretty broken to me.  I suspect the if (needs_sgvs_element) block
> > should do i++ so that gl_DrawID is one more than the VID/IID element (if
> > it exists).  I don't know if those are the right values, but they
> > certainly shouldn't be the same if both are present.
> > 
> > My guess is that all the tests use GLSL 1.30, which we don't expose on
> > Gen4-4.5 without overrides, so this is just broken and nobody has noticed.
> 
> I was suspecting the same when I replied to Rafael in his follow-up patch.
> I'll be getting an ILK soon and I was already thinking of looking into this
> some more.
> 

You'd need a G965 or G45 for that - Ironlake is too new.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170506/756a264f/attachment.sig>