[Mesa-dev] [PATCH 1/3] glsl: add gl_HelperInvocation system value

[Ilia Mirkin]

On Tue, Sep 15, 2015 at 12:51 AM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> On Tue, Sep 15, 2015 at 12:41 AM, Tapani Pälli <tapani.palli at intel.com> wrote:
>>
>>
>> On 09/15/2015 12:43 AM, Ilia Mirkin wrote:
>>>
>>> On Mon, Sep 14, 2015 at 4:35 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>>>>
>>>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>>>> ---
>>>>   src/glsl/builtin_variables.cpp             | 3 +++
>>>>   src/glsl/shader_enums.h                    | 1 +
>>>>   src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 1 +
>>>>   3 files changed, 5 insertions(+)
>>>>
>>>> diff --git a/src/glsl/builtin_variables.cpp
>>>> b/src/glsl/builtin_variables.cpp
>>>> index b5e2908..7ea5db8 100644
>>>> --- a/src/glsl/builtin_variables.cpp
>>>> +++ b/src/glsl/builtin_variables.cpp
>>>> @@ -1050,6 +1050,9 @@
>>>> builtin_variable_generator::generate_fs_special_vars()
>>>>         add_input(VARYING_SLOT_LAYER, int_t, "gl_Layer");
>>>>         add_input(VARYING_SLOT_VIEWPORT, int_t, "gl_ViewportIndex");
>>>>      }
>>>> +
>>>> +   if (state->is_version(450, 310))
>>>
>>>
>>> Hmmm... this should probably be
>>>
>>> state->ARB_ES3_1_compatibility_enable || state->is_version(450, 310).
>>>
>>> Since that doesn't exist, perhaps i should just add it in /* ... */ to
>>> be uncommented later? Or I guess I could go in and add all of those
>>> ext enables in. What do people think? It's easy enough to do any of
>>> these.
>>
>>
>> IMO comments would be enough for now, we can worry about ES3_1_compatibility
>> when we have 3.1 done.
>
> OK, that was my feeling as well.
>
>>
>>>> +      add_system_value(SYSTEM_VALUE_FRAG_HELPER, bool_t,
>>>> "gl_HelperInvocation");
>>>>   }
>>>>
>>>>
>>>> diff --git a/src/glsl/shader_enums.h b/src/glsl/shader_enums.h
>>>> index 7c598b6..6bc93a5 100644
>>>> --- a/src/glsl/shader_enums.h
>>>> +++ b/src/glsl/shader_enums.h
>>>> @@ -352,6 +352,7 @@ typedef enum
>>>>      SYSTEM_VALUE_SAMPLE_ID,
>>>>      SYSTEM_VALUE_SAMPLE_POS,
>>>>      SYSTEM_VALUE_SAMPLE_MASK_IN,
>>>> +   SYSTEM_VALUE_FRAG_HELPER,
>>
>>
>> Why not call it SYSTEM_VALUE_HELPER_INVOCATION? Most builtin variables are
>> named with matching names to spec.
>
> I didn't want something so unnecessarily long. Here are some counterexamples:
>
> ....
>
> I could have sworn there were some, but apparently not :) The
> VARYING_SLOT_* are a lot more counter-example-ish, like
> VARYING_SLOT_POS, VARYING_SLOT_FACE, VARYING_SLOT_PNTC, etc.
>
> Anyone else have thoughts on how these should be named?

ES 3.1 appears to be getting closer to reality... probably good to
close this one out... Reviews/opinions welcome.