Bug 95215 - ARB_shading_language_include is not implemented
Summary: ARB_shading_language_include is not implemented
Status: RESOLVED WONTFIX
Alias: None
Product: Mesa
Classification: Unclassified
Component: Mesa core (show other bugs)
Version: git
Hardware: All All
: medium enhancement
Assignee: mesa-dev
QA Contact: mesa-dev
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-04-30 18:26 UTC by Jamey Sharp
Modified: 2018-05-10 05:22 UTC (History)
7 users (show)

See Also:
i915 platform:
i915 features:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jamey Sharp 2016-04-30 18:26:55 UTC
The game "Divinity: Original Sin Enhanced Edition" uses ARB_shading_language_include, which is not yet implemented in Mesa.

Ernst Sjöstrand reported in bug #93551 that after setting MESA_GL_VERSION_OVERRIDE=4.2 and applying my Mesa patch from that bug:
> ... with RadeonSI I get a bunch of these
> glNamedStringARB errors when running the game with APITrace:
> 
> 2833: message: major api error 1: GL_INVALID_OPERATION in unsupported
> function called (unsupported extension or deprecated function?)
> 2833 @0 glNamedStringARB(type = GL_SHADER_INCLUDE_ARB, namelen = -1, name =
> "/Shaders/GlobalConstants_OGL.shdh", stringlen = 1553, string = "#define
> HD4000 0
> 
> glNamedStringARB is from ARB_shading_language_include, which is not
> implemented in Mesa afaict? Probably because the extension is not part of
> any GL specification?

Karol Herbst previously reported the same thing in this mail on the Mesa list:

https://lists.freedesktop.org/archives/mesa-dev/2016-March/109789.html

Karol also put up a WIP branch implementing the extension:

https://github.com/karolherbst/mesa/commits/ARB_shading_language_include

I guess next someone needs to review Karol's branch and see what it'll take to get it merged?
Comment 1 Ilia Mirkin 2016-04-30 18:31:41 UTC
Sounds like the game is buggy and needs fixing - I presume it runs fine on Catalyst drivers, which also don't support the ext.
Comment 2 Jamey Sharp 2016-04-30 21:06:47 UTC
(In reply to Ilia Mirkin from comment #1)
> Sounds like the game is buggy and needs fixing - I presume it runs fine on
> Catalyst drivers, which also don't support the ext.

Hmm. Is apitrace mis-leading us here?

`nm -D` tells me that the only uses of NamedStringARB are in libOGLBinding.so, shipped with the game. Specifically it uses the __glewNamedStringARB function pointer.

`readelf --relocs libOGLBinding.so | grep __glewNamedStringARB` tells me the address of that function pointer is written to offset 29c68, which I find used twice in `objdump -d libOGLBinding.so`.

In both cases, the following instructions check that the function pointer is not null. Inside `api::OpenGLRenderer::CreateShader(ls::ObjectHandle)`, if the function pointer is null, it calls `api::OpenGLRenderer::EmbedIncludes(ls::STDString&)`, which sounds sensible. Inside `api::OpenGLRenderer::CreateShader(ls::ObjectHandle)` the fallback goes to a function that isn't in the symbol table, but it does a bunch of string manipulation so I'm going to guess it's probably similar.

So: Does apitrace make __glewNamedStringARB non-null even when the function isn't available in the underlying driver? If so, then we're following a different code path in the game when we trace it. If that's the case, then this bug has nothing to do with any remaining rendering issues, but it would be nice to know how to make apitrace not do that so we can find real bugs.
Comment 3 Ilia Mirkin 2016-04-30 21:09:34 UTC
(In reply to Jamey Sharp from comment #2)
> (In reply to Ilia Mirkin from comment #1)
> > Sounds like the game is buggy and needs fixing - I presume it runs fine on
> > Catalyst drivers, which also don't support the ext.
> 
> Hmm. Is apitrace mis-leading us here?

More likely the game is buggy. Note that glXGetProcAddress("bla") != null. See https://dri.freedesktop.org/wiki/glXGetProcAddressNeverReturnsNULL/
Comment 4 Jamey Sharp 2016-04-30 21:21:35 UTC
(In reply to Ilia Mirkin from comment #3)
> (In reply to Jamey Sharp from comment #2)
> > (In reply to Ilia Mirkin from comment #1)
> > > Sounds like the game is buggy and needs fixing - I presume it runs fine on
> > > Catalyst drivers, which also don't support the ext.
> > 
> > Hmm. Is apitrace mis-leading us here?
> 
> More likely the game is buggy. Note that glXGetProcAddress("bla") != null.
> See https://dri.freedesktop.org/wiki/glXGetProcAddressNeverReturnsNULL/

OK, but __glewNamedStringARB *is* null when run on Mesa, so I guess GLEW's semantics are different from glXGetProcAddress.

Sure, there is a game bug here: GLEW's documentation says that the game should check if GLEW_ARB_shading_language_include is true, and doesn't say you can check for a non-null function pointer instead.

That doesn't change the fact that tracing apparently changes the game's behavior, which is obnoxious and leads to mis-diagnosed bug reports. Unless you're saying that tracing is actually not changing the game's behavior?
Comment 5 Jamey Sharp 2016-05-01 01:39:26 UTC
Well, that's interesting. I can't reproduce the error that Ernst or Karol reported. Running under the Debian packaged version of apitrace, which is from git commit 62ad71c6 apparently, runs just as well as untraced--which is to say there are plenty of rendering bugs, but apitrace never complains about calls to glNamedStringARB and there are no calls to that function recorded in the trace.

Ernst, et al: I have game version 2.0.119.430. What version do you have and how do you reproduce this issue? Can you provide evidence that any problems you're having are due to lack of this extension?
Comment 6 Ernst Sjöstrand 2016-05-01 13:03:24 UTC
In the big string field in glNamedStringARB there's a lot of names that relate to AMD hardware. Perhaps it's detecting that it's running on AMD and choosing a different rendering path, which happens to include a lot of glNamedStringARB?
Comment 7 Ilia Mirkin 2016-05-01 15:24:55 UTC
(In reply to Jamey Sharp from comment #4)
> (In reply to Ilia Mirkin from comment #3)
> > (In reply to Jamey Sharp from comment #2)
> > > (In reply to Ilia Mirkin from comment #1)
> > > > Sounds like the game is buggy and needs fixing - I presume it runs fine on
> > > > Catalyst drivers, which also don't support the ext.
> > > 
> > > Hmm. Is apitrace mis-leading us here?
> > 
> > More likely the game is buggy. Note that glXGetProcAddress("bla") != null.
> > See https://dri.freedesktop.org/wiki/glXGetProcAddressNeverReturnsNULL/
> 
> OK, but __glewNamedStringARB *is* null when run on Mesa, so I guess GLEW's
> semantics are different from glXGetProcAddress.

I can't think of anything that'd be different between running with apitrace and running with mesa directly. In both cases, the function pointer returned is non-null.
Comment 8 Kenneth Graunke 2016-05-02 01:42:34 UTC
FWIW, I would prefer not to implement ARB_shading_language_include.
Comment 9 farmboy0+freedesktop 2016-05-02 18:44:03 UTC
(In reply to Kenneth Graunke from comment #8)
> FWIW, I would prefer not to implement ARB_shading_language_include.

Why not?
Comment 10 Karol Herbst 2016-05-08 00:49:16 UTC
see this comment: 

https://bugs.freedesktop.org/show_bug.cgi?id=93551#c22
Comment 11 Karol Herbst 2016-05-08 00:50:30 UTC
(In reply to Kenneth Graunke from comment #8)
> FWIW, I would prefer not to implement ARB_shading_language_include.

well the game requires it, because even GL vendor spoofing to catalyst still keeps the game using this extension. and they don't want to support mesa anyway, at least the kind of gave me the feeling that they don't care.
Comment 12 Ian Romanick 2016-05-09 19:12:38 UTC
(In reply to Karol Herbst from comment #11)
> well the game requires it, because even GL vendor spoofing to catalyst still
> keeps the game using this extension. and they don't want to support mesa
> anyway, at least the kind of gave me the feeling that they don't care.

It seems very unlikely that the game requires it.  According to http://feedback.wildfiregames.com/report/opengl/feature/GL_ARB_shading_language_include, NVIDIA is the *only* vendor that implements this extension.  The game also runs on Mac OS, so unless they're using Metal, the don't use that extension on Apple.

To really verify that the game requires it, you'd probably have to hack up the version of GLEW that it's using (which may be linked to the app) to report GLEW_ARB_shading_language_include = false, or insert a shim so that either glXGetProcAddress("glNamedStringARB") returns NULL or glGetString and glGetStringi don't return GL_ARB_shading_language_include.
Comment 13 Karol Herbst 2016-05-10 18:11:51 UTC
(In reply to Ian Romanick from comment #12)
> (In reply to Karol Herbst from comment #11)
> > well the game requires it, because even GL vendor spoofing to catalyst still
> > keeps the game using this extension. and they don't want to support mesa
> > anyway, at least the kind of gave me the feeling that they don't care.
> 
> It seems very unlikely that the game requires it.  According to
> http://feedback.wildfiregames.com/report/opengl/feature/
> GL_ARB_shading_language_include, NVIDIA is the *only* vendor that implements
> this extension.  The game also runs on Mac OS, so unless they're using
> Metal, the don't use that extension on Apple.
> 
> To really verify that the game requires it, you'd probably have to hack up
> the version of GLEW that it's using (which may be linked to the app) to
> report GLEW_ARB_shading_language_include = false, or insert a shim so that
> either glXGetProcAddress("glNamedStringARB") returns NULL or glGetString and
> glGetStringi don't return GL_ARB_shading_language_include.

we already figured it out: the game engine checked the pointers of the functions against NULL
Comment 14 Timothy Arceri 2018-05-10 05:22:31 UTC
Closing as won't fix. The game don't require the extension it just doesn't check properly to see if its supported.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.