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

[Brian Paul]

On Tue, Dec 12, 2017 at 7:34 PM, <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_SECO
> ND_NONNAN);
> +      coords[1] = lp_build_max_ext(coord_bld, coords[1], coord_bld->zero,
> +                                   GALLIVM_NAN_RETURN_OTHER_SECO
> ND_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);
> --
>

Would it make sense to have a gallivm helper function for doing this?  Or
two, for min/max?

In any case, for both patches,
Reviewed-by: Brian Paul <brianp at vmware.com>


2.7.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20171212/55c83f54/attachment.html>