On 5 September 2013 10:42, Ian Romanick <[email protected]> wrote:

> On 09/03/2013 08:28 AM, Paul Berry wrote:
> > On 29 August 2013 09:05, Ian Romanick <[email protected]
> > <mailto:[email protected]>> wrote:
> >
> >     From: Ian Romanick <[email protected]
> >     <mailto:[email protected]>>
>
> [snip]
>
> >     +interpolation_modes = {
> >     +    'flat',
> >     +    'noperspective',
> >     +    'smooth',
> >     +    'default'
> >     +}
> >
> >
> > This uses Python's "set literal" syntax, which was introduced in Python
> > 2.7.  Piglit only requires 2.6, so I think we should change this to a
> list:
> >
> > interpolation_modes = [
> >     'flat',
> >     'noperspective',
> >     'smooth',
> >     'default'
> > ]
> >
> > Same with vertex_shader_variables and vertex_shader_variables_front_only.
> >
> > All of my other comments (below) are minor, so with this fixed, the
> > patch is:
> >
> > Reviewed-by: Paul Berry <[email protected]
> > <mailto:[email protected]>>
> >
> >
> >     +
> >     +vertex_shader_variables = {
> >     +    'gl_FrontColor',
> >     +    'gl_BackColor',
> >     +    'gl_FrontSecondaryColor',
> >     +    'gl_BackSecondaryColor'
> >     +}
> >     +
> >     +vertex_shader_variables_front_only = {
> >     +    'gl_FrontColor',
> >     +    'gl_FrontSecondaryColor',
> >     +}
> >     +
> >     +vertex_shader_variables_other_side = {
> >     +    'gl_FrontColor': 'gl_BackColor',
> >     +    'gl_BackColor': 'gl_FrontColor',
> >     +    'gl_FrontSecondaryColor': 'gl_BackSecondaryColor',
> >     +    'gl_BackSecondaryColor': 'gl_FrontSecondaryColor'
> >     +}
> >     +
> >     +fragment_shader_variables = {
> >     +    'gl_FrontColor': 'gl_Color',
> >     +    'gl_BackColor': 'gl_Color',
> >     +    'gl_FrontSecondaryColor': 'gl_SecondaryColor',
> >     +    'gl_BackSecondaryColor': 'gl_SecondaryColor'
> >     +}
> >
> >
> > Minor quibble: vertex_shader_variables_other_side and
> > fragment_shader_variables sound like names of lists, when they're in
> > fact mappings from one variable name to another.  Maybe rename to
> > something like "other_side_map" and "vs_to_fs_map"?
>
> Should I still use the { } syntax here?  Does that work in Python 2.6?
>
>
Yeah, the { } syntax is fine here, since these are dict literals, which
have been supported for ages.
_______________________________________________
Piglit mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/piglit

Reply via email to