[Piglit] [PATCH 4/4 v2] generators: Add a simple generator for enabled and disabled defines

[Ilia Mirkin]

On Tue, Apr 12, 2016 at 6:15 PM, Dylan Baker <baker.dylan.c at gmail.com> wrote:
> Quoting Ilia Mirkin (2016-04-12 13:37:12)
>> On Tue, Apr 12, 2016 at 4:25 PM, Dylan Baker <baker.dylan.c at gmail.com> wrote:
>> > This replaces a large number of handrolled tests (and adds many new
>> > ones) for testing that preprocessor defines are exposed correctly
>> > (either exposed if the extension is enabled, or not if it's disabled)
>> > across all shader stages. It tests for support in legacy (non
>> > core/compat) mode and in profile mode, unless the extension itself
>> > requires GLSL >= 140. It also covers all stages (fs, vs, gs, tcs, tes,
>> > and cs), and both GLES and GL.
>> >
>> > It drives all of this from a simple, easily extended list in the
>> > generator, and replaces over 1000 lines of code with about 300,
>> > including the template files, and generates nearly 2000 tests currently.
>> > These are GLSL parser tests and don't take more than a few seconds to
>> > run.
>> >
>> > The only known issue is that an extension can not currently be tested in
>> > both GLES and OpenGL.
>> >
>> > Signed-off-by: Dylan Baker <dylanx.c.baker at intel.com>
>> > ---
>> >
>> > v2: - add a template for testing that when an extension isn't available
>> >       it cannot be included. (Ilia)
>> >
>> > +from __future__ import (
>> > +    absolute_import, division, print_function, unicode_literals
>> > +)
>> > +import os
>> > +
>> > +from templates import template_dir
>> > +from modules import utils, glsl
>> > +
>> > +_TEMPLATES = template_dir(os.path.basename(os.path.splitext(__file__)[0]))
>> > +ENABLED_TEMPLATE = _TEMPLATES.get_template('enabled.glsl.mako')
>> > +DISABLED_TEMPLATE = _TEMPLATES.get_template('disabled.glsl.mako')
>> > +UNDEFINED_TEMPLATE = _TEMPLATES.get_template('undefined-require.glsl.mako')
>> > +
>> > +# A list of tuples with the full preprocess defined name, and the minimum
>> > +# supported version of that extension.
>> > +EXTENSIONS = [
>> > +    ("GL_ARB_draw_instanced", "110"),
>> > +    ("GL_ARB_draw_buffers", "110"),
>> > +    ("GL_ARB_enhanced_layouts", "140"),
>> > +    ("GL_ARB_separate_shader_objects", "110"),
>> > +    ("GL_ARB_texture_rectangle", "110"),
>> > +    ("GL_AMD_shader_trinary_minmax", "110"),
>> > +    ("GL_EXT_texture_array", "110"),
>> > +    ("GL_ARB_ES3_1_compatibility", "110"),
>> > +    ("GL_ARB_arrays_of_arrays", "110"),
>> > +    ("GL_ARB_fragment_coord_conventions", "110"),
>> > +    ("GL_ARB_fragment_layer_viewport", "110"),
>> > +    ("GL_ARB_explicit_attrib_location", "110"),
>> > +    ("GL_ARB_explicit_uniform_location", "110"),
>> > +    ("GL_ARB_shader_texture_lod", "110"),
>> > +    ("GL_AMD_conservative_depth", "110"),
>> > +    ("GL_ARB_conservative_depth", "110"),
>> > +    ("GL_ARB_shader_bit_encodign", "110"),
>> > +    ("GL_ARB_shader_clock", "110"),
>> > +    ("GL_ARB_uniform_buffer_object", "110"),
>> > +    ("GL_ARB_texture_cube_map_array", "110"),
>> > +    ("GL_ARB_shading_language_packing", "110"),
>> > +    ("GL_ARB_texture_multisample", "110"),
>> > +    ("GL_ARB_texture_query_levels", "110"),
>> > +    ("GL_ARB_texture_query_lod", "110"),
>> > +    ("GL_ARB_gpu_shader5", "110"),
>> > +    ("GL_ARB_gpu_shader_fp64", "150"),
>> > +    ("GL_ARB_vertex_attrib_64bit", "150"),
>> > +    ("GL_AMD_vertex_shader_layer", "110"),
>> > +    ("GL_AMD_vertex_shader_viewport_index", "110"),
>> > +    ("GL_ARB_shading_language_420pack", "110"),
>> > +    ("GL_ARB_sample_shading", "110"),
>> > +    ("GL_ARB_texture_gather", "110"),
>> > +    ("GL_ARB_shader_atomic_counters", "110"),
>> > +    ("GL_ARB_shader_atomic_counter_ops", "140"),
>> > +    ("GL_ARB_viewport_array", "110"),
>> > +    ("GL_ARB_compute_shader", "110"),
>> > +    ("GL_ARB_shader_image_load_store", "110"),
>> > +    ("GL_ARB_shader_image_size", "110"),
>> > +    ("GL_ARB_shader_texture_image_samples", "110"),
>> > +    # That is what the original hand written test required.
>> > +    ("GL_ARB_derivative_control", "150"),
>> > +    ("GL_ARB_shader_precision", "110"),
>> > +    ("GL_ARB_shader_storage_buffer_object", "110"),
>> > +    ("GL_ARB_tessellation_shader", "150"),
>> > +    ("GL_ARB_shader_subroutine", "150"),
>> > +    ("GL_ARB_shader_draw_parameters", "140"),
>> > +    ("GL_EXT_separate_shader_objects", "100"),
>> > +    ("GL_EXT_draw_buffers", "100"),
>> > +    ("GL_AMD_shader_stencil_export", "120"),
>> > +    ("GL_ARB_shader_stencil_export", "120"),
>> > +    ("GL_ARB_geometry_shader4", "110"),
>> > +    ("GL_OES_EGL_image_external", "100"),
>> > +    ("GL_EXT_shader_samples_identical", "110"),
>> > +    ("GL_EXT_shader_samples_identical", "310 es"),
>> > +    ("GL_OES_sample_variables", "300 es"),
>> > +    ("GL_OES_multisample_interpolation", "300 es"),
>> > +    ("GL_OES_standard_derivatives", "100"),
>> > +    ("GL_OES_texture_storage_multisample_2d_array", "300 es"),
>> > +    ("GL_OES_blend_func_extended", "100"),
>> > +    ("GL_OES_shader_image_atomic", "310 es"),
>> > +    ("GL_OES_geometry_shader", "310 es"),
>> > +    ("GL_OES_geometry_point_size", "310 es"),
>> > +    ("GL_EXT_gpu_shader5", "310 es"),
>> > +    ("GL_OES_gpu_shader5", "310 es"),
>> > +    ("GL_EXT_texture_buffer", "310 es"),
>> > +    ("GL_OES_texture_buffer", "310 es"),
>> > +]
>> > +EXTENSIONS = [(n, glsl.Version(v)) for n, v in EXTENSIONS]
>> > +
>> > +_EXTRA_REQUIRES = {
>> > +    'tesc': ['GL_ARB_tessellation_shader'],
>> > +    'tese': ['GL_ARB_tessellation_shader'],
>> > +    'comp': ['GL_ARB_compute_shader'],
>> > +}
>> > +
>> > +
>> > +def _gen_tests(ext, ver, stage, path, extra_name=''):
>> > +    """Generate the actual test files.
>> > +
>> > +    This generates both a disabled test for both disabled and enabled
>> > +    configurations.
>> > +
>> > +    Argments:
>> > +    ext -- the extension to be tested
>> > +    ver -- the minimum required version
>> > +    stage -- the shader stage to test
>> > +    path -- the path to put the test in, without the test name
>> > +
>> > +    Keyword Arguments:
>> > +    extra_name -- An extra string to put in the test name. Default: ''
>> > +
>> > +    """
>> > +    extra_extensions = _EXTRA_REQUIRES.get(stage, [])
>>
>> The extra extensions you have only apply to desktop GL, not ES. You
>> should be able to do compute with ES 3.10, for example. You don't
>> really distinguish those below though... as it is, you're generating a
>> ton of unrunnable ES tests.
>>
>> Geometry and tess will be supportable in ES 3.10+ as well, given that
>> GL_EXT/OES_tessellation/geometry_shader are available. (Which they are
>> not today, but will be eventually.)
>
> You're right. I apparently left that bit half finished. I'll spin a v3
> later that puts the OES requirements in ES shaders.
>
>>
>> > +    version = glsl.MinVersion.for_stage(stage, ver)
>> > +
>> > +    for test, template in [('enabled', ENABLED_TEMPLATE),
>> > +                           ('disabled-defined', DISABLED_TEMPLATE),
>> > +                           ('disabled-undefined', UNDEFINED_TEMPLATE)]:
>> > +        name = os.path.join(path, '{}{}.{}'.format(test, extra_name, stage))
>> > +
>> > +        # Open in bytes mode to avoid weirdness in python 2/3 compatibility
>> > +        with open(name, 'wb') as f:
>> > +            f.write(template.render(
>> > +                version=version,
>> > +                extension=ext,
>> > +                extra_extensions=extra_extensions))
>> > +        print(name)
>> > +
>> > +
>> > +def main():
>> > +    """Main function."""
>> > +    for ext, ver in EXTENSIONS:
>> > +        # Lower ext in the path name, but keep it capitalized for the generated
>> > +        # tests
>> > +        path = os.path.join('spec', ext[3:].lower(), 'preprocessor')
>> > +        utils.safe_makedirs(path)
>> > +
>> > +        for stage in ['vert', 'frag', 'geom', 'tesc', 'tese', 'comp']:
>> > +            # If the version is less than 140, generate two tests. One for
>> > +            # legacy (without profiles) and one for profiles.
>> > +            if not ver.is_es and ver < 140:
>> > +                _gen_tests(ext, ver, stage, path, extra_name='-legacy')
>> > +                _gen_tests(ext, glsl.Version('140'), stage, path,
>> > +                           extra_name='-profile')
>>
>> -core makes more sense IMHO.
>
> I thought about core, but is there any reasons these tests don't apply
> to compat as well?

You have -legacy (aka "compat"), and then you have "-profile", which I
think makes more sense as "-core". "legacy" vs "compat" - I don't
really care. But "profile" is really the wrong term for it...

  -ilia

>
>>
>> > +            else:
>> > +                _gen_tests(ext, ver, stage, path)
>> > +
>> > +
>> > +if __name__ == '__main__':
>> > +    main()