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 <brianp@vmware.com>
This commit is contained in:
Roland Scheidegger
2015-12-17 17:54:12 +01:00
parent 44e87b7b7b
commit 6743c68a11
2 changed files with 18 additions and 14 deletions

View File

@@ -94,30 +94,31 @@ static boolean TAG(do_cliptest)( struct pt_post_vs *pvs,
out->clip_pos[i] = position[i]; out->clip_pos[i] = position[i];
} }
/* Be careful with NaNs. Comparisons must be true for them. */
/* Do the hardwired planes first: /* Do the hardwired planes first:
*/ */
if (flags & DO_CLIP_XY_GUARD_BAND) { 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<<0);
if ( 0.50 * position[0] + position[3] < 0) mask |= (1<<1); 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<<2);
if ( 0.50 * position[1] + position[3] < 0) mask |= (1<<3); if (!( 0.50 * position[1] + position[3] >= 0)) mask |= (1<<3);
} }
else if (flags & DO_CLIP_XY) { else if (flags & DO_CLIP_XY) {
if (-position[0] + position[3] < 0) mask |= (1<<0); if (!(-position[0] + position[3] >= 0)) mask |= (1<<0);
if ( position[0] + position[3] < 0) mask |= (1<<1); 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<<2);
if ( position[1] + position[3] < 0) mask |= (1<<3); if (!( position[1] + position[3] >= 0)) mask |= (1<<3);
} }
/* Clip Z planes according to full cube, half cube or none. /* Clip Z planes according to full cube, half cube or none.
*/ */
if (flags & DO_CLIP_FULL_Z) { if (flags & DO_CLIP_FULL_Z) {
if ( position[2] + position[3] < 0) mask |= (1<<4); 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<<5);
} }
else if (flags & DO_CLIP_HALF_Z) { else if (flags & DO_CLIP_HALF_Z) {
if ( position[2] < 0) mask |= (1<<4); if (!( position[2] >= 0)) mask |= (1<<4);
if (-position[2] + position[3] < 0) mask |= (1<<5); if (!(-position[2] + position[3] >= 0)) mask |= (1<<5);
} }
if (flags & DO_CLIP_USER) { 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)) if (clipdist < 0 || util_is_inf_or_nan(clipdist))
mask |= 1 << plane_idx; mask |= 1 << plane_idx;
} else { } else {
if (dot4(clipvertex, plane[plane_idx]) < 0) if (!(dot4(clipvertex, plane[plane_idx]) >= 0))
mask |= 1 << plane_idx; 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 ); out = (struct vertex_header *)( (char *)out + info->stride );
} }
return need_pipeline != 0; return need_pipeline != 0;
} }

View File

@@ -1201,6 +1201,10 @@ generate_clipmask(struct draw_llvm *llvm,
cv_w = pos_w; cv_w = pos_w;
} }
/*
* Be careful with the comparisons and NaNs (using llvm's unordered
* comparisons here).
*/
/* Cliptest, for hardwired planes */ /* Cliptest, for hardwired planes */
/* /*
* XXX should take guardband into account (currently not in key). * XXX should take guardband into account (currently not in key).