On Tue, Dec 12, 2017 at 7:34 PM, <srol...@vmware.com> wrote: > From: Roland Scheidegger <srol...@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 <bri...@vmware.com> 2.7.4 > > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev