Module: Mesa
Branch: master
Commit: 77554d220d6d74b4d913dc37ea3a874e9dc550e4
URL:
http://cgit.freedesktop.org/mesa/mesa/commit/?id=77554d220d6d74b4d913dc37ea3a874e9dc550e4
Author: Roland Scheidegger <srol...@vmware.com>
Date: Tue Apr 24 18:25:55 2018 +0200
draw: fix different sign logic when clipping
The logic was flawed, since mul(x,y) will be <= 0 (exactly 0) when
the sign is the same but both numbers are sufficiently small
(if the product is smaller than 2^-128).
This could apparently lead to emitting a sufficient amount of
additional bogus vertices to overflow the allocated array for them,
hitting an assertion (still safe with release builds since we just
aborted clipping after the assertion in this case - I'm however unsure
if this is now really no longer possible, so that code stays).
Not sure if the additional vertices could cause other grief, I didn't
see anything wrong even when hitting the assertion.
Essentially, both +-0 are treated as positive (the vertex is considered
to be inside the clip volume for this plane), so integrate the logic
determining different sign into the branch there.
Reviewed-by: Jose Fonseca <jfons...@vmware.com>
---
src/gallium/auxiliary/draw/draw_pipe_clip.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/src/gallium/auxiliary/draw/draw_pipe_clip.c
b/src/gallium/auxiliary/draw/draw_pipe_clip.c
index ff80363a51..46118b6e67 100644
--- a/src/gallium/auxiliary/draw/draw_pipe_clip.c
+++ b/src/gallium/auxiliary/draw/draw_pipe_clip.c
@@ -47,11 +47,6 @@
/** Set to 1 to enable printing of coords before/after clipping */
#define DEBUG_CLIP 0
-
-#ifndef DIFFERENT_SIGNS
-#define DIFFERENT_SIGNS(x, y) ((x) * (y) <= 0.0F && (x) - (y) != 0.0F)
-#endif
-
#define MAX_CLIPPED_VERTICES ((2 * (6 + PIPE_MAX_CLIP_PLANES))+1)
@@ -291,8 +286,8 @@ static void emit_poly(struct draw_stage *stage,
/*
* If we ever generated a tri (regardless if it had area or not),
* skip all subsequent null tris.
- * FIXME: it is unclear why we always have to emit at least one
- * tri. Maybe this is hiding bugs elsewhere.
+ * FIXME: I think this logic was hiding bugs elsewhere. It should
+ * be possible now to always emit all tris.
*/
if (tri_null && tri_emitted) {
continue;
@@ -478,6 +473,7 @@ do_clip_tri(struct draw_stage *stage,
for (i = 1; i <= n; i++) {
struct vertex_header *vert = inlist[i];
boolean *edge = &inEdges[i];
+ boolean different_sign;
float dp = getclipdist(clipper, vert, plane_idx);
@@ -490,9 +486,12 @@ do_clip_tri(struct draw_stage *stage,
return;
outEdges[outcount] = *edge_prev;
outlist[outcount++] = vert_prev;
+ different_sign = dp < 0.0f;
+ } else {
+ different_sign = !(dp < 0.0f);
}
- if (DIFFERENT_SIGNS(dp, dp_prev)) {
+ if (different_sign) {
struct vertex_header *new_vert;
boolean *new_edge;
@@ -510,7 +509,7 @@ do_clip_tri(struct draw_stage *stage,
if (dp < 0.0f) {
/* Going out of bounds. Avoid division by zero as we
- * know dp != dp_prev from DIFFERENT_SIGNS, above.
+ * know dp != dp_prev from different_sign, above.
*/
float t = dp / (dp - dp_prev);
interp( clipper, new_vert, t, vert, vert_prev, viewport_index );
_______________________________________________
mesa-commit mailing list
mesa-commit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-commit
