<div dir="ltr">On 19 October 2013 02:25, Timothy Arceri <<a href="mailto:t_arceri@yahoo.com.au" target="_blank">t_arceri@yahoo.com.au</a>> wrote: <div class="gmail_extra"><div class="gmail_quote"> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im"> On Fri, 2013-10-18 at 10:26 -0700, Paul Berry wrote: > On 5 October 2013 04:21, Timothy Arceri <<a href="mailto:t_arceri@yahoo.com.au">t_arceri@yahoo.com.au</a>> wrote: </div><div class="im">> General comment: it seems redundant to have "arb_arrays_of_arrays" in > the folder name and "array-of-array-" as a prefix to each parser test. > Can we remove "array-of-array-" from the parser test file names (so > for example > tests/spec/arb_arrays_of_arrays/compiler/array-of-array-uniform.vert > becomes tests/spec/arb_arrays_of_arrays/compiler/uniform.vert). </div>Sure. <div class="im"> > > Calling this test "new syntax" is confusing. All three of these > syntaxes are new: > > > vec4[3][2] a; > vec4[2] a[3]; > > vec4 a[3][2]; > > > I'd suggest a naming convention that just names the syntax elements in > the order they appear. So for instance: > > > vec4[3][2] a; // tested by *-array-array-var > > vec4[2] a[3]; // tested by *-array-var-array > > vec4 a[3][2]; // tested by *-var-array-array </div>Ok, yes I like this much better. I didn't really like the other naming much either I just copied it from some of the existing glsl tests that test for a fail against this format. <div class="im">> > > @@ -0,0 +1,17 @@ > +/* [config] > + * expect_result: pass > + * glsl_version: 1.20 > + * require_extensions: GL_ARB_arrays_of_arrays > + * [end config] > + */ > +#version 120 > +#extension GL_ARB_arrays_of_arrays: enable > + > +attribute vec4 vert; > + > +void foo(vec4 [2] x[2]); > + > +void main() > +{ > > > It would be nice to include a call to the function here in main(), so > that we can verify that the compiler's type checking logic correctly > deduces that the function can be called. </div>Sure. <div><div class="h5">> > > + gl_Position = vert; > +} > diff --git > a/tests/spec/arb_arrays_of_arrays/compiler/array-of-array-function-parameter-declaration.vert b/tests/spec/arb_arrays_of_arrays/compiler/array-of-array-function-parameter-declaration.vert > new file mode 100644 > index 0000000..60fec71 > --- /dev/null > +++ > b/tests/spec/arb_arrays_of_arrays/compiler/array-of-array-function-parameter-declaration.vert > @@ -0,0 +1,17 @@ > +/* [config] > + * expect_result: pass > + * glsl_version: 1.20 > + * require_extensions: GL_ARB_arrays_of_arrays > + * [end config] > + */ > +#version 120 > +#extension GL_ARB_arrays_of_arrays: enable > + > +attribute vec4 vert; > + > +void foo(vec4 x[2][3]); > + > +void main() > +{ > > > Same comment applies here. > > > + gl_Position = vert; > +} > > > Can we also add a test to verify that the following works: > > void foo(vec4[3][2] x); > > > (this is explicitly allowed by the spec). > > > A similar comment goes for all the other tests in this patch. </div></div>ok will do. <div class="im"> > > As a side note, are you planning to add other tests in subsequent > patches? Glancing at the spec, it seems like there's a lot more that > still needs to be tested. </div>Yes I plan to send patches to cover more scenarios. I have been playing around with Mesa trying to implement the extension when time permits and have had these tests sitting around for a while. I thought I would submit what I had to get some feedback before continuing work on more as it seems I will have to create a fairly large amount of tests, I'm glad I did as it would have been a pain to have to go back retrospectively and rename them all, but I'm sorry if I have wasted your time this was not my intention. I will try submit the remaining tests as a series for the compiler tests rather than sending in bits and pieces. </blockquote><div> </div><div>You weren't wasting my time at all. I wish more people would submit things in small batches and get feedback early. It makes the whole process run more smoothly.</div> <div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Thanks for the feedback. Tim </blockquote></div> </div></div>