[Mesa-dev] [PATCH] glsl: add EXT_clip_cull_distance support based on ARB_cull_distance

[Emil Velikov]

On 24 May 2016 at 16:19, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> On Tue, May 24, 2016 at 11:11 AM, Tobias Klausmann
> <tobias.johannes.klausmann at mni.thm.de> wrote:
>>
>>
>> On 24.05.2016 03:42, Ilia Mirkin wrote:
>>>
>>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>>> ---
>>>   src/compiler/glsl/builtin_variables.cpp  | 12 +++++++----
>>>   src/compiler/glsl/glsl_parser_extras.cpp |  1 +
>>>   src/compiler/glsl/glsl_parser_extras.h   |  2 ++
>>>   src/compiler/glsl/linker.cpp             | 35
>>> ++++++++++++++++++--------------
>>>   src/mesa/main/extensions_table.h         |  1 +
>>>   5 files changed, 32 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/src/compiler/glsl/builtin_variables.cpp
>>> b/src/compiler/glsl/builtin_variables.cpp
>>> index e899f01..3d34028 100644
>>> --- a/src/compiler/glsl/builtin_variables.cpp
>>> +++ b/src/compiler/glsl/builtin_variables.cpp
>>> @@ -674,11 +674,14 @@ builtin_variable_generator::generate_constants()
>>>                   state->Const.MaxProgramTexelOffset);
>>>      }
>>>   -   if (state->is_version(130, 0)) {
>>> +   if (state->is_version(130, 0) || state->EXT_clip_cull_distance_enable)
>>> {
>>>         add_const("gl_MaxClipDistances", state->Const.MaxClipPlanes);
>>
>>
>> Add a has_clip_distance() func?
>>
>>> +   }
>>> +   if (state->is_version(130, 0)) {
>>>         add_const("gl_MaxVaryingComponents", state->ctx->Const.MaxVarying
>>> * 4);
>>>      }
>>> -   if (state->is_version(450, 0) || state->ARB_cull_distance_enable) {
>>> +   if (state->is_version(450, 0) || state->ARB_cull_distance_enable ||
>>> +       state->EXT_clip_cull_distance_enable) {
>>
>>
>> For the last series cull series in review, it was noted that a
>> has_cull_distance() func would be nice to for these checks (dave didn't see
>> that i guess). As you are touching it now again, maybe introduce that one,
>> when you are at it :)
>>
>> Other than that i think you are missing the entry in the parser for:
>>
>> #extension GL_EXT_clip_cull_distance : <behavior>
>
> You mean the #define. Oops :) Will do that.
>
>>
>> Other than that the patch looks fine to me!
>
> FWIW I've already pushed this with Dave's R-b (done over IRC last
> night). I'm personally not a huge fan of all the ->has_foo() helpers.
> I guess it could make sense here though. I'll send something out
> tonight.
>
if you can add a line in docs/relnotes/11.3.0.html that'll be great as well :-)

Thanks
Emil