[Mesa-dev] [PATCH] EGL: Implement the libglvnd interface for EGL.
Kyle Brenneman
kyle.brenneman at gmail.com
Wed Jan 4 18:34:06 UTC 2017
I've got an updated patch out.
Note that the Python scripts were largely unmodified from the ones in
the libglvnd tree. I don't know if divergence between the two is
something anyone is likely to care about, though, and I'd expect the
EGL-specific changes to libglvnd's version to be pretty minimal.
Other responses inline.
On 09/12/2016 03:32 PM, Dylan Baker wrote:
> Quoting Kyle Brenneman (2016-09-12 11:59:32)
>> Added separate --enable-libglvnd-glx and --enable-libglvnd-egl configure
>> options to enable libglvnd for GLX and EGL. The existing --enable-libglvnd
>> option will now enable both EGL and GLX.
>>
>> The new interface mostly just sits on top of the existing library. The only
>> change to the existing EGL code is to split the client extension string into
>> platform extensions and everything else. On non-glvnd builds, eglQueryString
>> will just concatenate the two strings.
>>
>> The EGL dispatch stubs are all generated. The script is based on the one used
>> to generate entrypoints in libglvnd itself.
>> ---
>> configure.ac | 161 ++-
>> src/egl/Makefile.am | 65 +-
>> src/egl/generate/egl.xml | 2412 ++++++++++++++++++++++++++++++++++
>> src/egl/generate/eglFunctionList.py | 191 +++
>> src/egl/generate/egl_other.xml | 47 +
>> src/egl/generate/genCommon.py | 223 ++++
>> src/egl/generate/gen_egl_dispatch.py | 223 ++++
>> src/egl/main/50_mesa.json.in | 6 +
>> src/egl/main/eglapi.c | 6 +-
>> src/egl/main/egldispatchstubs.c | 121 ++
>> src/egl/main/egldispatchstubs.h | 26 +
>> src/egl/main/eglglobals.c | 44 +-
>> src/egl/main/eglglobals.h | 13 +-
>> src/egl/main/eglglvnd.c | 75 ++
>> 14 files changed, 3525 insertions(+), 88 deletions(-)
>> create mode 100755 src/egl/generate/egl.xml
>> create mode 100644 src/egl/generate/eglFunctionList.py
>> create mode 100644 src/egl/generate/egl_other.xml
>> create mode 100644 src/egl/generate/genCommon.py
>> create mode 100755 src/egl/generate/gen_egl_dispatch.py
>> create mode 100644 src/egl/main/50_mesa.json.in
>> create mode 100644 src/egl/main/egldispatchstubs.c
>> create mode 100644 src/egl/main/egldispatchstubs.h
>> create mode 100644 src/egl/main/eglglvnd.c
>>
>> diff --git a/src/egl/generate/eglFunctionList.py b/src/egl/generate/eglFunctionList.py
>> new file mode 100644
>> index 0000000..39d3dd5
>> --- /dev/null
>> +++ b/src/egl/generate/eglFunctionList.py
>> @@ -0,0 +1,191 @@
>> +#!/usr/bin/python
> Is this a script or a module? It looks more like a module. If it's a
> module please drop the chbang, other wise please change it to
> '/usr/bin/env python2'. Archlinux has made and stuck to a very bad
> decisions of having 'python' point to 'python3' instead of 'python2'
> like upstream suggests. Archlinux is a big enough consumer we need to
> cater to this, and we have't seen any breakages as a result.
eglFunctionList.py and genCommon.py are both modules, so I'll take the
chbangs out.
As for the Python version, the script in the updated patch will work
correctly with Python 2 or 3. The command line in the Makefile includes
the Python executable anyway, so the chbang is more documentation than
anything else.
>> +
>> +"""
>> +Contains a list of EGL functions to generate dispatch functions for.
>> +
>> +This is used from gen_egl_dispatch.py.
>> +
>> +EGL_FUNCTIONS is a sequence of (name, eglData) pairs, where name is the name
>> +of the function, and eglData is a dictionary containing data about that
>> +function.
>> +
>> +The values in the eglData dictionary are:
>> +- method (string):
>> + How to select a vendor library. See "Method values" below.
>> +
>> +- prefix (string):
>> + This string is prepended to the name of the dispatch function. If
>> + unspecified, the default is "" (an empty string).
>> +
>> +- static (boolean)
>> + If True, this function should be declared static.
>> +
>> +- "public" (boolean)
>> + If True, the function should be exported from the library. Vendor libraries
>> + generally should not use this.
>> +
>> +- extension (string):
>> + If specified, this is the name of a macro to check for before defining a
>> + function. Used for checking for extension macros and such.
>> +
>> +- retval (string):
>> + If specified, this is a C expression with the default value to return if we
>> + can't find a function to call. By default, it will try to guess from the
>> + return type: EGL_NO_whatever for the various handle types, NULL for
>> + pointers, and zero for everything else.
>> +
>> +method values:
>> +- "custom"
>> + The dispatch stub will be hand-written instead of generated.
>> +
>> +- "none"
>> + No dispatch function exists at all, but the function should still have an
>> + entry in the index array. This is for other functions that a stub may need
>> + to call that are implemented in libEGL itself.
>> +
>> +- "display"
>> + Select a vendor from an EGLDisplay argument.
>> +
>> +- "device"
>> + Select a vendor from an EGLDeviceEXT argument.
>> +
>> +- "current"
>> + Select the vendor that owns the current context.
>> +"""
>> +
>> +def _eglFunc(name, method, static=False, public=False, inheader=None, prefix="", extension=None, retval=None):
>> + """
>> + A convenience function to define an entry in the EGL function list.
>> + """
>> + if (inheader == None):
>> + inheader = (not public)
> Simplify this to: 'inheader = inheader or not public'
That changes the semantics if (inheader == False) instead of (inheader
== None). The point of using None is that lets the caller explicitly
specify True or False, but gives a convenient default if the caller
doesn't specify either.
> +)
> +
>
> diff --git a/src/egl/generate/genCommon.py b/src/egl/generate/genCommon.py
> new file mode 100644
> index 0000000..5781275
> --- /dev/null
> +++ b/src/egl/generate/genCommon.py
> @@ -0,0 +1,223 @@
> +#!/usr/bin/env python
> The same as the last chbang
>
>> +
>> +# (C) Copyright 2015, NVIDIA CORPORATION.
>> +# All Rights Reserved.
>> +#
>> +# 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
>> +# on the rights to use, copy, modify, merge, publish, distribute, sub
>> +# license, 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 NON-INFRINGEMENT. IN NO EVENT SHALL
>> +# IBM AND/OR ITS SUPPLIERS 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.
>> +#
>> +# Authors:
>> +# Kyle Brenneman <kbrenneman at nvidia.com>
>> +
>> +import sys
>> +import collections
>> +import re
>> +import xml.etree.cElementTree as etree
> Please sort the above imports
>
>> +def getFunctionsFromRoots(roots):
>> + functions = {}
>> + for root in roots:
>> + for func in _getFunctionList(root):
>> + functions[func.name] = func
>> + functions = functions.values()
>> +
>> + # Sort the function list by name.
>> + functions = sorted(functions, key=lambda f: f.name)
> This avoids creating a dictionary, just to turn it into a list,
> and avoids building any intermediate data structures.
>
> functions = sorted((f for f in (_getFunctionList(r) for r in roots))),
> key=lambda f: f.name)
That's not quite equivalent. Using the dictionary step ensures that the
resulting list doesn't have any duplicate entries in it.
>> +
>> + # Assign a slot number to each function. This isn't strictly necessary,
>> + # since you can just look at the index in the list, but it makes it easier
>> + # to include the slot when formatting output.
>> + for i in xrange(len(functions)):
>> + functions[i] = functions[i]._replace(slot=i)
> I really don't like relying on protected methods like this. It's asking
> for trouble
The _replace method isn't protected -- it's part of the spec for
collections.namedtuple. The leading underscore is just to avoid name
conflicts with the actual member names:
https://docs.python.org/2.7/library/collections.html#collections.somenamedtuple._replace
>
>> +
>> + return functions
>> +
>> +def getExportNamesFromRoots(target, roots):
>> + """
>> + Goes through the <feature> tags from gl.xml and returns a set of OpenGL
>> + functions that a library should export.
>> +
>> + target should be one of "gl", "gldispatch", "opengl", "glesv1", or
>> + "glesv2".
>> + """
>> + featureNames = _LIBRARY_FEATURE_NAMES[target]
>> + if (featureNames == None):
> 'if featureNames is None'. Use is with singletons like 'None, False,
> True, etc'
>
>> + return set(func.name for func in getFunctionsFromRoots(roots))
>> +
>> + names = set()
>> + for root in roots:
>> + features = []
>> + for featElem in root.findall("feature"):
>> + if (featElem.get("name") in featureNames):
> It is really not idomatic to use parans in if statments in python, and
> it doesn't seem like we do that much in mesa, so could we please drop
> them?
>
>> + features.append(featElem)
>> + for featElem in root.findall("extensions/extension"):
>> + if (featElem.get("name") in featureNames):
>> + features.append(featElem)
>> + for featElem in features:
>> + for commandElem in featElem.findall("require/command"):
>> + names.add(commandElem.get("name"))
>> + return names
>> +
>> +class FunctionArg(collections.namedtuple("FunctionArg", "type name")):
>> + @property
>> + def dec(self):
>> + """
>> + Returns a "TYPE NAME" string, suitable for a function prototype.
>> + """
>> + rv = str(self.type)
>> + if(not rv.endswith("*")):
> 'if not rv.endswith...
>
>> + rv += " "
>> + rv += self.name
>> + return rv
>> +
>> +class FunctionDesc(collections.namedtuple("FunctionDesc", "name rt args slot")):
>> + def hasReturn(self):
>> + """
>> + Returns true if the function returns a value.
>> + """
>> + return (self.rt != "void")
> No need for parens here either
>
>> +
>> + @property
>> + def decArgs(self):
>> + """
>> + Returns a string with the types and names of the arguments, as you
>> + would use in a function declaration.
>> + """
>> + if(len(self.args) == 0):
> 'if not self.args'
>
>> + return "void"
>> + else:
>> + return ", ".join(arg.dec for arg in self.args)
>> +
>> + @property
>> + def callArgs(self):
>> + """
>> + Returns a string with the names of the arguments, as you would use in a
>> + function call.
>> + """
>> + return ", ".join(arg.name for arg in self.args)
>> +
>> + @property
>> + def basename(self):
>> + assert(self.name.startswith("gl"))
> Don't use parens with assert. Assert is a keyword in python, so using
> parens doesn't do what you expect.
>
> If you care to know why:
> Since assert's signature is 'assert <test>, <message>' someone will come
> along and extend this with a message 'assert(<test>, <message>), and
> then things fail spectacularly because you've passed assert a tuple with
> a string in it, so it now *always* evaluates True.
>
>> diff --git a/src/egl/generate/gen_egl_dispatch.py b/src/egl/generate/gen_egl_dispatch.py
>> new file mode 100755
>> index 0000000..1e145aa
>> --- /dev/null
>> +++ b/src/egl/generate/gen_egl_dispatch.py
>> @@ -0,0 +1,223 @@
>> +#!/usr/bin/python
> Please modify this chbange as well
>
> Presumably this should have a license header as well?
>
>> +
>> +"""
>> +Generates dispatch functions for EGL.
>> +
>> +The list of functions and arguments is read from the Khronos's XML files, with
>> +additional information defined in the module eglFunctionList.
>> +"""
>> +
>> +import sys
>> +import collections
>> +import imp
> please sort these as well
>
>> +
>> +import genCommon
>> +
>> +def main():
>> + if (len(sys.argv) < 4):
>> + print("Usage: %r source|header <function_list> <xml_file> [xml_file...]" % (sys.argv[0],))
>> + sys.exit(2)
> Most of our scripts use argparse (which is built-in to python 2.7+,
> which is conincidentlyw hat we support), wcould w use that to simplify
> this as well?
>
> something like:
>
> parser = argparse.ArgumentParser()
> parser.add_argument('target',
> choices=['header', 'source'],
> help="...")
> parser.add_argument('func_list_file',
> help="...")
> parser.add_argument('xml_files',
> nargs='*',
> help="...")
> args = parser.parse_args()
>
>> +
>> + target = sys.argv[1]
>> + funcListFile = sys.argv[2]
>> + xmlFiles = sys.argv[3:]
>> +
>> + # The function list is a Python module, but it's specified on the command
>> + # line.
>> + eglFunctionList = imp.load_source("eglFunctionList", funcListFile)
>> +
>> + xmlFunctions = genCommon.getFunctions(xmlFiles)
>> + xmlByName = dict((f.name, f) for f in xmlFunctions)
>> + functions = []
>> + for (name, eglFunc) in eglFunctionList.EGL_FUNCTIONS:
>> + func = xmlByName[name]
>> + eglFunc = fixupEglFunc(func, eglFunc)
>> + functions.append((func, eglFunc))
>> +
>> + # Sort the function list by name.
>> + functions = sorted(functions, key=lambda f: f[0].name)
>> +
>> + if (target == "header"):
>> + text = generateHeader(functions)
>> + elif (target == "source"):
>> + text = generateSource(functions)
>> + else:
>> + raise ValueError("Invalid target: %r" % (target,))
> Using argparse would remove the need for tihs else too.
>
>> + sys.stdout.write(text)
>> +
>> +def fixupEglFunc(func, eglFunc):
>> + result = dict(eglFunc)
>> + if (result.get("prefix") == None):
> if result.get("prefix") is None:
>
>> + result["prefix"] = ""
>> +
>> + if (result.get("extension") != None):
> if result.get("extension") is not None:
>
>> + text = "defined(" + result["extension"] + ")"
>> + result["extension"] = text
>> +
>> + if (result["method"] in ("none", "custom")):
>> + return result
>> +
>> + if (result["method"] not in ("display", "device", "current")):
>> + raise ValueError("Invalid dispatch method %r for function %r" % (result["method"], func.name))
>> +
>> + if (func.hasReturn()):
>> + if (result.get("retval") == None):
>> + result["retval"] = getDefaultReturnValue(func.rt)
>> +
>> + return result
>> +
>> +def generateHeader(functions):
> textwrap.dedent is your friend here.
>
>> + text = r"""
>> +#ifndef G_EGLDISPATCH_STUBS_H
>> +#define G_EGLDISPATCH_STUBS_H
>> +
>> +#ifdef __cplusplus
>> +extern "C" {
>> +#endif
>> +
>> +#include <EGL/egl.h>
>> +#include <EGL/eglext.h>
>> +#include "glvnd/libeglabi.h"
>> +
>> +""".lstrip("\n")
>> +
>> + text += "enum {\n"
>> + for (func, eglFunc) in functions:
>> + text += generateGuardBegin(func, eglFunc)
>> + text += " __EGL_DISPATCH_" + func.name + ",\n"
>> + text += generateGuardEnd(func, eglFunc)
>> + text += " __EGL_DISPATCH_COUNT\n"
>> + text += "};\n"
>> +
>> + for (func, eglFunc) in functions:
>> + if (eglFunc["inheader"]):
>> + text += generateGuardBegin(func, eglFunc)
>> + text += "{f.rt} EGLAPIENTRY {ex[prefix]}{f.name}({f.decArgs});\n".format(f=func, ex=eglFunc)
>> + text += generateGuardEnd(func, eglFunc)
>> +
>> + text += r"""
>> +#ifdef __cplusplus
>> +}
>> +#endif
>> +#endif // G_EGLDISPATCH_STUBS_H
>> +"""
>> + return text
>> +
>> +def generateSource(functions):
> I would strongly look into use mako for this funciton, we already use it
> in mesa.
Is there a strong preference for using Mako versus regular Python
templates in a case like this?
The original script from libglvnd uses normal templates since I didn't
want to add the extra dependency, but if the rest of Mesa already
requires Mako, then there's no reason this one couldn't either.
>
>> --
>> 2.7.4
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170104/20b6e411/attachment-0001.html>
More information about the mesa-dev
mailing list