<div dir="ltr">On Fri, Sep 11, 2015 at 11:57 PM, Emil Velikov <span dir="ltr"><<a href="mailto:emil.l.velikov@gmail.com" target="_blank">emil.l.velikov@gmail.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On 11 September 2015 at 19:07, Krzesimir Nowak <<a href="mailto:krzesimir@kinvolk.io">krzesimir@kinvolk.io</a>> wrote:<br>
> With that, sp_sampler_view instances are not abused anymore as a local<br>
> storage, so we can later make them constant.<br>
> ---<br>
>  src/gallium/drivers/softpipe/sp_tex_sample.c | 36 +++++++++++++++++-----------<br>
>  src/gallium/drivers/softpipe/sp_tex_sample.h |  4 +---<br>
>  2 files changed, 23 insertions(+), 17 deletions(-)<br>
><br>
> diff --git a/src/gallium/drivers/softpipe/sp_tex_sample.c b/src/gallium/drivers/softpipe/sp_tex_sample.c<br>
> index 489cae7..d5a7ed6 100644<br>
> --- a/src/gallium/drivers/softpipe/sp_tex_sample.c<br>
> +++ b/src/gallium/drivers/softpipe/sp_tex_sample.c<br>
<br>
</span><span class="">> @@ -3594,11 +3596,16 @@ sp_tgsi_get_samples(struct tgsi_sampler *tgsi_sampler,<br>
>        float cs[TGSI_QUAD_SIZE];<br>
>        float ct[TGSI_QUAD_SIZE];<br>
>        float cp[TGSI_QUAD_SIZE];<br>
> +      float faces[TGSI_QUAD_SIZE];<br>
><br>
> -      convert_cube(sp_sview, sp_samp, s, t, p, c0, cs, ct, cp);<br>
> +      convert_cube(sp_sview, sp_samp, s, t, p, c0, cs, ct, cp, faces);<br>
><br>
> +      filt_args.faces = faces;<br>
</span>If I remember it correctly the contents of faces will become invalid<br>
and as we exit the function, thus any attempt to use them (via<br>
filt_args.faces) and things will go crazy.<br></blockquote><div><br></div><div>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.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
>        sample_mip(sp_sview, sp_samp, cs, ct, cp, c0, lod, &filt_args, rgba);<br>
>     } else {<br>
> +      static const float zero_faces[TGSI_QUAD_SIZE] = {0.0f, 0.0f, 0.0f, 0.0f};<br>
> +<br>
> +      filt_args.faces = zero_faces;<br>
</span>Here we should be safe due to the static qualifier.<br>
<span class=""><br>
>        sample_mip(sp_sview, sp_samp, s, t, p, c0, lod, &filt_args, rgba);<br>
>     }<br>
>  }<br>
<br>
</span><span class="">> diff --git a/src/gallium/drivers/softpipe/sp_tex_sample.h b/src/gallium/drivers/softpipe/sp_tex_sample.h<br>
> index 72b4a1a..6743b7e 100644<br>
> --- a/src/gallium/drivers/softpipe/sp_tex_sample.h<br>
> +++ b/src/gallium/drivers/softpipe/sp_tex_sample.h<br>
> @@ -72,6 +72,7 @@ typedef void (*img_filter_func)(struct sp_sampler_view *sp_sview,<br>
>  struct filter_args {<br>
>     enum tgsi_sampler_control control;<br>
>     const int8_t *offset;<br>
> +   const float *faces;<br>
</span>Afaict during calculation of face (in convert_cube) uint type is used.<br>
Won't this cause unnecessary int <> float conversions ?<br>
<br></blockquote><div><br></div><div>Good point, I haven't noticed that.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Cheers,<br>
Emil<br>
</blockquote></div><br></div></div>