On Fri, Oct 09, 2015 at 05:53:43AM -0700, Jason Ekstrand wrote:
> On Fri, Oct 9, 2015 at 12:00 AM, Pohjolainen, Topi
> <topi.pohjolai...@intel.com> wrote:
> > On Thu, Oct 08, 2015 at 05:22:42PM -0700, Jason Ekstrand wrote:
> >> This is really an input into the shader compiler so it kind of makes sense
> >> in the key.  Also, given where it's placed into the key, it doesn't
> >> actually make it any bigger.
> >
> > But the key specifically controls recompiles, right? Before this didn't
> > trigger a recompile but now it does. I'm probably missing something...
> 
> You're not missing anything.  It can cause recompiles.  However,
> whether or not you're gles3 doesn't change that often. It can change
> due to doing meta operations but those shaders are tiny so it's
> probably not a big deal.
> 
> That said, this is one of the sketchier patches in the series.  I'm
> 100% ok with leaving it out and just passing use_legacy_snorm_formula
> as a parameter.  I was just hoping I could reduce the parameter bloat
> a little.

I don't mind changing this. I was just concerned if we had a bug earlier as we
didn't recompile when switching between gles3 and non-gles3.

> 
> >> ---
> >>  src/mesa/drivers/dri/i965/brw_program.h           | 2 ++
> >>  src/mesa/drivers/dri/i965/brw_vec4.cpp            | 3 +--
> >>  src/mesa/drivers/dri/i965/brw_vec4_vs_visitor.cpp | 9 ++++-----
> >>  src/mesa/drivers/dri/i965/brw_vs.c                | 3 +++
> >>  src/mesa/drivers/dri/i965/brw_vs.h                | 5 +----
> >>  5 files changed, 11 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/src/mesa/drivers/dri/i965/brw_program.h 
> >> b/src/mesa/drivers/dri/i965/brw_program.h
> >> index cf0522a..2a9d1e9 100644
> >> --- a/src/mesa/drivers/dri/i965/brw_program.h
> >> +++ b/src/mesa/drivers/dri/i965/brw_program.h
> >> @@ -91,6 +91,8 @@ struct brw_vs_prog_key {
> >>
> >>     bool clamp_vertex_color:1;
> >>
> >> +   bool use_legacy_snorm_formula:1;
> >> +
> >>     /**
> >>      * How many user clipping planes are being uploaded to the vertex 
> >> shader as
> >>      * push constants.
> >> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp 
> >> b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> >> index 4b8390f..4b03967 100644
> >> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> >> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> >> @@ -1992,8 +1992,7 @@ brw_vs_emit(struct brw_context *brw,
> >>
> >>        vec4_vs_visitor v(brw->intelScreen->compiler, brw, key, prog_data,
> >>                          vp->Base.nir, brw_select_clip_planes(&brw->ctx),
> >> -                        mem_ctx, shader_time_index,
> >> -                        !_mesa_is_gles3(&brw->ctx));
> >> +                        mem_ctx, shader_time_index);
> >>        if (!v.run()) {
> >>           if (prog) {
> >>              prog->LinkStatus = false;
> >> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_vs_visitor.cpp 
> >> b/src/mesa/drivers/dri/i965/brw_vec4_vs_visitor.cpp
> >> index 485a80e..9cf04cd 100644
> >> --- a/src/mesa/drivers/dri/i965/brw_vec4_vs_visitor.cpp
> >> +++ b/src/mesa/drivers/dri/i965/brw_vec4_vs_visitor.cpp
> >> @@ -77,7 +77,8 @@ vec4_vs_visitor::emit_prolog()
> >>              /* ES 3.0 has different rules for converting signed normalized
> >>               * fixed-point numbers than desktop GL.
> >>               */
> >> -            if ((wa_flags & BRW_ATTRIB_WA_SIGN) && 
> >> !use_legacy_snorm_formula) {
> >> +            if ((wa_flags & BRW_ATTRIB_WA_SIGN) &&
> >> +                !key->use_legacy_snorm_formula) {
> >>                 /* According to equation 2.2 of the ES 3.0 specification,
> >>                  * signed normalization conversion is done by:
> >>                  *
> >> @@ -304,14 +305,12 @@ vec4_vs_visitor::vec4_vs_visitor(const struct 
> >> brw_compiler *compiler,
> >>                                   const nir_shader *shader,
> >>                                   gl_clip_plane *clip_planes,
> >>                                   void *mem_ctx,
> >> -                                 int shader_time_index,
> >> -                                 bool use_legacy_snorm_formula)
> >> +                                 int shader_time_index)
> >>     : vec4_visitor(compiler, log_data, &key->tex, &vs_prog_data->base, 
> >> shader,
> >>                    mem_ctx, false /* no_spills */, shader_time_index),
> >>       key(key),
> >>       vs_prog_data(vs_prog_data),
> >> -     clip_planes(clip_planes),
> >> -     use_legacy_snorm_formula(use_legacy_snorm_formula)
> >> +     clip_planes(clip_planes)
> >>  {
> >>  }
> >>
> >> diff --git a/src/mesa/drivers/dri/i965/brw_vs.c 
> >> b/src/mesa/drivers/dri/i965/brw_vs.c
> >> index 38de98f..ecaeefa 100644
> >> --- a/src/mesa/drivers/dri/i965/brw_vs.c
> >> +++ b/src/mesa/drivers/dri/i965/brw_vs.c
> >> @@ -31,6 +31,7 @@
> >>
> >>
> >>  #include "main/compiler.h"
> >> +#include "main/context.h"
> >>  #include "brw_context.h"
> >>  #include "brw_vs.h"
> >>  #include "brw_util.h"
> >> @@ -329,6 +330,8 @@ brw_vs_populate_key(struct brw_context *brw,
> >>        key->clamp_vertex_color = ctx->Light._ClampVertexColor;
> >>     }
> >>
> >> +   key->use_legacy_snorm_formula = !_mesa_is_gles3(&brw->ctx);
> >> +
> >>     /* _NEW_POINT */
> >>     if (brw->gen < 6 && ctx->Point.PointSprite) {
> >>        for (i = 0; i < 8; i++) {
> >> diff --git a/src/mesa/drivers/dri/i965/brw_vs.h 
> >> b/src/mesa/drivers/dri/i965/brw_vs.h
> >> index c927cac..8cf9fa1 100644
> >> --- a/src/mesa/drivers/dri/i965/brw_vs.h
> >> +++ b/src/mesa/drivers/dri/i965/brw_vs.h
> >> @@ -91,8 +91,7 @@ public:
> >>                     const nir_shader *shader,
> >>                     gl_clip_plane *clip_planes,
> >>                     void *mem_ctx,
> >> -                   int shader_time_index,
> >> -                   bool use_legacy_snorm_formula);
> >> +                   int shader_time_index);
> >>
> >>  protected:
> >>     virtual dst_reg *make_reg_for_system_value(int location,
> >> @@ -113,8 +112,6 @@ private:
> >>     struct brw_vs_prog_data * const vs_prog_data;
> >>
> >>     gl_clip_plane *clip_planes;
> >> -
> >> -   bool use_legacy_snorm_formula;
> >>  };
> >>
> >>  } /* namespace brw */
> >> --
> >> 2.5.0.400.gff86faf
> >>
> >> _______________________________________________
> >> mesa-dev mailing list
> >> mesa-dev@lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to