Bugzilla – Bug 29910
Mesa advertises bogus GL_ARB_shading_language_120
Last modified: 2010-09-22 09:51:08 UTC
I noticed this in
OpenGL renderer string: Software Rasterizer
OpenGL version string: 2.1 Mesa 7.8.2
OpenGL shading language version string: 1.20
but grepping the last git master (mesa_7_6_1_rc1-12198-g99f3c9c) shows mention of this extension in multiple places. It is not a listed ARB extension at http://www.opengl.org/registry/ - the correct way to indicate support for GLSL 1.20 is through the SHADING_LANGUAGE_VERSION query (as Mesa does).
That is bogus. I believe the reason Mesa does this is there is no other way for a driver to advertise to the core that it supports 1.20 than enabling an extension. As far as I'm aware 1.20 doesn't require any extra hardware support, so we could probably fix this by always advertising 1.20 if GLSL is supported by the driver.
Bruce, are you talking about seeing GL_ARB_shading_language_120 in the list of extensions?
Ian is right that drivers would enable ctx->Extensions.ARB_shading_language_120 if they support it.
I see that NVIDIA doesn't list GL_ARB_shading_language_120 in the extension string. I guess we need some way to make this extension "invisible".
> Bruce, are you talking about seeing GL_ARB_shading_language_120 in the list of
Sorry yes, that's what I meant: it's the fact that it's showing up in GL_EXTENSIONS that is a bug. I haven't looked at the internal interfaces between drivers and the core.
Created attachment 38410 [details] [review]
This patch does away with the internal GL_ARB_shading_language_120 extension and replaces it with a new ctx->Const.GLSLVersion field. We can/should eventually pass this into the GLSL compiler.
If there aren't any concerns with this patch I'll commit it in a few days.
Review of attachment 38410 [details] [review]:
I see two minor problems with the patch.
1. I think we still need GL_VERSION_2_1_functions to show up in the extensions list. Otherwise there are cases where the new functions won't be available from GetProcAddress. The r300 driver handles this a different way, so i965 could just copy that.
2. GLSL ES needs 100 to be a valid GLSL version.
Other than that, it looks great to me.
Created attachment 38415 [details] [review]
Thanks for reviewing, Ian.
Here's an updated patch.
I'm unable to test the Intel driver at this time. Could you give it a try?
The change for ES is also untested but if there's anything wrong I'm sure it can be easily fixed up.
The addition to r600_context.c seems to be mis-indented, although that might just be my browser.
I've committed the second patch (e7087175f8a04f777403366fb34b58edd00f4d60)
Created attachment 38867 [details] [review]
Patch that compiles, but doesn't work
Unfortunately, the patch breaks the build for me:
brw_context.c:45:35: error: array type has incomplete element type
brw_context.c:46:22: error: ‘GL_VERSION_2_1_functions’ undeclared here (not in a function)
brw_context.c: In function ‘brwCreateContext’:
brw_context.c:163:4: warning: implicit declaration of function ‘driInitExtensions’
brw_context.c: At top level:
brw_context.c:45:35: warning: ‘gl_21_extension’ defined but not used
I attached a patch which moves this to intel_extensions.c, where it probably belongs...unfortunately, it doesn't /work/...running a random piglit test gives:
Mesa 7.10-devel implementation error: Trying to enable unknown extension: GL_VERSION_2_1
Perhaps you really just want to call _mesa_enable_2_1_extensions?
Author: Kristian Høgsberg <email@example.com>
Date: Wed Sep 22 11:01:11 2010 -0400
intel: Fix GL_ARB_shading_language_120 commit
Fix commit e7087175f8a04f777403366fb34b58edd00f4d60. Move the reference to
GL_VERSION_2_1_functions to intel_extensions.c where it's available,
don't try to enable a non-existing extension and advertise 1.20 for all
intel chipsets, not just GEN4 and up.
This looks fine for now, and I'm going to cherry-pick both patches to 7.9. In the future we will want to expose the supported versions as a bitmask. In later versions of OpenGL core profile the older shading languages cease to be supported. This may also allow a more unified method for testing support for GLSL ES (which is version 1.00).
There will be a lot of other changes required to support this, so we can cross that bridge when we come to it.