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

[Ilia Mirkin]

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?

>
>
>>>      /*@}*/
>>>
>>>      /**
>>> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>>> b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>>> index c3a8c11..fc728d1 100644
>>> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>>> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>>> @@ -4614,6 +4614,7 @@ const unsigned
>>> _mesa_sysval_to_semantic[SYSTEM_VALUE_MAX] = {
>>>      TGSI_SEMANTIC_SAMPLEID,
>>>      TGSI_SEMANTIC_SAMPLEPOS,
>>>      TGSI_SEMANTIC_SAMPLEMASK,
>>> +   0, /* gl_HelperInvocation */
>>>
>>>      /* Tessellation shaders
>>>       */
>>> --
>>> 2.4.6
>>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>