[Piglit] [PATCH 10/29] glapi: Read list of core extensions
Paul Berry
stereotype441 at gmail.com
Fri May 25 09:19:56 PDT 2012
On 21 May 2012 11:08, Pauli Nieminen <pauli.nieminen at linux.intel.com> wrote:
> There is extensions that have been pulled as is into core. Those
> extensions only declare extension functions in gl.spec. To allow stub
> loader know that required function is supported because of GL version we
> need to add core version to the extension functions.
>
Can you explain in more detail why this is necessary? I thought that when
an extension is pulled into core, the implementation was required to list
that extension in its extension string. So the stub loader should already
know that these functions are supported.
>
> Signed-off-by: Pauli Nieminen <pauli.nieminen at linux.intel.com>
> ---
> glapi/parse_glspec.py | 34 ++++++++++++++++++++++++++++++++--
> 1 files changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/glapi/parse_glspec.py b/glapi/parse_glspec.py
> index f513164..1097e3c 100644
> --- a/glapi/parse_glspec.py
> +++ b/glapi/parse_glspec.py
> @@ -126,6 +126,8 @@ import sys
>
> GLSPEC_HEADER_REGEXP = re.compile(r'^(\w+)\((.*)\)$')
> GLSPEC_ATTRIBUTE_REGEXP = re.compile(r'^\s+(\w+)\s+(.*)$')
> +GLSPEC_EXT_VERSION_REGEXP =
> re.compile(r'^passthru:\s+/\*\sOpenGL\s+([0-9.]+)\s+.*reuses')
> +GLSPEC_EXT_REGEXP = re.compile(r'^passthru:\s+/\*\s+(\w+)')
>
I'm concerned about extracting this information out of the "passthru:"
comment lines, since they're intended for human consumption, not machine
consumption. It looks to me like the same information is present in the
"version" field underneath each function declaration--I would prefer to
extract the information from there.
> GL_VERSION_REGEXP = re.compile('^VERSION_([0-9])_([0-9])(_DEPRECATED)?$')
> ENUM_REGEXP = re.compile(r'^\s+(\w+)\s+=\s+(\w+)$')
>
> @@ -284,6 +286,10 @@ class Api(object):
> # 'extension_name': 'GL_ARB_sync' }
> self.categories = {}
>
> + # Api.core_exts is a dict to map backport extension to GL version
> + # 'GL_ARB_sync': ['3.3']
> + self.core_exts = {}
> +
>
Is this intended to represent:
(a). only those extensions that were written after version X of GL was
created, for the purpose of allowing implementations that don't fully
support GL version X to expose a subset of its functionality?
(b). all extensions that are required to be present in a conformant
implementation of GL version X?
Your use of the term "backport extension" makes it sound like it's (a), but
the name "core_exts" makes it sound like it's (b). I would recommend
changing the comment to clarify what the intent is, as well as to clarify
which GL version is being referred to (e.g. "Api.core_exts is a dict to map
an extension name to the first GL version in which that extension is
required. Extensions that are not required by any version of GL do not
appear in this dict.")
Also, it looks like the values in the dict are actually lists of versions.
Can you explain what it would mean to have multiple entries in the list?
> # Convert each line in the gl.tm file into a key/value pair in
> # self.type_translation, mapping an abstract type name to a C
> # type.
> @@ -311,13 +317,29 @@ class Api(object):
> # ('Foo', ['bar', 'baz'],
> # {'x': ['value1'], 'y': ['value2 other_info', 'value3 more_info']})
> @staticmethod
> - def group_gl_spec_functions(f):
> + def group_gl_spec_functions(self, f):
>
If we're going to pass self to this function, we might as well just remove
the "@staticmethod" annotation so that we can call it like a normal method.
> function_name = None
> param_names = None
> attributes = None
> + version = None
> for line in filter_comments(f):
> + m = GLSPEC_EXT_VERSION_REGEXP.match(line)
> + if m:
> + version = m.group(1)
> + continue
> + if version != None:
> + m = GLSPEC_EXT_REGEXP.match(line)
> + if m:
> + ext = m.group(1)
> + ext = 'GL_' + ext
> + if ext in self.core_exts:
> + self.core_exts[ext].append(version)
> + else:
> + self.core_exts[ext] = [version]
> + continue
> m = GLSPEC_HEADER_REGEXP.match(line)
> if m:
> + version = None
> if function_name:
> yield function_name, param_names, attributes
> function_name = m.group(1)
>
With this change, group_gl_spec_functions() now has two outputs which it
returns in completely different ways: one which it outputs via "yield"
statements, and one which it pokes into self.core_exts. I would prefer to
just change it from a generator function to an ordinary function (that
builds up a result in a list and then returns it), that way we can have it
return both outputs in the normal way.
> @@ -339,7 +361,7 @@ class Api(object):
> # Process the data in gl.spec, and populate self.functions,
> # self.synonyms, and self.categories based on it.
> def read_gl_spec(self, f):
> - for name, param_names, attributes in
> self.group_gl_spec_functions(f):
> + for name, param_names, attributes in
> self.group_gl_spec_functions(self, f):
>
With "@staticmethod" removed this change shouldn't be needed.
> if name in self.functions:
> raise Exception(
> 'Function {0!r} appears more than once'.format(name))
> @@ -415,6 +437,14 @@ class Api(object):
> 'param_types': param_types,
> 'category': [category],
> }
> + if category in self.core_exts:
> + for ver in self.core_exts[category]:
> + self.functions[name]['category'].append(ver)
> + if ver not in self.categories:
> + self.categories[ver] = {
> + 'kind': 'GL',
> + 'gl_10x_version': int(ver.replace('.',''))
> + }
>
If possible, I would prefer to keep the amount of data munging in
parse_glspec.py to a minimum, so that its output is simply a
transliteration of the source spec file into a more machine readable form,
and have gen_dispatch.py do the interpretation. Would it suit your needs
to store the information you want in self.functions[name]['version'] so
that it's more similar to the organization of gl.spec?
> self.synonyms.add_singleton(name)
> for alias in attributes['alias']:
> self.synonyms.add_alias(name, alias)
> --
> 1.7.5.4
>
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20120525/ea645ba0/attachment-0001.htm>
More information about the Piglit
mailing list