[Piglit] [PATCH 4/5] generators/gen_vs_in_fp64: Adds support to specify the GL type in the VBO

[Andres Gomez]

On Wed, 2016-06-29 at 09:48 -0700, Dylan Baker wrote:
> Quoting Andres Gomez (2016-06-14 14:37:00)
> > Signed-off-by: Andres Gomez <agomez at igalia.com>
> > ---
> >  generated_tests/gen_vs_in_fp64.py                  | 266
> > +++++++++++++++++----
> >  .../gen_vs_in_fp64/regular.shader_test.mako        |  21 +-
> >  2 files changed, 222 insertions(+), 65 deletions(-)
> > 
> > diff --git a/generated_tests/gen_vs_in_fp64.py
> > b/generated_tests/gen_vs_in_fp64.py
> > index 88c4952..af7e81b 100644
> > --- a/generated_tests/gen_vs_in_fp64.py
> > +++ b/generated_tests/gen_vs_in_fp64.py
> > @@ -72,47 +72,165 @@ DOUBLE_NORMAL_VALUES   =
> > ['0xffefffffffffffff', # Negative maximum normalized
> >                            '0x4b1e35ed24eb6496', # +7.23401345e+53
> >                            '0x7fefffffffffffff'] # Positive maximum
> > normalized
> >  
> > -DSCALAR_TYPES          = [glsltypes.DOUBLE]
> > -
> > -DVEC_TYPES             = [glsltypes.DVEC2, glsltypes.DVEC3,
> > glsltypes.DVEC4]
> > -
> > -DMAT_TYPES             = [glsltypes.DMAT2, glsltypes.DMAT2X3,
> > glsltypes.DMAT2X4,
> > +FLOAT_POS_ZERO         = ['0x00000000'] #  0.0
> 
> Our python style is to put at least two spaces between the last
> interpreted character and the # for inline comments:
> ex: FLOAT_POS_ZERO         = ['0x00000000']  #  0.0

I will correct this.

[snip]

> The only other concern that I have is that this generator takes 45
> seconds after this patch to run, and it's not because any piece is
> particularly slow, its just the sheer number of tests this generator
> creates (~18000), which accounts for 1/3 of piglit's tests. While I
> appreciate the comprehensiveness of this approach, I wonder if it's
> more
> than we really want in standard piglit profiles, and whether we would
> be
> better off cutting this down to a subset and calling it good enough.

I think you are quite right on this.

In any case, this patch is not adding more tests than the ones that are
already there before it. The next patch in the series adds some more
tests, though.

I think I can reduce a lot the amount of tests used to check the
correct implementation of GL_ARB_vertex_attrib_64bit since it is true
that, probably, although all are different, many can be considered
redundant on the functionality checked.

WDYT about landing this series as is and I send a follow up patch to
reduce the amount of tests? Or you would rather change already this
series to include the reduction too?

Thanks for taking a look at this!

-- 

Br,

Andres