[Mesa-stable] [PATCH 14.1/15] glapi: Store exec table version info outside the XML
Ian Romanick
idr at freedesktop.org
Wed May 20 20:22:22 PDT 2015
On 05/19/2015 07:29 PM, Ilia Mirkin wrote:
> 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?
For now. There are quite a few functions that are only added by a GL
version that currently either incorrectly don't check the version (e.g.,
everything in src/mesa/main/shaderapi.c) or have spurious checks that
could be removed.
>> + """
>> + 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]
Unfortunately, that would be a large amount of work, but I'd love to see
this kind of testing happen.
glXGetProcAddress will *never* return NULL. If you call
glXGetProcAddress on a function libGL doesn't know, it will generate a
new dispatch function. Since you can call glXGetProcAddress before you
create a context, libGL has no way to know what the driver will support.
Even if you have a context, you could create another context on a
different GPU with different capabilities. If the driver never puts
anything in that new location in the dispatch table, calling the
function returned by glXGetProcAddress may crash.
A hypothetical piglit test would need to:
1. Call glXGetProcAddress directly without Waffle or some other layer
"helping." If NULL is returned (as on all the closed-source drivers),
the test passes.
2. Call the function. If GL_INVALID_OPERATION is generated, the test
passes. If the test crashes, the test passes. That last bit is the
hard part. :)
I would really like to have tests like this for a lot of different
functions. I first started thinking about it when we removed the
ARB_vertex_program / ARB_fragment_program functions from core profile.
That said, dispatch_sanity can cover most of the useful things. I have
some patches on the way out...
> With that, and the above nits,
>
> Reviewed-by: Ilia Mirkin <imirkin at alum.mit.edu>
> _______________________________________________
> mesa-stable mailing list
> mesa-stable at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-stable
>
More information about the mesa-stable
mailing list