<div dir="ltr">On Tue, Dec 12, 2017 at 7:34 PM, <span dir="ltr"><<a href="mailto:sroland@vmware.com" target="_blank">sroland@vmware.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">From: Roland Scheidegger <<a href="mailto:sroland@vmware.com" target="_blank">sroland@vmware.com</a>><br>
<br>
Cube texture wrapping is a bit special since the values (post face<br>
projection) always are within [0,1], so we took advantage of that and<br>
omitted some clamps.<br>
However, we can still get NaNs (either because the coords already had NaNs,<br>
or the face projection generated them), and in fact we didn't handle them<br>
quite safely. I've seen -INT_MAX + 1 been propagated through as the final int<br>
coord value, albeit I didn't observe a crash. (Not quite a coincidence, since<br>
any stride mul with -INT_MAX or -INT_MAX+1 will turn up as a small positive<br>
number - nevertheless, I'd rather not try my luck, I'm not entirely sure it<br>
can't really turn up negative neither due to seamless coord swapping, plus<br>
ifloor of a NaN is not guaranteed to return -INT_MAX by any standard. And<br>
we kill off NaNs similarly with ordinary texture wrapping too.)<br>
So kill off the NaNs by using the common max against zero method.<br>
---<br>
src/gallium/auxiliary/<wbr>gallivm/lp_bld_sample_soa.c | 11 +++++++++++<br>
1 file changed, 11 insertions(+)<br>
<br>
diff --git a/src/gallium/auxiliary/galliv<wbr>m/lp_bld_sample_soa.c b/src/gallium/auxiliary/galliv<wbr>m/lp_bld_sample_soa.c<br>
index 571a968..ff8cbf6 100644<br>
--- a/src/gallium/auxiliary/galliv<wbr>m/lp_bld_sample_soa.c<br>
+++ b/src/gallium/auxiliary/galliv<wbr>m/lp_bld_sample_soa.c<br>
@@ -1123,6 +1123,17 @@ lp_build_sample_image_linear(s<wbr>truct lp_build_sample_context *bld,<br>
*/<br>
/* should always have normalized coords, and offsets are undefined */<br>
assert(bld->static_sampler_st<wbr>ate->normalized_coords);<br>
+ /*<br>
+ * The coords should all be between [0,1] however we can have NaNs,<br>
+ * which will wreak havoc. In particular the y1_clamped value below<br>
+ * can be -INT_MAX (on x86) and be propagated right through (probably<br>
+ * other values might be bogus in the end too).<br>
+ * So kill off the NaNs here.<br>
+ */<br>
+ coords[0] = lp_build_max_ext(coord_bld, coords[0], coord_bld->zero,<br>
+ GALLIVM_NAN_RETURN_OTHER_SECO<wbr>ND_NONNAN);<br>
+ coords[1] = lp_build_max_ext(coord_bld, coords[1], coord_bld->zero,<br>
+ GALLIVM_NAN_RETURN_OTHER_SECO<wbr>ND_NONNAN);<br>
coord = lp_build_mul(coord_bld, coords[0], flt_width_vec);<br>
/* instead of clamp, build mask if overflowed */<br>
coord = lp_build_sub(coord_bld, coord, half);<br>
<span class="m_-6504999156020544365HOEnZb"><font color="#888888">--<br></font></span></blockquote><div><br></div><div>Would it make sense to have a gallivm helper function for doing this? Or two, for min/max?</div><div><br></div><div>In any case, for both patches,<br></div><div>Reviewed-by: Brian Paul <<a href="mailto:brianp@vmware.com" target="_blank">brianp@vmware.com</a>></div><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="m_-6504999156020544365HOEnZb"><font color="#888888">
2.7.4<br>
<br>
______________________________<wbr>_________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/<wbr>mailman/listinfo/mesa-dev</a><br>
</font></span></blockquote></div><br></div></div>