On 11/10/2015 01:06 PM, Ilia Mirkin wrote:
On Tue, Sep 15, 2015 at 12:51 AM, Ilia Mirkin <imir...@alum.mit.edu> wrote:
On Tue, Sep 15, 2015 at 12:41 AM, Tapani Pälli <tapani.pa...@intel.com> wrote:


On 09/15/2015 12:43 AM, Ilia Mirkin wrote:

On Mon, Sep 14, 2015 at 4:35 PM, Ilia Mirkin <imir...@alum.mit.edu> wrote:

Signed-off-by: Ilia Mirkin <imir...@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.


I vote again for using the exact name, it'll be easier to understand from the code later. You have my r-b with that change!

// Tapani
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to