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

[Ilia Mirkin]

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