On 26/08/16 21:17, Ilia Mirkin wrote:
On Fri, Aug 26, 2016 at 3:08 PM, Miklós Máté <mtm...@gmail.com> wrote:
On 08/26/2016 07:47 PM, Ilia Mirkin wrote:
On Fri, Aug 26, 2016 at 1:39 PM, Miklós Máté <mtm...@gmail.com> wrote:
On 08/26/2016 06:46 PM, Ilia Mirkin wrote:
On Fri, Aug 26, 2016 at 12:42 PM, Miklós Máté <mtm...@gmail.com> wrote:
Tomb Raider 2013 uses #version 420 in compute shaders, and current Mesa
rejects them, because the local size qualifiers require 430.

Signed-off-by: Miklós Máté <mtm...@gmail.com>
---
    src/compiler/glsl/glsl_parser.yy | 5 ++---
    1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/compiler/glsl/glsl_parser.yy
b/src/compiler/glsl/glsl_parser.yy
index 4043dae..6857443 100644
--- a/src/compiler/glsl/glsl_parser.yy
+++ b/src/compiler/glsl/glsl_parser.yy
@@ -1615,10 +1615,9 @@ layout_qualifier_id:
          for (int i = 0; i < 3; i++) {
             if (match_layout_qualifier(local_size_qualifiers[i], $1,
                                        state) == 0) {
-            if (!state->has_compute_shader()) {
IIRC this is the only use of ->has_compute_shader() so you could also
remove that function. I sent a similar patch earlier that also removed
the GL_ARB_compute_shader enable as it's not anywhere in the specs,
but was told that a lot of people had stuck it in their shaders
anyways, so I guess that bit has to stay.

Chances are, though, that you want to go through and enable bits like
shared memory, etc, not just the local_size. Have a look at

https://patchwork.freedesktop.org/patch/107119/

Feel free to respin it without the removal of the extension enable.
I think your patch is much more comprehensive, my patch only makes Tomb
Raider happy. Without the removal of the enable bit, it looks good to me.
BTW, would I be correct in guessing that you have a IVB/Haswell, and
have applied the fp64 patches, but still don't have
GL_ARB_stencil_texturing and thus no GL 4.3? I'm pretty sure others
have been successful in running it on mesa.

    -ilia

I have radeonsi with GL 4.3, but it seems the #version 420 directive is what
matters for the compiler. I did the patch based on replaying the trace
linked in https://bugs.freedesktop.org/show_bug.cgi?id=95190 with glretrace,
The game itself seems to swallow all errors, and it just goes on without the
compute shaders silently if they fail to compile. Setting stuff to "ultra"
and "tressfx" require compute, as those are unavailable with GL 4.1, but I
don't see any difference when they are enabled. At least the hair is
supposed to look much different with tressfx AFAIK. Maybe something is still
missing...
Ah, perhaps it was captured with a GL 4.2-exposing driver then. I
think a lot of these games just stick "#version <latest supported by
driver>" into their shaders (which IMHO is a mistake, but ... wtvr).
Although it was Karol doing the trace, which likely means nouveau.
Perhaps it was during the short period of time that we had all the GL
4.3 features available but were too chicken to enable it.

   -ilia

Hi,

sorry for the long delay. I now captured a trace with GL 4.3-capable radeonsi, and you're right, the game inserts "#version <the highest supported>" into the shaders. When 430 is supported, everything works fine. Therefore, this patch is not required for Tomb Raider 2013, but I still think that compute shaders are supposed to work with "#version 420".

I found out that I wasn't able to see the difference in the hair quality, because the TressFX hair cannot be toggled on-the-fly. The game has to be restarted to apply this setting, and AFAICT this is the only setting that needs restart. In his video https://www.youtube.com/watch?v=kM2bfQ7apkQ TotalBiscuit can toggle it on-the-fly at 10:20, so this capability was somehow lost when the game was ported to Linux.

MM

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to