On Tue, 2016-05-31 at 17:26 -0400, Ilia Mirkin wrote:
> On Tue, May 31, 2016 at 5:09 PM, Ian Romanick <i...@freedesktop.org>
> wrote:
> >
> > On 05/31/2016 01:48 PM, Ian Romanick wrote:
> > >
> > > On 05/31/2016 11:56 AM, Ilia Mirkin wrote:
> > > >
> > > > On Tue, May 31, 2016 at 2:52 PM, Ian Romanick <idr@freedesktop.
> > > > org> wrote:
> > > > >
> > > > > From: Ian Romanick <ian.d.roman...@intel.com>
> > > > >
> > > > > Issue #16 of the GL_ARB_program_interface_query makes it
> > > > > pretty clear
> > > > > array shader inputs of gl_PerVertex blocks should be reported
> > > > > as
> > > > > gl_PerVertex.gl_Foo. Piglit tests were recently changed to
> > > > > expect
> > > > > this behavior, and this change makes those tests pass again.
> > > > I'd like to note that you're now removing arrays of *all*
> > > > interfaces,
> > > > not just gl_PerVertex. I suspect this is OK, but just want to
> > > > double-check. Assuming that was your intent,
> > > It was the intent when I wrote it, but I wasn't sure whether or
> > > not it
> > > was correct. I ran it through piglit, CTS, and dEQP, and there
> > > were no
> > > changes. I was intending to go dig deeper in the spec, but I
> > > forgot.
> > > I'll double check that.
> > Right... so here's the weird thing. I feel like it's all
> > junk. The
> > ARB_piq spec clearly says:
> >
> > * For an active interface block declared as an array of
> > instances,
> > separate entries will be generated for each active
> > instance. The name
> > of the instance is formed by concatenating the block name,
> > the "["
> > character, an integer identifying the instance number, and
> > the "]"
> > character.
> >
> > By my reading, even gl_PerVertex stuff should come through as
> > gl_PerVertex[0].gl_Foo through gl_PerVertex[#].gl_Foo. If that's
> > true,
> > then even the dEQP tests are bunk.
> >
> > Before we would return BlockName[#array_size].foo, and that is
> > definitely wrong. Now we'll return BlockName.foo. This possibly
> > not
> > right, but I don't think it's worse than what we did before... but
> > we
> > now do what dEQP expects for gl_PerVertex.
> >
> > I think I want to push this patch with some of the commentary above
> > added to the commit message. How does that sound?
> Looking closer, I think you're right - issue 16 says:
>
> (16) How are inputs and outputs in the built-in interface block
> "gl_PerVertex" enumerated?
>
> RESOLVED: We will follow the default rule for enumerating
> block members
> in the OpenGL API, which is:
>
> * If a variable is a member of an interface block without an
> instance
> name, it is enumerated using just the variable name.
>
> * If a variable is a member of an interface block with an
> instance
> name, it is enumerated as "BlockName.Member", where
> "BlockName" is
> the name of the interface block (not the instance name) and
> "Member"
> is the name of the variable.
>
> For example, in the following code:
>
> uniform Block1 {
> int member1;
> };
> uniform Block2 {
> int member2;
> } instance2;
> uniform Block3 {
> int member3;
> } instance3[2]; // uses two separate buffer bindings
>
> the three uniforms (if active) are enumerated as "member1",
> "Block2.member2", and "Block3.member3".
>
> However the ACTUAL rules for enumerating block members are:
>
> * For an active interface block not declared as an array of
> block
> instances, a single entry will be generated, using the block
> name from
> the shader source.
>
> * For an active interface block declared as an array of
> instances,
> separate entries will be generated for each active
> instance. The name
> of the instance is formed by concatenating the block name,
> the "["
> character, an integer identifying the instance number, and
> the "]"
> character.
>
> So I think in that example, it should be Block1.member1, and
> Block3[0].member3 + Block3[1].member3?
>
> Anyways, this is all very confusing to me. I'd like to withdraw my
> R-b, since I clearly don't understand what's going on. Hopefully a
> more experienced spec reader will be able to do a real review.
Yeah this seems like an oversight to me. I was looking at this the
other day and it seemed to me we should be using or at least doing
something like how the names are generated in the
program_resource_visitor in link_uniforms.cpp which is used for both
UBO's and interface blocks.
I also added create_xfb_varying_names() a while back too which does a
similar thing.
The code in add_shader_variable() that deals with structs to me also
looks like it should be taking into account arrays.
>
> Cheers,
>
> -ilia
> _______________________________________________
> mesa-stable mailing list
> mesa-sta...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-stable
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
