[Mesa-dev] [PATCH] glsl: allow local_size qualifiers in compute shader regardless of version

[Ilia Mirkin]

On Fri, Aug 26, 2016 at 3:08 PM, Miklós Máté <mtmkls at 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é <mtmkls at 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é <mtmkls at 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é <mtmkls at 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