[Mesa-dev] [PATCH 2/2] gallivm: fix an issue with NaNs with seamless cube filtering

[Jose Fonseca]

On 13/12/17 02:34, sroland at vmware.com wrote:
> From: Roland Scheidegger <sroland at vmware.com>
> 
> Cube texture wrapping is a bit special since the values (post face
> projection) always are within [0,1], so we took advantage of that and
> omitted some clamps.
> However, we can still get NaNs (either because the coords already had NaNs,
> or the face projection generated them), and in fact we didn't handle them
> quite safely. I've seen -INT_MAX + 1 been propagated through as the final int
> coord value, albeit I didn't observe a crash. (Not quite a coincidence, since
> any stride mul with -INT_MAX or -INT_MAX+1 will turn up as a small positive
> number - nevertheless, I'd rather not try my luck, I'm not entirely sure it
> can't really turn up negative neither due to seamless coord swapping, plus
> ifloor of a NaN is not guaranteed to return -INT_MAX by any standard. And
> we kill off NaNs similarly with ordinary texture wrapping too.)
> So kill off the NaNs by using the common max against zero method.
> ---
>   src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c | 11 +++++++++++
>   1 file changed, 11 insertions(+)
> 
> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c b/src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c
> index 571a968..ff8cbf6 100644
> --- a/src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c
> +++ b/src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c
> @@ -1123,6 +1123,17 @@ lp_build_sample_image_linear(struct lp_build_sample_context *bld,
>          */
>         /* should always have normalized coords, and offsets are undefined */
>         assert(bld->static_sampler_state->normalized_coords);
> +      /*
> +       * The coords should all be between [0,1] however we can have NaNs,
> +       * which will wreak havoc. In particular the y1_clamped value below
> +       * can be -INT_MAX (on x86) and be propagated right through (probably
> +       * other values might be bogus in the end too).
> +       * So kill off the NaNs here.
> +       */
> +      coords[0] = lp_build_max_ext(coord_bld, coords[0], coord_bld->zero,
> +                                   GALLIVM_NAN_RETURN_OTHER_SECOND_NONNAN);
> +      coords[1] = lp_build_max_ext(coord_bld, coords[1], coord_bld->zero,
> +                                   GALLIVM_NAN_RETURN_OTHER_SECOND_NONNAN);
>         coord = lp_build_mul(coord_bld, coords[0], flt_width_vec);
>         /* instead of clamp, build mask if overflowed */
>         coord = lp_build_sub(coord_bld, coord, half);
> 

Series LGTM.

Reviewed-by: Jose Fonseca <jfonseca at vmware.com>