From 77554d220d6d74b4d913dc37ea3a874e9dc550e4 Mon Sep 17 00:00:00 2001 From: Roland Scheidegger Date: Tue, 24 Apr 2018 18:25:55 +0200 Subject: [PATCH] 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 --- 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 ff80363a51e..46118b6e67d 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 ); -- 2.30.2