[Mesa-dev] [PATCH 1/7] softpipe: Move the faces array from view to filter_args

[Brian Paul]

On 09/11/2015 04:49 PM, Krzesimir Nowak wrote:
> On Fri, Sep 11, 2015 at 11:57 PM, Emil Velikov <emil.l.velikov at gmail.com
> <mailto:emil.l.velikov at gmail.com>> wrote:
>
>     On 11 September 2015 at 19:07, Krzesimir Nowak <krzesimir at kinvolk.io
>     <mailto:krzesimir at kinvolk.io>> wrote:
>     > With that, sp_sampler_view instances are not abused anymore as a local
>     > storage, so we can later make them constant.
>     > ---
>     >  src/gallium/drivers/softpipe/sp_tex_sample.c | 36 +++++++++++++++++-----------
>     >  src/gallium/drivers/softpipe/sp_tex_sample.h |  4 +---
>     >  2 files changed, 23 insertions(+), 17 deletions(-)
>     >
>     > diff --git a/src/gallium/drivers/softpipe/sp_tex_sample.c b/src/gallium/drivers/softpipe/sp_tex_sample.c
>     > index 489cae7..d5a7ed6 100644
>     > --- a/src/gallium/drivers/softpipe/sp_tex_sample.c
>     > +++ b/src/gallium/drivers/softpipe/sp_tex_sample.c
>
>     > @@ -3594,11 +3596,16 @@ sp_tgsi_get_samples(struct tgsi_sampler *tgsi_sampler,
>     >        float cs[TGSI_QUAD_SIZE];
>     >        float ct[TGSI_QUAD_SIZE];
>     >        float cp[TGSI_QUAD_SIZE];
>     > +      float faces[TGSI_QUAD_SIZE];
>     >
>     > -      convert_cube(sp_sview, sp_samp, s, t, p, c0, cs, ct, cp);
>     > +      convert_cube(sp_sview, sp_samp, s, t, p, c0, cs, ct, cp, faces);
>     >
>     > +      filt_args.faces = faces;
>     If I remember it correctly the contents of faces will become invalid
>     and as we exit the function, thus any attempt to use them (via
>     filt_args.faces) and things will go crazy.
>
>
> And that's fine - filt_args variable itself goes out of scope when you
> exit the function. And we do not store the pointer to faces anywhere for
> later reuse or anything.
>
>
>     >        sample_mip(sp_sview, sp_samp, cs, ct, cp, c0, lod, &filt_args, rgba);
>     >     } else {
>     > +      static const float zero_faces[TGSI_QUAD_SIZE] = {0.0f, 0.0f, 0.0f, 0.0f};
>     > +
>     > +      filt_args.faces = zero_faces;
>     Here we should be safe due to the static qualifier.
>
>     >        sample_mip(sp_sview, sp_samp, s, t, p, c0, lod, &filt_args, rgba);
>     >     }
>     >  }
>
>     > diff --git a/src/gallium/drivers/softpipe/sp_tex_sample.h b/src/gallium/drivers/softpipe/sp_tex_sample.h
>     > index 72b4a1a..6743b7e 100644
>     > --- a/src/gallium/drivers/softpipe/sp_tex_sample.h
>     > +++ b/src/gallium/drivers/softpipe/sp_tex_sample.h
>     > @@ -72,6 +72,7 @@ typedef void (*img_filter_func)(struct sp_sampler_view *sp_sview,
>     >  struct filter_args {
>     >     enum tgsi_sampler_control control;
>     >     const int8_t *offset;
>     > +   const float *faces;
>     Afaict during calculation of face (in convert_cube) uint type is used.
>     Won't this cause unnecessary int <> float conversions ?
>
>
> Good point, I haven't noticed that.

Me either. :(

I already pushed the series, so perhaps you can follow-up with a 
one-line fix.

-Brian