I'll second Topi's comment. Other than that, not much jumpped out at me. I did make a comment on 7 which also applies (somewhat) to 8 and I made another comment on 5. However, none of this is blockers so this series is
Reviewed-by: Jason Ekstrand <jason.ekstr...@intel.com> On Wed, Mar 11, 2015 at 11:59 AM, Kenneth Graunke <kenn...@whitecape.org> wrote: > On Wednesday, March 11, 2015 10:44:26 AM Pohjolainen, Topi wrote: > > On Mon, Mar 09, 2015 at 01:58:51AM -0700, Kenneth Graunke wrote: > > > The NIR backend hardcodes brw_wm_prog_key at the moment, which won't > > > work when we support scalar VS. We could use get_tex(), but it's a > > > static method. I was going to promote it to fs_visitor, but then > > > realized that both parameters (stage and key) are already members. > > > > > > It then occured to me that we could just set up a pointer in the > > > constructor, and skip having a function altogether. > > > > > > This patch also converts all existing users to use key_tex. > > > > > > Signed-off-by: Kenneth Graunke <kenn...@whitecape.org> > > > > With the few nits: > > > > Reviewed-by: Topi Pohjolainen <topi.pohjolai...@intel.com> > > > > > --- > > > src/mesa/drivers/dri/i965/brw_fs.h | 2 + > > > src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 3 +- > > > src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 55 > ++++++++++++---------------- > > > 3 files changed, 27 insertions(+), 33 deletions(-) > > > > > > diff --git a/src/mesa/drivers/dri/i965/brw_fs.h > b/src/mesa/drivers/dri/i965/brw_fs.h > > > index ee6ba98..7f4916a 100644 > > > --- a/src/mesa/drivers/dri/i965/brw_fs.h > > > +++ b/src/mesa/drivers/dri/i965/brw_fs.h > > > @@ -418,6 +418,8 @@ public: > > > void visit_atomic_counter_intrinsic(ir_call *ir); > > > > > > const void *const key; > > > + struct brw_sampler_prog_key_data *key_tex; > > > > This could be constant pointer as well, we only use it for reading in the > > visitor. (Also prevents the need to cast the constant 'key' pointer to > > non-constant in init()). > > Good catch! I've changed it to: > > const struct brw_sampler_prog_key_Data *key_tex; > > and changed the casts in init() to be "const struct brw_wm_prog_key" and > so on. I couldn't add the second 'const' (which prevents assignments to > key_tex) since I can't really set this from a constructor initializer > list - I need the switch statement in init(). But, this is probably the > most important one. > > Also fixed the word wrapping. Thanks! > > _______________________________________________ > 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