[Mesa-stable] [Mesa-dev] [PATCH 18.0 v2] i965, gallium: Disable ARB_get_program_binary for compat profiles

[Marek Olšák]

Please keep the extension exposed in GL compat. Eventually we want to
expose all extensions in GL compat.

st_init_limits is the proper place - please use that. While
st_init_limits doesn't have the API parameter, both of its call sites
have it, so it's just a matter of passing it to the function.

Thanks,
Marek

On Fri, Feb 23, 2018 at 1:48 AM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> On Thu, Feb 22, 2018 at 7:41 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>> On Thu, Feb 22, 2018 at 7:39 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>>> On 23 February 2018 at 00:17, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>>>> On Thu, Feb 22, 2018 at 7:11 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>>>>>> diff --git a/src/mesa/state_tracker/st_context.c b/src/mesa/state_tracker/st_context.c
>>>>>> index d3e7d3fb7fa..4c14245a38f 100644
>>>>>> --- a/src/mesa/state_tracker/st_context.c
>>>>>> +++ b/src/mesa/state_tracker/st_context.c
>>>>>> @@ -518,6 +518,15 @@ st_create_context_priv(struct gl_context *ctx, struct pipe_context *pipe,
>>>>>>           ctx->Const.ShaderCompilerOptions[i].EmitNoIndirectSampler = true;
>>>>>>     }
>>>>>>
>>>>>> +   /* The QT framework has a bug in their shader program cache, which is built
>>>>>> +    * on GL_ARB_get_program_binary. In an effort to allow them to fix the bug
>>>>>> +    * we don't enable binary formats for compatibility profiles.
>>>>>> +    * This is only being done on the 18.0 release branch.
>>>>>> +    */
>>>>>> +   if (ctx->API == API_OPENGL_COMPAT) {
>>>>>> +      ctx->Const.NumProgramBinaryFormats = 0;
>>>>>> +   }
>>>>>> +
>>>>> Please move this alike the i965 instance -> adjust the constant within
>>>>> st_init_limits.
>>>>
>>>> Nope. It's in the right place where it is. (I put it there, Jordan
>>>> just added it to his patch.)
>>>>
>>> Silly question incoming - Why? It seems misleading to set the const in
>>> one place, only to change it elsewhere?
>>
>> Doing what you suggest won't compile. Limits (and extensions) are used
>> to infer ctx->API. So you can't set them based on ctx->API, it's
>> circular. So this, like a handful of other things, are
>> post-auto-everything fixups.
>
> Actually not strictly correct -- st_init_limits is used as part of the
> GLX_MESA_query_renderer dance. It does not currently receive a ctx
> because one doesn't necessarily exist, but it definitely knows what
> API it is in all cases, so that could be passed in as an argument.
>
> However that would be more change for what seems like no reason. Other
> dependencies can be more complicated (the circular dependencies start
> hitting when you start depending on versions of things), so the
> post-fixup approach is still going to be needed. And this will all get
> backed out, so no need to modify a bunch of surrounding code.
>
>   -ilia
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev