On 8 October 2011 06:49, Brian Paul <<a href="mailto:brian.e.paul@gmail.com" target="_blank">brian.e.paul@gmail.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"> <div>On Fri, Oct 7, 2011 at 6:28 PM, Paul Berry <<a href="mailto:stereotype441@gmail.com" target="_blank">stereotype441@gmail.com</a>> wrote: > --- > tests/util/piglit-util.c | 12 ++++++++++++ > tests/util/piglit-util.h | 1 + > 2 files changed, 13 insertions(+), 0 deletions(-) > > diff --git a/tests/util/piglit-util.c b/tests/util/piglit-util.c > index 358b0dc..b8a0219 100644 > --- a/tests/util/piglit-util.c > +++ b/tests/util/piglit-util.c > @@ -108,6 +108,18 @@ bool piglit_is_extension_supported(const char *name) > return found; > } > > +void piglit_require_GL_version(double required_version) > +{ > + double gl_version = strtod((char *) glGetString(GL_VERSION), NULL); </div>I don't think we can use strtod(). The OpenGL version string always uses '.' while strtod()'s decimal point character is local-dependent. I think we could get by with something like const GLUbyte *verStr = glGetString(GL_VERSION); int version = verStr[0] * 10 + verStr[2]; </blockquote><div> Hmm, I did not think of this. (...goes and learns a lot about locales...) It turns out that my version works ok even if the computer's default locale uses ',' as the decimal separator, because the computer's default locale doesn't take effect until you call setlocale(LC_ALL, ""), which Piglit doesn't do. Until then you're always in the "C" locale, which uses '.' as the decimal separator (see <a href="http://www.chemie.fu-berlin.de/chemnet/use/info/libc/libc_19.html#SEC323" target="_blank">http://www.chemie.fu-berlin.de/chemnet/use/info/libc/libc_19.html#SEC323</a>). However, I still like your version better, because (a) you don't have to research locales to see why it works, and (b) it neatly avoids any issues with rounding errors. So I'm going to go with it. Incidentally, while investigating this stuff I discovered that a lot of the glean tests don't use ">=" for their version comparisons where they should. For example, in tshaderapi.cpp, we have this: const char *version = (const char *) glGetString(GL_VERSION); if (strncmp(version, "2.0", 3) == 0 || strncmp(version, "2.1", 3) == 0 || strncmp(version, "3.0", 3) == 0) { return true; } else { env->log << name << ": skipped. Requires GL 2.0, 2.1 or 3.0.\n"; return false; } Which means that the "shaderapi" test gets skipped on a GL implementation that advertises support for GL version 3.1 or newer. Worse yet, since the test doesn't use the standard piglit mechanism to report that it's skipping, this shows up in the Piglit results as a "pass". Really this code should be replaced with: piglit_require_GL_version(20); (taking your suggestion of using an integer to represent ten times the version rather than a floating-point value). </div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"> BTW, I think a pigilt_get_gl_version() function would be useful too. <div><div></div><div> </div></div></blockquote><div> It turns out that Piglit already has such a function (I didn't realize this either until this morning): void piglit_get_gl_version(bool *es, int* major, int* minor); But this type signature is horrible because it means if you want to compare version numbers you have to jump through hoops: /* Are we using GL with a version of at least 3.1? */ bool es; int major; int minor; piglit_get_gl_version(&es, &major, &minor); if (es && (major > 3 || (major == 3 && minor >= 1))) I would far rather change this to: bool piglit_is_gles(); int piglit_get_gl_version(); /* returns 10*major + minor */ So we could do this: if (piglit_is_gles() && piglit_get_gl_version() >= 31) Fortunately piglit_get_gl_version() is only called from one place right now, so this should be easy to change. I'll go ahead and fold your suggestions on piglit_require_GL_version() into this patch. I'll submit a separate patch series to fix the problems with piglit_get_gl_version() and the glean tests. Paul </div></div>