[Piglit] [PATCH 6/8] arb_gpu_shader_fp64: use generator to test in/out attributes
Dylan Baker
baker.dylan.c at gmail.com
Tue Mar 15 16:38:08 UTC 2016
Quoting Andres Gomez (2016-03-15 08:50:47)
> From: "Juan A. Suarez Romero" <jasuarez at igalia.com>
>
> Vertex shader inputs and fragment shader outputs can only be
> single-precision values.
>
> From GL_ARB_gpu_shader_fp64 spec:
>
> "Modify Section 4.3.4, Inputs, p. 31
>
> (modify third paragraph of the section, p. 31) ... Vertex shader inputs
> can only be single-precision floating-point scalars, vectors, or
> matrices, or signed and unsigned integers and integer vectors. Vertex
> shader inputs can also form arrays of these types, but not structures."
>
> "Modify Section 4.3.6, Outputs, p. 33
>
> (modify third paragraph of the section, p. 33) They can only be float,
> double, single- or double-precision floating-point vectors or matrices,
> signed or unsigned integers or integer vectors, or arrays or structures
> of any these.
>
> (modify last paragraph, p. 33) ... Fragment outputs can only be float,
> single-precision floating-point vectors, signed or unsigned integers or
> integer vectors, or arrays of these. ..."
>
> Contributed by Andres Gomez.
>
> Signed-off-by: Andres Gomez <agomez at igalia.com>
> ---
> generated_tests/CMakeLists.txt | 8 +
> generated_tests/gen_inout_fp64.py | 219 +++++++++++++++++++++
> .../templates/gen_inout_fp64/template.frag.mako | 15 ++
> .../templates/gen_inout_fp64/template.vert.mako | 15 ++
> .../gen_inout_fp64/vs-out-fs-in_template.mako | 25 +--
> .../execution/vs-out-fs-in-double.shader_test | 43 ----
> .../preprocessor/fs-output-double.frag | 23 ---
> .../preprocessor/vs-input-double.vert | 24 ---
> 8 files changed, 270 insertions(+), 102 deletions(-)
> create mode 100644 generated_tests/gen_inout_fp64.py
> create mode 100644 generated_tests/templates/gen_inout_fp64/template.frag.mako
> create mode 100644 generated_tests/templates/gen_inout_fp64/template.vert.mako
> rename tests/spec/arb_gpu_shader_fp64/execution/vs-out-fs-in-double-2.shader_test => generated_tests/templates/gen_inout_fp64/vs-out-fs-in_template.mako (56%)
> delete mode 100644 tests/spec/arb_gpu_shader_fp64/execution/vs-out-fs-in-double.shader_test
> delete mode 100644 tests/spec/arb_gpu_shader_fp64/preprocessor/fs-output-double.frag
> delete mode 100644 tests/spec/arb_gpu_shader_fp64/preprocessor/vs-input-double.vert
>
> diff --git a/generated_tests/CMakeLists.txt b/generated_tests/CMakeLists.txt
> index 0dc617b..d5698ae 100644
> --- a/generated_tests/CMakeLists.txt
> +++ b/generated_tests/CMakeLists.txt
> @@ -117,6 +117,13 @@ piglit_make_generated_tests(
> gen_constant_array_size_tests_fp64.py
> builtin_function_fp64.py)
> piglit_make_generated_tests(
> + inout_fp64.list
> + gen_inout_fp64.py
> + templates/gen_inout_fp64/template.frag.mako
> + templates/gen_inout_fp64/template.vert.mako
> + templates/gen_inout_fp64/vs-out-fs-in_template.mako
> + )
> +piglit_make_generated_tests(
> shader_precision_tests.list
> gen_shader_precision_tests.py
> builtin_function.py
> @@ -173,6 +180,7 @@ add_custom_target(gen-gl-tests
> interpolation-qualifier-built-in-variable.list
> builtin_uniform_tests_fp64.list
> constant_array_size_tests_fp64.list
> + inout_fp64.list
> shader_precision_tests.list
> shader_image_load_store_tests.list
> variable_index_read_tests.list
> diff --git a/generated_tests/gen_inout_fp64.py b/generated_tests/gen_inout_fp64.py
> new file mode 100644
> index 0000000..9b64ff7
> --- /dev/null
> +++ b/generated_tests/gen_inout_fp64.py
> @@ -0,0 +1,219 @@
> +# coding=utf-8
> +#
> +# Copyright © 2016 Intel Corporation
> +#
> +# Permission is hereby granted, free of charge, to any person obtaining a
> +# copy of this software and associated documentation files (the "Software"),
> +# to deal in the Software without restriction, including without limitation
> +# the rights to use, copy, modify, merge, publish, distribute, sublicense,
> +# and/or sell copies of the Software, and to permit persons to whom the
> +# Software is furnished to do so, subject to the following conditions:
> +#
> +# The above copyright notice and this permission notice (including the next
> +# paragraph) shall be included in all copies or substantial portions of the
> +# Software.
> +#
> +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> +# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> +# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> +# DEALINGS IN THE SOFTWARE.
> +
> +"""Generate in/out fp64 tests."""
> +
> +from __future__ import print_function, division, absolute_import
> +import optparse
> +import os
> +import sys
> +import itertools
> +
> +from templates import template_dir
> +from modules import utils
> +
> +TEMPLATES = template_dir(os.path.basename(os.path.splitext(__file__)[0]))
> +
> +def get_dir_name(ver, test_type):
> + """Returns the directory name to save tests given a GLSL version."""
> +
> + assert isinstance(ver, str)
> + assert isinstance(test_type, str)
> + if ver == '150':
> + feature_dir = 'arb_gpu_shader_fp64'
> + else:
> + feature_dir = 'glsl-' + ver[0] + '.' + ver[1:]
> +
> + return os.path.join('spec', feature_dir, test_type,
> + 'inout')
> +
> +
> +def get_config(ver):
> + """Returns the config string for compiler tests given a
> + GLSL version
> + """
> + assert isinstance(ver, str)
> + config = (' * expect_result: fail\n'
> + ' * glsl_version: {0}\n').format(ver[0] + '.' + ver[1:])
Think this would be clearer as: 'glsl_version: {}.{}\n'.format(ver[0], ver[1:])
There are some other instances of this too.
I'm also not sure that I like building the config block in the python
layer, I would personally prefer to see it done in the mako layer, I
think it would be cleaner there. This applies to all of the get_*
functions too.
> + if ver == '150':
> + config += ' * require_extensions: GL_ARB_gpu_shader_fp64\n'
> + return config
> +
> +
> +def get_require(ver):
> + """Returns the require string for shader runner tests
> + given a GLSL version
> + """
> + assert isinstance(ver, str)
> + require = 'GLSL >= {0}\n'.format(ver[0] + '.' + ver[1:])
> + if ver == '150':
> + require += 'GL_ARB_gpu_shader_fp64\n'
> + return require
> +
> +
> +def get_preprocessor(ver):
> + """Returns the preprocessor string for a test given a
> + GLSL version
> + """
> + assert isinstance(ver, str)
> + preprocessor = '#version {0}\n'.format(ver)
> + if ver == '150':
> + preprocessor += '#extension GL_ARB_gpu_shader_fp64 : require\n'
> + return preprocessor
> +
> +
> +def get_comments(ver, shader):
> + """Returns the comments string for compiler tests
> + given a GLSL version and shader stage
> + """
> + assert isinstance(ver, str)
> + assert shader in ('vert', 'frag')
> + if ver == '150':
> + if shader == 'frag':
> + return (
> + ' *\n'
> + ' * GL_ARB_gpu_shader_fp64 spec states:\n'
> + ' *\n'
> + ' * "Fragment outputs can only be float, single-precision\n'
> + ' * floating-point vectors, signed or unsigned integers or\n'
> + ' * integer vectors, or arrays of these."\n')
> + elif shader == 'vert':
> + return (
> + ' *\n'
> + ' * GL_ARB_gpu_shader_fp64 spec states:\n'
> + ' *\n'
> + ' * "Vertex shader inputs can only be single-precision\n'
> + ' * floating-point scalars, vectors, or matrices, or signed and\n'
> + ' * unsigned integers and integer vectors. Vertex shader inputs\n'
> + ' * can also form arrays of these types, but not structures."\n')
> +
> + return ''
> +
> +
> +def generate_inout(type_name, shader, ver, names_only):
> + """Generate in/out GLSL compilation tests."""
> +
> + assert isinstance(type_name, str)
> + assert shader in ('vert', 'frag')
> + assert isinstance(ver, str)
> + assert isinstance(names_only, bool)
> +
> + filename = os.path.join(
> + get_dir_name(ver, 'compiler'),
> + '{0}-{1}put-{2}.{3}'.format('fs' if shader == 'frag' else 'vs',
> + 'out' if shader == 'frag' else 'in',
> + type_name, shader))
> +
> + print(filename)
> +
> + if not names_only:
> + with open(filename, 'w') as test_file:
> + test_file.write(TEMPLATES.get_template(
> + 'template.{0}.mako'.format(shader)).render_unicode(
> + config=get_config(ver),
> + comments=get_comments(ver, shader),
> + preprocessor=get_preprocessor(ver),
> + type_name=type_name,
> + extra_params=',0.0' if type_name in ['dvec2', 'dvec3'] else ''))
> +
> +
> +def generate_pipe(type_name, ver, names_only):
> + """Generate in/out shader runner tests."""
> +
> + assert isinstance(type_name, str)
> + assert isinstance(ver, str)
> + assert isinstance(names_only, bool)
> +
> + filename = os.path.join(
> + get_dir_name(ver, 'execution'),
> + 'vs-out-fs-in-{0}.shader_test'.format(type_name))
> +
> + print(filename)
> +
> + if not names_only:
> + with open(filename, 'w') as test_file:
> + test_file.write(TEMPLATES.get_template(
> + 'vs-out-fs-in_template.mako').render_unicode(
> + require=get_require(ver),
> + preprocessor=get_preprocessor(ver),
> + type_name=type_name))
> +
> +
> +def all_inout_tests(names_only):
> + """Creates all the combinations for in/out compilation tests."""
> +
> + assert isinstance(names_only, bool)
> + type_names = ['double', 'dvec2', 'dvec3', 'dvec4',
> + 'dmat2', 'dmat2x3', 'dmat2x4',
> + 'dmat3x2', 'dmat3', 'dmat3x4',
> + 'dmat4x2', 'dmat4x3', 'dmat4']
> + shaders = ['frag', 'vert']
> + glsl_ver = ['150', '400']
> + for ver in glsl_ver:
> + utils.safe_makedirs(get_dir_name(ver, 'compiler'))
> +
> + for t_name, shader, ver in itertools.product(type_names, shaders, glsl_ver):
> + yield t_name, shader, ver, names_only
> +
> +
> +def all_pipe_tests(names_only):
> + """Creates all the combinations for in/out shader runnertests."""
> +
> + assert isinstance(names_only, bool)
> + type_names = ['double', 'dvec2', 'dvec3', 'dvec4']
> + glsl_ver = ['150', '400']
> + for ver in glsl_ver:
> + utils.safe_makedirs(get_dir_name(ver, 'execution'))
> +
> + for t_name, ver in itertools.product(type_names, glsl_ver):
> + yield t_name, ver, names_only
> +
> +
> +def main():
> + """Main function."""
> +
> + parser = optparse.OptionParser(
> + description="Generate in/out tests for fp64",
> + usage="usage: %prog [-h] [--names-only]")
> + parser.add_option(
> + '--names-only',
> + dest='names_only',
> + action='store_true',
> + help="Don't output files, just generate a list of filenames to stdout")
> +
> + (options, args) = parser.parse_args()
most of piglit uses argparse, and it also doesn't need the extra code
for extra arguments, or use of bool. Could we use argparse
instead?
> +
> + if len(args) != 0:
> + # User gave extra args.
> + parser.print_help()
> + sys.exit(1)
> +
> + for args in all_inout_tests(bool(options.names_only)):
> + generate_inout(*args)
> +
> + for args in all_pipe_tests(bool(options.names_only)):
> + generate_pipe(*args)
> +
> +
> +if __name__ == '__main__':
> + main()
> diff --git a/generated_tests/templates/gen_inout_fp64/template.frag.mako b/generated_tests/templates/gen_inout_fp64/template.frag.mako
> new file mode 100644
> index 0000000..3ba1c4e
> --- /dev/null
> +++ b/generated_tests/templates/gen_inout_fp64/template.frag.mako
> @@ -0,0 +1,15 @@
> +/* [config]
> +${config}\
> + * [end config]
> +${comments}\
> + */
> +
> +${preprocessor}\
> +
> +out ${type_name} color;
> +
> +void main()
> +{
> + color = ${type_name}(0.0);
> +}
> +
> diff --git a/generated_tests/templates/gen_inout_fp64/template.vert.mako b/generated_tests/templates/gen_inout_fp64/template.vert.mako
> new file mode 100644
> index 0000000..cb8db6d
> --- /dev/null
> +++ b/generated_tests/templates/gen_inout_fp64/template.vert.mako
> @@ -0,0 +1,15 @@
> +/* [config]
> +${config}\
> + * [end config]
> +${comments}\
> + */
> +
> +${preprocessor}\
> +
> +in ${type_name} vertex;
> +
> +void main()
> +{
> + gl_Position = vec4(vertex${extra_params});
> +}
> +
> diff --git a/tests/spec/arb_gpu_shader_fp64/execution/vs-out-fs-in-double-2.shader_test b/generated_tests/templates/gen_inout_fp64/vs-out-fs-in_template.mako
> similarity index 56%
> rename from tests/spec/arb_gpu_shader_fp64/execution/vs-out-fs-in-double-2.shader_test
> rename to generated_tests/templates/gen_inout_fp64/vs-out-fs-in_template.mako
> index 5c3ef95..db93c79 100644
> --- a/tests/spec/arb_gpu_shader_fp64/execution/vs-out-fs-in-double-2.shader_test
> +++ b/generated_tests/templates/gen_inout_fp64/vs-out-fs-in_template.mako
Most of the templates are named in the format:
<template>.<final type>.mako,
(which you did for the other templates), could you do that for this one
as well?
> @@ -1,34 +1,35 @@
> -# test emitting a single double from vs->fs works
> +# test emitting a ${type_name} from vs->fs works
> # when originally written this failed in varying lowering
>
> [require]
> -GLSL >= 1.50
> -GL_ARB_gpu_shader_fp64
> +${require}\
>
> [vertex shader]
> -#version 150
> -#extension GL_ARB_gpu_shader_fp64 : require
> +${preprocessor}\
>
> uniform double arg0;
> +
> in vec4 vertex;
> -flat out double dout1;
> +flat out ${type_name} dout1;
> +
> void main()
> {
> gl_Position = vertex;
> - dout1 = arg0;
> + dout1 = ${type_name}(arg0);
> }
>
> [fragment shader]
> -#version 150
> -#extension GL_ARB_gpu_shader_fp64 : require
> +${preprocessor}\
>
> -flat in double dout1;
> +flat in ${type_name} dout1;
> uniform double tolerance;
> uniform double expected;
> +out vec4 color;
> +
> void main()
> {
> - double result = trunc(dout1);
> - gl_FragColor = distance(result, expected) <= tolerance ? vec4(0.0, 1.0, 0.0, 1.0) : vec4(1.0, 0.0, 0.0, 1.0);
> + ${type_name} result = trunc(dout1);
> + color = distance(result, ${type_name}(expected)) <= tolerance ? vec4(0.0, 1.0, 0.0, 1.0) : vec4(1.0, 0.0, 0.0, 1.0);
> }
>
> [vertex data]
> diff --git a/tests/spec/arb_gpu_shader_fp64/execution/vs-out-fs-in-double.shader_test b/tests/spec/arb_gpu_shader_fp64/execution/vs-out-fs-in-double.shader_test
> deleted file mode 100644
> index c01db72..0000000
> --- a/tests/spec/arb_gpu_shader_fp64/execution/vs-out-fs-in-double.shader_test
> +++ /dev/null
> @@ -1,43 +0,0 @@
> -# test truncating a double holds precision
> -[require]
> -GLSL >= 1.50
> -GL_ARB_gpu_shader_fp64
> -
> -[vertex shader]
> -#version 150
> -#extension GL_ARB_gpu_shader_fp64 : require
> -uniform double arg0;
> -in vec4 vertex;
> -flat out dvec4 dout1;
> -void main()
> -{
> - gl_Position = vertex;
> - dout1 = dvec4(arg0);
> -}
> -
> -[fragment shader]
> -#version 150
> -#extension GL_ARB_gpu_shader_fp64 : require
> -#
> -flat in dvec4 dout1;
> -uniform double tolerance;
> -uniform double expected;
> -void main()
> -{
> - dvec4 result = trunc(dout1);
> - gl_FragColor = distance(result, dvec4(expected)) <= tolerance ? vec4(0.0, 1.0, 0.0, 1.0) : vec4(1.0, 0.0, 0.0, 1.0);
> -}
> -
> -[vertex data]
> -vertex/float/2
> --1.0 -1.0
> - 1.0 -1.0
> - 1.0 1.0
> --1.0 1.0
> -
> -[test]
> -uniform double arg0 1.7976931348623157E+308
> -uniform double expected 1.7976931348623157E+308
> -uniform double tolerance 2.0000000000000002e-05
> -draw arrays GL_TRIANGLE_FAN 0 4
> -probe rgba 0 0 0.0 1.0 0.0 1.0
> diff --git a/tests/spec/arb_gpu_shader_fp64/preprocessor/fs-output-double.frag b/tests/spec/arb_gpu_shader_fp64/preprocessor/fs-output-double.frag
> deleted file mode 100644
> index 111f01c..0000000
> --- a/tests/spec/arb_gpu_shader_fp64/preprocessor/fs-output-double.frag
> +++ /dev/null
> @@ -1,23 +0,0 @@
> -// [config]
> -// expect_result: fail
> -// glsl_version: 1.50
> -// require_extensions: GL_ARB_gpu_shader_fp64
> -// [end config]
> -//
> -// GL_ARB_gpu_shader_fp64 spec states:
> -//
> -// "Fragment outputs can only be float, single-precision
> -// floating-point vectors, signed or unsigned integers or
> -// integer vectors, or arrays of these."
> -//
> -
> -#version 150
> -#extension GL_ARB_gpu_shader_fp64: require
> -
> -out dvec4 color;
> -
> -void main()
> -{
> - color = dvec4(0.0, 0.0, 0.0, 0.0);
> -}
> -
> diff --git a/tests/spec/arb_gpu_shader_fp64/preprocessor/vs-input-double.vert b/tests/spec/arb_gpu_shader_fp64/preprocessor/vs-input-double.vert
> deleted file mode 100644
> index 930248b..0000000
> --- a/tests/spec/arb_gpu_shader_fp64/preprocessor/vs-input-double.vert
> +++ /dev/null
> @@ -1,24 +0,0 @@
> -// [config]
> -// expect_result: fail
> -// glsl_version: 1.50
> -// require_extensions: GL_ARB_gpu_shader_fp64
> -// [end config]
> -//
> -// GL_ARB_gpu_shader_fp64 spec states:
> -//
> -// "Vertex shader inputs can only be single-precision
> -// floating-point scalars, vectors, or matrices, or signed and
> -// unsigned integers and integer vectors. Vertex shader inputs
> -// can also form arrays of these types, but not structures."
> -//
> -
> -#version 150
> -#extension GL_ARB_gpu_shader_fp64: require
> -
> -in dvec4 vertex;
> -
> -void main()
> -{
> - gl_Position = vertex;
> -}
> -
> --
> 2.1.0
>
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/piglit
I don't really have the expertise to review the generated output, but
this looks pretty good from a python/mako point of view as is. There are
a few comments that I think would make it better, but I'm opened to
being convinced otherwise.
Also, thank you for using all of the helpers like safe_makedirs and
template_dir.
Dylan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: signature
URL: <https://lists.freedesktop.org/archives/piglit/attachments/20160315/eee1d8af/attachment.sig>
More information about the Piglit
mailing list