[Mesa-dev] [PATCH v2 2/3] mesa: actually support GLSL version overrides in compat profile

[Emil Velikov]

On 3 May 2018 at 00:10, Timothy Arceri <tarceri at itsqueeze.com> wrote:
>
>
> On 03/05/18 02:51, Emil Velikov wrote:
>>
>> On 2 May 2018 at 11:27, Timothy Arceri <tarceri at itsqueeze.com> wrote:
>>>
>>> ---
>>>   src/mesa/main/version.c | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/src/mesa/main/version.c b/src/mesa/main/version.c
>>> index 84babd69e2f..540f5482034 100644
>>> --- a/src/mesa/main/version.c
>>> +++ b/src/mesa/main/version.c
>>> @@ -591,6 +591,8 @@ _mesa_get_version(const struct gl_extensions
>>> *extensions,
>>>            if (consts->GLSLVersion > 140) {
>>>               consts->GLSLVersion = 140;
>>>            }
>>> +         /* Support GLSL version overrides in compat profile */
>>> +         _mesa_override_glsl_version(consts);
>>
>>
>> Why are we allowing this only for compat? As-is this feels very dirty
>> and skimming through the existing code doesn't help much.
>
>
> Because the code above this hard-codes compat to glsl 140. We have no
> consts->GLSLVersionCompat (and probably shouldn't) so drivers will otherwise
> set what ever they support in core to compat. This is how the code currently
> works and this patch allows the override to work as expected.
>
>>
>> * classic drivers - 965, starting at create_context
>> _mesa_initialize_context -> _mesa_init_constants -> GLSLVersion (120) +
>> override
>> intelInitExtensions -> GLSLVersion + override combo
>> _mesa_compute_version -> _mesa_get_version -> [optional] cap up-to 140
>> -> override
>> _mesa_compute_version -> tweak/match GLSL version based on the GL version
>>
>> Not to mention the initial 120 (effectively) in
>> _mesa_initialize_context is bonkers for the following:
>>   - Intel Gen2 (GL 1.3) and Gen3 (GL 1.4 or 2.1)
>>   - nouveau vieux - GL 1.2 or 1.3
>>   - radeon (r100/r200) - GL 1.3
>>
>> * gallium - two paths - create_screen and create_context, latter more
>> or less identical to i965
>> For the create_screen part:
>> st_api_query_versions (for max_gl*_version) -> _mesa_init_constants -> see
>> above
>> st_api_query_versions (for max_gl*_version) -> st_init_extensions ->
>> GLSLVersion + override combo
>> st_api_query_versions (for max_gl*_version) -> _mesa_get_version -> see
>> above
>>
>> As you can see things are hairy.
>>
>> A few ideas that come to mind:
>>   - drop the _mesa_init_constants bits and update any drivers needed
>>   - each of GLSLVersion, override and tweaks should happen [ideally]
>> once per ctx.
>
>
> I have no interest in tackling this right now. I disagree that this change
> requires such a big overhaul. Right now this fixes a bug, we can tidy up
> later. We are likely going to need to tweak how versioning works once 3.2
> compat profiles become supported.
>
The gallium suggestion is the big overhaul, the couple above are dead trivial.
Most of the work is tracking the lovely codepaths.

That said, I'm fine with this if a) some of the 'why' goes into the
commit message and b) things are fixed before/as we get GL 3.2 compat.
Just let's agree that env. overrides and drirc workarounds are mere hacks.

Thanks
Emil