https://bugs.freedesktop.org/show_bug.cgi?id=66067

Nicholas Miell nmiell@gmail.com changed:

What |Removed |Added ---------------------------------------------------------------------------- Attachment #81244|text/plain |image/png mime type| |

https://bugs.freedesktop.org/show_bug.cgi?id=66067

--- Comment #3 from Nicholas Miell nmiell@gmail.com --- Bug 36236 is about Trine, not Trine 2, but possibly.

There's also bug 60553, which involves Trine 2, but a Northerns Islands GPU and differences in rendering when fullscreen vs. windowed.

https://bugs.freedesktop.org/show_bug.cgi?id=66067

--- Comment #5 from Nicholas Miell nmiell@gmail.com --- Created attachment 82061 --> https://bugs.freedesktop.org/attachment.cgi?id=82061&action=edit Call 5654455, GL_COLOR_ATTACMENT0, Catalyst

https://bugs.freedesktop.org/show_bug.cgi?id=66067

--- Comment #6 from Nicholas Miell nmiell@gmail.com --- It finally occurred to me to test the apitrace with Mesa LIBGL_ALWAYS_SOFTWARE=1 and it was rendered correctly.

https://bugs.freedesktop.org/show_bug.cgi?id=66067

--- Comment #7 from Grigori Goronzy greg@chown.ath.cx --- This actually seems to be a bug in Trine 2. The fragment shader uses a shadow sampler to sample the normal texture, which is RGBA. This cannot work as shadow sampling is only defined for depth textures. According to the ARB_fragment_program_shadow specification (Trine 2 uses those old assembly-like shaders for some reason), it is undefined behaviour to sample from non-depth textures. Obviously, Trine 2 relies on a certain behavior, i.e. that shadow samplers are silently changed into normal 2D samplers when not sampling from depth textures.

I have contacted Frozenbyte support, let's see if they'll do something about. Otherwise we need to check if it is reasonably possible and sensible to implement these undefined cases like other drivers. This might get ugly, though.

If you just want to play the game and don't mind hacking mesa, try the nasty patch I attached. This will break other applications, but makes Trine 2 work.

https://bugs.freedesktop.org/show_bug.cgi?id=66067

--- Comment #9 from Nicholas Miell nmiell@gmail.com --- (In reply to comment #7)

Trine 2 uses those old assembly-like shaders for some reason

It's the minimum spec target profile for Nvidia's Cg compiler.

This cannot work as shadow sampling is only defined for depth textures. According to the ARB_fragment_program_shadow specification (Trine 2 uses those old assembly-like shaders for some reason), it is undefined behaviour to sample from non-depth textures.

ARB_fragment_program_shadow may leave it undefined, but the GL spec says the texture unit operates in the normal manner and texture comparison is bypassed if the currently bound texture's base internal format is not DEPTH_COMPONENT or DEPTH_STENCIL.

https://bugs.freedesktop.org/show_bug.cgi?id=66067

--- Comment #11 from Grigori Goronzy greg@chown.ath.cx ---

ARB_fragment_program_shadow may leave it undefined, but the GL spec ...

Which spec exactly? GL specifications only cover GLSL shaders. ARB_fragment_program is separate from that and has its own rules. The extension specification clearly states that behaviour is undefined.

Besides, OpenGL contradicts itself a little bit: in GL 4.4 spec, 15.2.1 it states that behaviour is undefined for this case, too. It makes much more sense anyway, shadow sampling is wildly different from sampling a texture normally in terms of what the sampling result look like.

https://bugs.freedesktop.org/show_bug.cgi?id=66067

--- Comment #13 from Roland Scheidegger sroland@vmware.com --- (In reply to comment #12)

