[Piglit] [PATCH 1/4] arb_shader_precision: support scalar values in shader_runner_format

[Dylan Baker]

On Thu, Feb 19, 2015 at 11:17:05AM -0600, Micah Fedke wrote:
> 
> 
> On 02/19/2015 12:50 AM, Dylan Baker wrote:
> > On Thu, Feb 19, 2015 at 12:06:15AM -0600, Micah Fedke wrote:
> >> Prep work for using arrays of tolerances in the shader_test files.
> >>
> >> ---
> >>    generated_tests/gen_shader_precision_tests.py | 34
> >> +++++++++++++++++----------
> >>    1 file changed, 22 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/generated_tests/gen_shader_precision_tests.py
> >> b/generated_tests/gen_shader_precision_tests.py
> >> index 0d24a80..cfa5065 100644
> >> --- a/generated_tests/gen_shader_precision_tests.py
> >> +++ b/generated_tests/gen_shader_precision_tests.py
> >> @@ -68,6 +68,10 @@ trig_builtins = ('sin', 'cos', 'tan',
> >>                     'sinh', 'cosh', 'tanh',                   'asinh',
> >> 'acosh', 'atanh')
> >>    +def _is_sequence(arg):
> >> +    return (not hasattr(arg, "strip") and
> >> +            hasattr(arg, "__iter__"))
> >> +
> >
> > All of the built-in sequence types (list, set, frozenset, tuple) inherit
> > from one of the ABC's in collections, collections.Sequence is probably
> > the right one. Would using isinstance(arg, collections.Sequence) work?
> > This just feels...  abusive, definitely not idiomatic.
> 
> Oh shoot, is my stackexchange showing?  I would actually welcome some 
> instruction here.  I'm attempting to account for numpy's array types as 
> well as standard collections.

I haven't tested, but this should work:
isinstance(arg, (collection.Sequence, numpy.ndarray))

> 
> >
> >>    def make_indexers(signature):
> >>       """Build a list of strings which index into every possible
> >>       value of the result.  For example, if the result is a vec2,
> >> @@ -105,20 +109,26 @@ def shader_runner_format(values):
> >>        "probe rgba" command.  Bools are converted to 0's and 1's, and
> >>        values are separated by spaces.
> >>        """
> >> -    transformed_values = []
> >> -    retval = ''
> >> -    for value in values:
> >> -        if isinstance(value, (bool, np.bool_)):
> >> -            transformed_values.append(int(value))
> >> -        else:
> >> -            transformed_values.append(value)
> >> -    for x in transformed_values:
> >> -        if isinstance(x,np.float32):
> >> -            retval+=' {0}'.format('{0:1.8e}'.format(x))
> >> -        else:
> >> -            retval+=' {0}'.format(repr(x))
> >> +
> >> +    if _is_sequence(values):
> >> +        transformed_values = []
> >> +        retval = ''
> >> +        for value in values:
> >> +            if isinstance(value, (bool, np.bool_)):
> >> +                transformed_values.append(int(value))
> >> +            else:
> >> +                transformed_values.append(value)
> >> +        for x in transformed_values:
> >> +            if isinstance(x,np.float32):
> >> +                retval+=' {0}'.format('{0:1.8e}'.format(x))
> >> +            else:
> >> +                retval+=' {0}'.format(repr(x))
> >> +    else:
> >> +        retval = '{}'.format(values)
> >
> > What is values? I assume in this else block it's a string? If it is a
> > string then just setting retval = values should be sufficient.
> 
> values can be a float or a list.  Now that I look at this more 
> carefully, I could eliminate the bool handling altogether, and clean up 
> the nested format as well.  Would something more like this be clearer?
> 
> def shader_runner_format(values):
>      """Format the given values for use in a shader_runner "uniform" or
>      "probe rgba" command.  Values are separated by spaces.
>      """
> 
>      if _is_sequence(values):
>          retval = ''
>          for x in values:
>              if isinstance(x,np.float32) or isinstance(x,float):

you can pass a tuple of types to isinstance:
isinstance(x, (np.float32, float))

Also, are you sure you're always going to get float32 and not float64?

>                  retval+=' {0:1.8e}'.format(x)
>              else:
>                  retval+=' {0}'.format(repr(x))
>      else:

Adding an assert here would make this clearer

>          retval = '{0:1.8e}'.format(values)
> 
>      return retval
> 
> 
> >
> >> +
> >>        return retval
> >>    +
> >>    def main():
> >>        """ Main function """
> >>    -- 2.2.2
> >> _______________________________________________
> >> Piglit mailing list
> >> Piglit at lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/piglit
> 
> -- 
> 
> Micah Fedke
> Collabora Ltd.
> +44 1223 362967
> https://www.collabora.com/
> https://twitter.com/collaboraltd
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20150219/a04bc179/attachment.sig>