<div dir="ltr">I'd suggest changing to a commit subject that describes precisely what change you're making in the active voice, e.g. something like "Fix shader_runner regexps to match new version requirement syntax". The existing commit subject ("shader_runner regression with gles") is too vague.<br>
<div><br>On 17 January 2013 15:08, Tom Gall <span dir="ltr"><<a href="mailto:tom.gall@linaro.org" target="_blank">tom.gall@linaro.org</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Augment the parser that examines the shader_test file passed in<br>
with the test to determine which brand of shader_running to<br>
execute, shader_runner, shader_runner_gles2, etc.<br>
<br>
This fixes a bug where GLSL ES if first in the [required] section<br>
could match against GL after which the wrong shader_runner command<br>
would be used due to the wrong gl api being used.<br></blockquote><div><br></div><div>This seems like a red herring to me. How would "GLSL ES" have matched any of the preexisting regular expressions? It seems like the substantive change you're making here is to change the regular expressions from matching "GL >= N.M es" (which was the correct syntax before Stuart Abercrombie's patch "shader_runner: Alter GL/GLSL ES version requirement syntax.") to matching "GL ES >= N.M" (which is the correct syntax now).<br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Last if plain GLSL is found, GL is presumed as the api. If GLSL ES<br>
is found, GL ES is presumed as the api.<br></blockquote><div><br></div><div>This seems reasonable, but it's a separate feature and should be in a separate patch.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
When dispatch is implemented for GLES all this can go away.<br>
<br>
Signed-off-by: Tom Gall <<a href="mailto:tom.gall@linaro.org">tom.gall@linaro.org</a>> <br></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
---<br>
framework/shader_test.py | 26 ++++++++++++++++++--------<br>
1 file changed, 18 insertions(+), 8 deletions(-)<br>
<br>
diff --git a/framework/shader_test.py b/framework/shader_test.py<br>
index 335a6c1..4db291b 100755<br>
--- a/framework/shader_test.py<br>
+++ b/framework/shader_test.py<br>
@@ -107,16 +107,18 @@ class ShaderTest(PlainExecTest):<br>
common = {<br>
'cmp' : r'(<|<=|=|>=|>)',<br>
'gl_version' : r'(\d.\d)',<br>
- 'gles2_version' : r'(2.\d\s+es)',<br>
- 'gles3_version' : r'(3.\d\s+es)',<br>
+ 'gles2_version' : r'(2.\d\s)',<br>
+ 'gles3_version' : r'(3.\d\s)',<br>
'comment' : r'(#.*)',<br>
}<br>
<br>
cls.__re_require_header = re.compile(r'^\s*\[require\]\s*{comment}?$'.format(**common))<br>
cls.__re_end_require_block = re.compile(r'^\s*\['.format(*common))<br>
cls.__re_gl = re.compile(r'^\s*GL\s*{cmp}\s*{gl_version}\s*{comment}?$'.format(**common))<br>
- cls.__re_gles2 = re.compile(r'^\s*GL\s*{cmp}\s*{gles2_version}\s*{comment}?$'.format(**common))<br>
- cls.__re_gles3 = re.compile(r'^\s*GL\s*{cmp}\s*{gles3_version}\s*{comment}?$'.format(**common))<br>
+ cls.__re_gles2 = re.compile(r'^\s*GL ES\s*{cmp}\s*{gles2_version}\s*{comment}?$'.format(**common))<br>
+ cls.__re_gles3 = re.compile(r'^\s*GL ES\s*{cmp}\s*{gles3_version}\s*{comment}?$'.format(**common))<br>
+ cls.__re_glsl = re.compile(r'^\s*GLSL\s*{cmp}\s*{gl_version}\s*{comment}?$'.format(**common))<br>
+ cls.__re_glsles = re.compile(r'^\s*GLSL ES\s*{cmp}\s*{gl_version}\s*{comment}?$'.format(**common))<br>
cls.__re_gl_unknown = re.compile(r'^\s*GL\s*{cmp}'.format(**common))<br>
<br>
def __init__(self, shader_runner_args, run_standalone=False):<br>
@@ -174,15 +176,23 @@ class ShaderTest(PlainExecTest):<br>
else:<br>
continue<br>
elif parse_state == PARSE_FIND_GL_REQUIREMENT:<br>
- if cls.__re_gl.match(line) is not None:<br>
- self.__gl_api = ShaderTest.API_GL<br>
- return<br>
- elif cls.__re_gles2.match(line) is not None:<br>
+ if cls.__re_gles2.match(line) is not None:<br></blockquote><div><br></div><div>I don't think it should be necessary to remove the cls.__re_gl test from here and put it at the end. The regular expression for cls.__re_gl used to overlap with those for cls.__re_gles2 and cls.__re_gles3, but thanks to the other changes in your patch, it doesn't anymore, so order should be irrelevant. Or am I missing something?<br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
self.__gl_api = ShaderTest.API_GLES2<br>
return<br>
elif cls.__re_gles3.match(line) is not None:<br>
self.__gl_api = ShaderTest.API_GLES3<br>
return<br>
+ elif cls.__re_glsles.match(line) is not None:<br>
+ if self.__gl_api is None:<br>
+ self.__gl_api = ShaderTest.API_GLES2<br>
+ return<br>
+ elif cls.__re_glsl.match(line) is not None:<br>
+ if self.__gl_api is None:<br>
+ self.__gl_api = ShaderTest.API_GL<br>
+ return<br>
+ elif cls.__re_gl.match(line) is not None:<br>
+ self.__gl_api = ShaderTest.API_GL<br>
+ return<br>
elif cls.__re_gl_unknown.match(line) is not None:<br>
self.__report_failure("Failed to parse GL requirement: " + line)<br>
self.__gl_api = ShaderTest.API_ERROR<br>
<span class=""><font color="#888888">--<br>
1.7.10.4<br>
<br>
_______________________________________________<br>
Piglit mailing list<br>
<a href="mailto:Piglit@lists.freedesktop.org">Piglit@lists.freedesktop.org</a><br>
<a href="http://lists.freedesktop.org/mailman/listinfo/piglit" target="_blank">http://lists.freedesktop.org/mailman/listinfo/piglit</a><br>
</font></span></blockquote></div><br></div></div></div>