[Mesa-stable] [PATCH 14.1/15] glapi: Store exec table version info outside the XML

Ilia Mirkin imirkin at alum.mit.edu
Tue May 19 19:29:37 PDT 2015


On Tue, May 19, 2015 at 3:54 PM, Ian Romanick <idr at freedesktop.org> wrote:
> From: Ian Romanick <ian.d.romanick at intel.com>
>
> Currently on the functions that are exclusive to core-profile are
> implemented.  The remainder continue to live in the XML.  Additional
> functions can be moved later.
>
> The functions for GL_ARB_draw_indirect and GL_ARB_multi_draw_indirect
> are put in the dispatch table inside the VBO module, so they do not need
> to be moved over.
>
> The diff of src/mesa/main/api_exec.c before and after this patch is as
> expected.  All of the functions listed in apiexec.py moved out of a 'if
> (_mesa_is_desktop(ctx))' block into a new 'if (ctx->API ==
> API_OPENGL_CORE)' block.
>
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
> Cc: Dave Airlie <airlied at redhat.com>
> Cc: Ilia Mirkin <imirkin at alum.mit.edu>
> Cc: Dylan Baker <baker.dylan.c at gmail.com>
> Cc: "10.6" <mesa-stable at lists.freedesktop.org>
> ---
>  src/mapi/glapi/gen/Makefile.am   |   3 +-
>  src/mapi/glapi/gen/apiexec.py    | 142 +++++++++++++++++++++++++++++++++++++++
>  src/mapi/glapi/gen/gl_genexec.py |  54 ++++++++++++---
>  3 files changed, 187 insertions(+), 12 deletions(-)
>  create mode 100644 src/mapi/glapi/gen/apiexec.py
>
> diff --git a/src/mapi/glapi/gen/Makefile.am b/src/mapi/glapi/gen/Makefile.am
> index adebd5c..64bc2ff 100644
> --- a/src/mapi/glapi/gen/Makefile.am
> +++ b/src/mapi/glapi/gen/Makefile.am
> @@ -61,6 +61,7 @@ EXTRA_DIST= \
>         $(MESA_GLAPI_DIR)/glapi_x86-64.S \
>         $(MESA_GLAPI_DIR)/glapi_sparc.S \
>         $(COMMON_GLX) \
> +       apiexec.py \
>         gl_apitemp.py \
>         gl_enums.py \
>         gl_genexec.py \
> @@ -267,7 +268,7 @@ $(MESA_GLAPI_DIR)/glapi_sparc.S: gl_SPARC_asm.py $(COMMON)
>  $(MESA_DIR)/main/enums.c: gl_enums.py $(COMMON)
>         $(PYTHON_GEN) $< -f $(srcdir)/gl_and_es_API.xml > $@
>
> -$(MESA_DIR)/main/api_exec.c: gl_genexec.py $(COMMON)
> +$(MESA_DIR)/main/api_exec.c: gl_genexec.py apiexec.py $(COMMON)
>         $(PYTHON_GEN) $< -f $(srcdir)/gl_and_es_API.xml > $@
>
>  $(MESA_DIR)/main/dispatch.h: gl_table.py $(COMMON)
> diff --git a/src/mapi/glapi/gen/apiexec.py b/src/mapi/glapi/gen/apiexec.py
> new file mode 100644
> index 0000000..1a9f042
> --- /dev/null
> +++ b/src/mapi/glapi/gen/apiexec.py
> @@ -0,0 +1,142 @@
> +#!/usr/bin/env python2

I wouldn't put that line there (or make the file +x, which you didn't
as well). It's not an executable script.

