Module: Mesa Branch: master Commit: 6743c68a1190ffeedc0d124ca2dd9c22b3bc851c URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=6743c68a1190ffeedc0d124ca2dd9c22b3bc851c
Author: Roland Scheidegger <[email protected]> Date: Thu Dec 17 17:54:12 2015 +0100 draw: fix clip test with NaNs NaNs mean it should be clipped, otherwise the NaNs might get passed to the next stages (if clipping didn't happen for another reason already), which might cause all kind of problems. The llvm path got this right already (possibly by luck), but this isn't used when there's a gs active. Found by code inspection, verified with some hacked piglit test and some more hacked debug output. (Note the clipper can still itself incorrectly generate NaN and INF position values in its output prims (at least after w divide / viewport transform) even if the inputs weren't NaNs, if the position data of the vertices is "sufficiently bad".) Reviewed-by: Brian Paul <[email protected]> --- src/gallium/auxiliary/draw/draw_cliptest_tmp.h | 28 ++++++++++++------------ src/gallium/auxiliary/draw/draw_llvm.c | 4 ++++ 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/src/gallium/auxiliary/draw/draw_cliptest_tmp.h b/src/gallium/auxiliary/draw/draw_cliptest_tmp.h index c57e5f4..34add82 100644 --- a/src/gallium/auxiliary/draw/draw_cliptest_tmp.h +++ b/src/gallium/auxiliary/draw/draw_cliptest_tmp.h @@ -94,30 +94,31 @@ static boolean TAG(do_cliptest)( struct pt_post_vs *pvs, out->clip_pos[i] = position[i]; } + /* Be careful with NaNs. Comparisons must be true for them. */ /* Do the hardwired planes first: */ if (flags & DO_CLIP_XY_GUARD_BAND) { - if (-0.50 * position[0] + position[3] < 0) mask |= (1<<0); - if ( 0.50 * position[0] + position[3] < 0) mask |= (1<<1); - if (-0.50 * position[1] + position[3] < 0) mask |= (1<<2); - if ( 0.50 * position[1] + position[3] < 0) mask |= (1<<3); + if (!(-0.50 * position[0] + position[3] >= 0)) mask |= (1<<0); + if (!( 0.50 * position[0] + position[3] >= 0)) mask |= (1<<1); + if (!(-0.50 * position[1] + position[3] >= 0)) mask |= (1<<2); + if (!( 0.50 * position[1] + position[3] >= 0)) mask |= (1<<3); } else if (flags & DO_CLIP_XY) { - if (-position[0] + position[3] < 0) mask |= (1<<0); - if ( position[0] + position[3] < 0) mask |= (1<<1); - if (-position[1] + position[3] < 0) mask |= (1<<2); - if ( position[1] + position[3] < 0) mask |= (1<<3); + if (!(-position[0] + position[3] >= 0)) mask |= (1<<0); + if (!( position[0] + position[3] >= 0)) mask |= (1<<1); + if (!(-position[1] + position[3] >= 0)) mask |= (1<<2); + if (!( position[1] + position[3] >= 0)) mask |= (1<<3); } /* Clip Z planes according to full cube, half cube or none. */ if (flags & DO_CLIP_FULL_Z) { - if ( position[2] + position[3] < 0) mask |= (1<<4); - if (-position[2] + position[3] < 0) mask |= (1<<5); + if (!( position[2] + position[3] >= 0)) mask |= (1<<4); + if (!(-position[2] + position[3] >= 0)) mask |= (1<<5); } else if (flags & DO_CLIP_HALF_Z) { - if ( position[2] < 0) mask |= (1<<4); - if (-position[2] + position[3] < 0) mask |= (1<<5); + if (!( position[2] >= 0)) mask |= (1<<4); + if (!(-position[2] + position[3] >= 0)) mask |= (1<<5); } if (flags & DO_CLIP_USER) { @@ -144,7 +145,7 @@ static boolean TAG(do_cliptest)( struct pt_post_vs *pvs, if (clipdist < 0 || util_is_inf_or_nan(clipdist)) mask |= 1 << plane_idx; } else { - if (dot4(clipvertex, plane[plane_idx]) < 0) + if (!(dot4(clipvertex, plane[plane_idx]) >= 0)) mask |= 1 << plane_idx; } } @@ -190,7 +191,6 @@ static boolean TAG(do_cliptest)( struct pt_post_vs *pvs, out = (struct vertex_header *)( (char *)out + info->stride ); } - return need_pipeline != 0; } diff --git a/src/gallium/auxiliary/draw/draw_llvm.c b/src/gallium/auxiliary/draw/draw_llvm.c index 89ed045..f25dafe 100644 --- a/src/gallium/auxiliary/draw/draw_llvm.c +++ b/src/gallium/auxiliary/draw/draw_llvm.c @@ -1201,6 +1201,10 @@ generate_clipmask(struct draw_llvm *llvm, cv_w = pos_w; } + /* + * Be careful with the comparisons and NaNs (using llvm's unordered + * comparisons here). + */ /* Cliptest, for hardwired planes */ /* * XXX should take guardband into account (currently not in key). _______________________________________________ mesa-commit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-commit