(In reply to comment #11)

...

...

ARB_fragment_program_shadow may leave it undefined, but the GL spec ...

Which spec exactly? GL specifications only cover GLSL shaders. ARB_fragment_program is separate from that and has its own rules. The extension specification clearly states that behaviour is undefined.

Section 8.23.1 of the GL 4.4 spec, which I paraphrased in comment #9.

This only covers the DEPTH_TEXTURE_MODE but not the shadow target of arb_fragment_program_shadow. To quote from there: How should ARB_fragment_program_shadow function? a. Simply remove the interaction with ARB_shadow so that TEXTURE_COMPARE_MODE behaves exactly as specified in the OpenGL 1.4 specification. b. Add "SHADOW" targets to texture lookup instructions. TEXTURE_COMPARE_MODE is ignored. For samples from a SHADOW target TEXTURE_COMPARE_MODE is treated as COMPARE_R_TO_TEXTURE; otherwise, it is treated as NONE. c. Like (b), but with undefined results if TEXTURE_COMPARE_MODE and/or the internal format of the texture does not match the target. d. A hybrid of (a) and (b), where the SHADOW target means to use the TEXTURE_COMPARE_MODE state.

RESOLVED - Option c, undefined behavior when the target and mode do not match.

So if the cg compiler really does something as you suggested it is simply crazy to expect this to work. At least I see absolutely nothing why that GL wording somehow would apply to this (non-core) extension.

https://bugs.freedesktop.org/show_bug.cgi?id=66067

Nicholas Miell nmiell@gmail.com changed:

What |Removed |Added ---------------------------------------------------------------------------- Blocks| |77449

https://bugs.freedesktop.org/show_bug.cgi?id=66067

--- Comment #15 from smoki smoki00790@gmail.com ---

With current driver stack, kernel 3.16-rc3, mesa-10.3-devel (up to commit 370184e813b25b463ad3dc9ca814231c98b95864) and llvm 3.5 snapshot (svn svn212056) on radeonsi (Radeon 8400), etc... much of this darkness in Trine 2 disappear... i mean render much nearly correct :), only some little parts remain being black :).

https://bugs.freedesktop.org/show_bug.cgi?id=66067

--- Comment #17 from smoki smoki00790@gmail.com ---

Yes just checked out your trace and got the same screenshot... yes sorry you are right, but seems to me radeonsi was having a more black even than this seems like onother bug which got fixed on top of this bug (Sylvain from comment 14 may confirm that with radeonsi). It was like this https://bugs.freedesktop.org/attachment.cgi?id=99363 and now is much better, but not complitely correct yet.

Sorry it is just that radeonsi is now in the same state as r600 to actually have this bug you describe :).

https://bugs.freedesktop.org/show_bug.cgi?id=66067

--- Comment #19 from Grigori Goronzy greg@chown.ath.cx --- By the way, I really wonder how this manages to work correctly with the blobs. Are they fixing up the shader on demand somehow?

Trine 2 (the Steam version) was updated a while ago, but sadly they haven't fixed this, despite promising to do so. :(

https://bugs.freedesktop.org/show_bug.cgi?id=66067

--- Comment #21 from Michel Dänzer michel@daenzer.net --- Created attachment 102470 --> https://bugs.freedesktop.org/attachment.cgi?id=102470&action=edit radeonsi: Handle sampler depth compare mode

This Mesa patch seems to fix the apitrace for me. Can you confirm?

https://bugs.freedesktop.org/show_bug.cgi?id=66067

--- Comment #23 from smoki smoki00790@gmail.com --- (In reply to comment #21)

Created attachment 102470 [details] [review] radeonsi: Handle sampler depth compare mode

This Mesa patch seems to fix the apitrace for me. Can you confirm?

I can confirm :) i tried this patch and this works fine for me on Kabini (Radeon 8400), 'glretrace -b -S 5670984' produce same image as Catalyst. Cool :).

https://bugs.freedesktop.org/show_bug.cgi?id=66067

--- Comment #25 from smoki smoki00790@gmail.com --- (In reply to comment #23)

(In reply to comment #21)

...

Created attachment 102470 [details] [review] [review] radeonsi: Handle sampler depth compare mode

This Mesa patch seems to fix the apitrace for me. Can you confirm?

I can confirm :) i tried this patch and this works fine for me on Kabini (Radeon 8400), 'glretrace -b -S 5670984' produce same image as Catalyst. Cool :).

BTW just to be aware of i see some side effects of this patch, for example performance in Unigine Sanctuary goes down by ~40% for me :).

https://bugs.freedesktop.org/show_bug.cgi?id=66067

--- Comment #27 from smoki smoki00790@gmail.com --- Probably. Anyway with this patch somehow Unigine Sanctuary high shader is affected, that huge performance drop happens only on Shaders:High setings, but as i see performance is not reduced at all on Medium or Low shaders setting.

https://bugs.freedesktop.org/show_bug.cgi?id=66067

--- Comment #29 from Nicholas Miell nmiell@gmail.com --- Does it ever actually sample from a depth texture or is unconditionally converting the SHADOW2D to 2D safe?

https://bugs.freedesktop.org/show_bug.cgi?id=66067

--- Comment #31 from Daniel Scharrer daniel@constexpr.org --- I have found another game that hits this: he Book of Unwritten Tales: The Critter Chronicles. Again, my workaround works for me. Unlike Trine however, this game ships its Cg shader sources as plain text files.

