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

[Ilia Mirkin]

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.

  -ilia