> +
> +# Copyright (C) 2015 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.
> +
> +class exec_info():
> +    """Information relating GL APIs to a function.
> +
> +    Each of the four attributes of this class, compatibility, core, es1, and
> +    es2, specify the minimum API version where a function can possibly exist
> +    in Mesa.  The version is specified as an integer of (real GL version *
> +    10).  For example, glCreateProgram was added in OpenGL 2.0, so
> +    compatibility=20 and core=31.
> +
> +    If the attribute is None, then it cannot be supported by that
> +    API.  For example, glNewList was removed from core profiles, so
> +    compatibility=10 and core=None.
> +
> +    Each of the attributes that is not None must have a valid value.  The
> +    valid ranges are:
> +
> +        compatiblity: [10, 30]
> +        core: [31, )
> +        es1: [10, 11]
> +        es2: [20, )
> +
> +    These ranges are enforced by the constructor.

No harm in doing this, but in practice, only the es2 version is used
for anything, right?

> +    """
> +    def __init__(self, compatibility=None, core=None, es1=None, es2=None):
> +        if compatibility is not None:
> +            assert isinstance(compatibility, int)
> +            assert compatibility >= 10
> +            assert compatibility <= 30
> +
> +        if core is not None:
> +            assert isinstance(core, int)
> +            assert core >= 31
> +
> +        if es1 is not None:
> +            assert isinstance(es1, int)
> +            assert es1 == 10 or es1 == 11
> +
> +        if es2 is not None:
> +            assert isinstance(es2, int)
> +            assert es2 >= 20
> +
> +        self.compatibility = compatibility
> +        self.core = core
> +        self.es1 = es1
> +        self.es2 = es2
> +
> +functions = {
> +    # OpenGL 3.1 / GL_ARB_texture_buffer_object.  Mesa only exposes this
> +    # extension with core profile.
> +    "TexBuffer": exec_info(core=31),
> +
> +    # OpenGL 3.2 / GL_ARB_geometry_shader4.  Mesa does not support
> +    # GL_ARB_geometry_shader4, so OpenGL 3.2 is required.
> +    "FramebufferTexture": exec_info(core=32),
> +
> +    # OpenGL 4.0 / GL_ARB_gpu_shader_fp64.  The extension spec says:
> +    #
> +    #     "OpenGL 3.2 and GLSL 1.50 are required."
> +    "Uniform1d": exec_info(core=32),
> +    "Uniform2d": exec_info(core=32),
> +    "Uniform3d": exec_info(core=32),
> +    "Uniform4d": exec_info(core=32),
> +    "Uniform1dv": exec_info(core=32),
> +    "Uniform2dv": exec_info(core=32),
> +    "Uniform3dv": exec_info(core=32),
> +    "Uniform4dv": exec_info(core=32),
> +    "UniformMatrix2dv": exec_info(core=32),
> +    "UniformMatrix3dv": exec_info(core=32),
> +    "UniformMatrix4dv": exec_info(core=32),
> +    "UniformMatrix2x3dv": exec_info(core=32),
> +    "UniformMatrix2x4dv": exec_info(core=32),
> +    "UniformMatrix3x2dv": exec_info(core=32),
> +    "UniformMatrix3x4dv": exec_info(core=32),
> +    "UniformMatrix4x2dv": exec_info(core=32),
> +    "UniformMatrix4x3dv": exec_info(core=32),
> +    "GetUniformdv": exec_info(core=32),
> +
> +    # OpenGL 4.1 / GL_ARB_vertex_attrib_64bit.  The extension spec says:
> +    #
> +    #     "OpenGL 3.0 and GLSL 1.30 are required.
> +    #
> +    #     ARB_gpu_shader_fp64 (or equivalent functionality) is required."
> +    #
> +    # For Mesa this effectively means OpenGL 3.2 is required.  It seems
> +    # unlikely that Mesa will ever get support for any of the NV extensions
> +    # that add "equivalent functionality."
> +    "VertexAttribL1d": exec_info(core=32),
> +    "VertexAttribL2d": exec_info(core=32),
> +    "VertexAttribL3d": exec_info(core=32),
> +    "VertexAttribL4d": exec_info(core=32),
> +    "VertexAttribL1dv": exec_info(core=32),
> +    "VertexAttribL2dv": exec_info(core=32),
> +    "VertexAttribL3dv": exec_info(core=32),
> +    "VertexAttribL4dv": exec_info(core=32),
> +    "VertexAttribLPointer": exec_info(core=32),
> +    "GetVertexAttribLdv": exec_info(core=32),
> +
> +    # OpenGL 4.1 / GL_ARB_viewport_array.  The extension spec says:
> +    #
> +    #     "OpenGL 3.2 or the EXT_geometry_shader4 or ARB_geometry_shader4
> +    #     extensions are required."
> +    #
> +    # Mesa does not support either of the geometry shader extensions, so
> +    # OpenGL 3.2 is required.
> +    "ViewportArrayv": exec_info(core=32),
> +    "ViewportIndexedf": exec_info(core=32),
> +    "ViewportIndexedfv": exec_info(core=32),
> +    "ScissorArrayv": exec_info(core=32),
> +    "ScissorIndexed": exec_info(core=32),
> +    "ScissorIndexedv": exec_info(core=32),
> +    "DepthRangeArrayv": exec_info(core=32),
> +    "DepthRangeIndexed": exec_info(core=32),
> +    # GetFloati_v also GL_ARB_shader_atomic_counters
> +    # GetDoublei_v also GL_ARB_shader_atomic_counters
> +
> +    # OpenGL 4.3 / GL_ARB_texture_buffer_range.  Mesa can expose the extension
> +    # with OpenGL 3.1.
> +    "TexBufferRange": exec_info(core=31),
> +}
> diff --git a/src/mapi/glapi/gen/gl_genexec.py b/src/mapi/glapi/gen/gl_genexec.py
> index 4e76fe3..ab00e32 100644
> --- a/src/mapi/glapi/gen/gl_genexec.py
> +++ b/src/mapi/glapi/gen/gl_genexec.py
> @@ -29,6 +29,7 @@ import collections
>  import license
>  import gl_XML
>  import sys, getopt
> +import apiexec
>
>
>  exec_flavor_map = {
> @@ -175,18 +176,49 @@ class PrintCode(gl_XML.gl_print_base):
>                  raise Exception(
>                      'Unrecognized exec flavor {0!r}'.format(f.exec_flavor))
>              condition_parts = []
> -            if f.desktop:
> -                if f.deprecated:
> +            if f.name in apiexec.functions:
> +                ex = apiexec.functions[f.name]
> +                unconditional_count = 0
> +
> +                if ex.compatibility is not None:
>                      condition_parts.append('ctx->API == API_OPENGL_COMPAT')
> -                else:
> -                    condition_parts.append('_mesa_is_desktop_gl(ctx)')
> -            if 'es1' in f.api_map:
> -                condition_parts.append('ctx->API == API_OPENGLES')
> -            if 'es2' in f.api_map:
> -                if f.api_map['es2'] > 2.0:
> -                    condition_parts.append('(ctx->API == API_OPENGLES2 && ctx->Version >= {0})'.format(int(f.api_map['es2'] * 10)))
> -                else:
> -                    condition_parts.append('ctx->API == API_OPENGLES2')
> +                    unconditional_count += 1
> +
> +                if ex.core is not None:
> +                    condition_parts.append('ctx->API == API_OPENGL_CORE')
> +                    unconditional_count += 1
> +
> +                if ex.es1 is not None:
> +                    condition_parts.append('ctx->API == API_OPENGLES')
> +                    unconditional_count += 1
> +
> +                if ex.es2 is not None:
> +                    if ex.es2 > 20:
> +                        condition_parts.append('(ctx->API == API_OPENGLES2 && ctx->Version >= {0})'.format(ex.es2))
> +                    else:
> +                        condition_parts.append('ctx->API == API_OPENGLES2')
> +                        unconditional_count += 1
> +
> +                # If the function is unconditionally available in all four
> +                # APIs, then it is always available.  Replace the complex
> +                # tautology condition with "true" and let GCC do the right
> +                # thing.
> +                if unconditional_count == 4:
> +                    condition_parts = ['true']
> +            else:
> +                if f.desktop:
> +                    if f.deprecated:
> +                        condition_parts.append('ctx->API == API_OPENGL_COMPAT')
> +                    else:
> +                        condition_parts.append('_mesa_is_desktop_gl(ctx)')
> +                if 'es1' in f.api_map:
> +                    condition_parts.append('ctx->API == API_OPENGLES')
> +                if 'es2' in f.api_map:
> +                    if f.api_map['es2'] > 2.0:
> +                        condition_parts.append('(ctx->API == API_OPENGLES2 && ctx->Version >= {0})'.format(int(f.api_map['es2'] * 10)))
> +                    else:
> +                        condition_parts.append('ctx->API == API_OPENGLES2')
> +
>              if not condition_parts:
>                  # This function does not exist in any API.
>                  continue
> --
> 2.1.0
>

This all seems fine. I trust you on the function selection. However as
this is all subtle and easily breakable, could I convince you to write
a piglit test that verifies some of this behaviour? i.e. that
GetProcAddress fails for a few of those functions in a compat context
[and you can skip if the extensions are advertised, so that we don't
fail on blobs that provide a high-version GL]

With that, and the above nits,

Reviewed-by: Ilia Mirkin <imirkin at alum.mit.edu>


More information about the mesa-stable mailing list