On 21 May 2012 11:08, Pauli Nieminen <<a href="mailto:pauli.nieminen@linux.intel.com" target="_blank">pauli.nieminen@linux.intel.com</a>> wrote: <div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> 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. </blockquote><div> 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. </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> Signed-off-by: Pauli Nieminen <<a href="mailto:pauli.nieminen@linux.intel.com" target="_blank">pauli.nieminen@linux.intel.com</a>> --- 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+)') </blockquote><div> 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. </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> 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 = {} + </blockquote><div> 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? </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> # Convert each line in the <a href="http://gl.tm" target="_blank">gl.tm</a> 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): </blockquote><div> 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. </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> 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) </blockquote><div> 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. </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> @@ -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): </blockquote><div> With "@staticmethod" removed this change shouldn't be needed. </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> 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('.','')) + } </blockquote><div> 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? </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> self.synonyms.add_singleton(name) for alias in attributes['alias']: self.synonyms.add_alias(name, alias) -- 1.7.5.4 _______________________________________________ Piglit mailing list <a href="mailto:Piglit@lists.freedesktop.org" target="_blank">Piglit@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/piglit" target="_blank">http://lists.freedesktop.org/mailman/listinfo/piglit</a> </blockquote></div>