On 19 October 2013 02:25, Timothy Arceri <[email protected]> wrote:
> > > On Fri, 2013-10-18 at 10:26 -0700, Paul Berry wrote: > > On 5 October 2013 04:21, Timothy Arceri <[email protected]> wrote: > > > > 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). > > Sure. > > > > > 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 > > 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. > > > > > > @@ -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. > > Sure. > > > > > > + 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. > > ok will do. > > > > > > 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. > > 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. > 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. > > Thanks for the feedback. > > Tim >
_______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
