[Piglit] [PATCH] tests/spec: ARB_arrays_of_arrays compiler tests

[Paul Berry]

On 19 October 2013 02:25, Timothy Arceri <t_arceri at yahoo.com.au> wrote:

>
>
> On Fri, 2013-10-18 at 10:26 -0700, Paul Berry wrote:
> > On 5 October 2013 04:21, Timothy Arceri <t_arceri at yahoo.com.au> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20131019/11e8dc0c/attachment-0001.html>