[Piglit] [PATCH 1/2] generators/modules: Add GLSL types representations
dylan at pnwbakers.com
Mon Jun 13 18:30:57 UTC 2016
Quoting Andres Gomez (2016-06-13 09:35:29)
> On Fri, 2016-06-10 at 16:42 -0700, Dylan Baker wrote:
> > +class Scalar(Container):
> > + """A base class for Scalar types."""
> > + def __init__(self, name, contains):
> > + super(Scalar, self).__init__(name, is_scalar=True,
> > contains=contains)
> I would be passing rows=1 and columns=1. In the end, this is correct
> from my POV and avoids to have to be doing always (type.columns or 1)
> in the rest of the code ...
I went back and forth on that. It definitely makes this generator
simpler, but I couldn't decide if it was correct to have it be 1 instead
of None. But maybe you're right maybe 1 is better. I guess in the worst
case we can always change it back if 1 is wrong.
I'll change it to default to 1.
Thanks for reviewing this.
> > +class Vector(Container):
> > + """A base class for Vector Containers."""
> > + def __init__(self, name, rows, contains):
> > + assert isinstance(rows, int), 'rows must be an integer'
> > + assert rows in [2, 3, 4], 'vecs can only be 2-4 in GLSL'
> > +
> > + super(Vector, self).__init__(name, is_vector=True,
> > rows=rows,
> > + contains=contains)
> I would be passing columns=1 for the same reason than above.
> In any case, consider this a nit pick. You can ignore freely.
> Other than that, this is:
> Reviewed-by: Andres Gomez <agomez at igalia.com>
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 473 bytes
More information about the Piglit