Changing the tex2Dproj() calls to use the correct overload by replacing "tex2Dproj(cookieMap, IN.LightSpacePos)" with "tex2Dproj(cookieMap, IN.LightSpacePos.xyw)" in kAGE_phong.cgfx and kAGE_phong_simple.cgfx fixes it for me. I've sent an email to the developer so hopefully this will be fixed in the game.

Meanwhile Trine is still broken on radeonsi. Since it works with fglrx, do we know how it handles mismatched GL_TEXTURE_COMPARE_MODE and sampler in the shader?

I suspect that the Cg compiler always compiles tex2Dproj to a TEX instruction with the SHADOW2D target and relies on OpenGL implementations always transparently using normal sampling on non-depth non-stencil textures.

It selects between the 2D and SHADOW2D samplers based on the number of components in the coordinate vector. This matches the documentation [1] which, unlike the tutorial you quoted, makes no mention of the shadow test depending on the texture format or compare mode.

[1] http://http.developer.nvidia.com/Cg/tex2Dproj.html

https://bugs.freedesktop.org/show_bug.cgi?id=66067

--- Comment #33 from Marek Olšák maraeo@gmail.com --- Our hardware doesn't have a compare mode state in the sampler. We tried to emulate it in the shader, but the performance was bad. Thus, there is no plan to fix misbehaving apps at the moment.

https://bugs.freedesktop.org/show_bug.cgi?id=66067

--- Comment #35 from Roland Scheidegger sroland@vmware.com --- (In reply to Daniel Scharrer from comment #34)

Came across another game that does this wrong: Never Alone (http://store.steampowered.com/app/295790) However this one does need shadow comparison in other shaders.

I've contacted the developers about this (as I have for the other affected games) but I fear that this problem will just keep coming up - especially with the NVIDIA driver ignoring the shadow sampler from the shader and Catalyst also having a workaround (at least the Trine games rendered correctly with it the last time I tried). Additionally at one of the developers I have contacted suggested they are unlikely to release another patch so this will likely remain broken.

Would it be possible to - instead of branching in the shader - create shader variants if the TEXTURE_COMPARE_MODE does not match up with the use in the shader?

Possible, sure. Essentially you'd have to put a mask into the shader key indicating which texture units have the PIPE_TEX_COMPARE_R_TO_TEXTURE bit set in the sampler (well, for used units and only those which actually have shadow targets). But then you'd have a dependency on sampler changes so you'd need to recheck that on changes there and recompile (well, recompilation shouldn't be an issue, as you'd only ever have to do it for those totally broken shaders, and only once for each shader).

I blame cg - makes it super easy to mistakenly use the shadow variant of a texture instruction. At least I haven't seen anyone mistakenly using shadow variant with glsl... I'm wondering though why anyone is still using that stuff.

Again, would be good to know how the blob handles this.

It would be possible to recognize this bug only on ARB_program shaders, so you only have some extra cost there - that is make some shader key there and recompile if it doesn't match currently bound samplers. This actually should be easier - core gl requires depth_compare_mode being ignored for non-depth textures (which is why the sampler code in mesa/st checks the base format and only sets depth compare mode with depth textures) but there is no such requirement in ARB_fp_shadow as this needs to match (well, just as it needs to match the target in the shader... just don't tell me the apps don't get this right neither...). This means you don't have a dependency on the bound textures, only samplers. I suppose it could be done in the mesa state tracker that way outside of drivers but I'm not sure if anyone is really thrilled... Those shaders are really simply terribly broken, broken, broken (and people are probably less interested in trying to come up with some quite hacky, creative workarounds if it's really clearly the fault of others...) - there's very good reasons behavior is undefined for such shaders.

https://bugs.freedesktop.org/show_bug.cgi?id=66067

--- Comment #37 from Nicholas Miell nmiell@gmail.com --- You could look for "# cgc version" near the beginning of the program source (almost certainly the second line), but that assumes the shaders weren't post-processed to strip out all the comments.

Alternately, you could just disable it entirely by default and include a driconf option.

The fact remains that the games work correctly on other OpenGL implementations and are ancient enough that they're no longer supported and will never be fixed.

https://bugs.freedesktop.org/show_bug.cgi?id=66067

GitLab Migration User gitlab-migration@fdo.invalid changed:

What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |RESOLVED Resolution|--- |MOVED

--- Comment #39 from GitLab Migration User gitlab-migration@fdo.invalid --- -- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/mesa/mesa/issues/444.