From fbf95ce0742a4683d6a1a1a101fc7ef104c29886 Mon Sep 17 00:00:00 2001 From: Roland Scheidegger Date: Fri, 23 Nov 2018 02:31:24 +0100 Subject: [PATCH] draw: fix infinite loop in line stippling The calculated length of a line may be infinite, if the coords we get are bogus. This leads to an infinite loop in line stippling. To prevent this test for this explicitly (although technically on at least x86 sse it would actually work without the explicit test, as long as we use the int-converted length value). While here also get rid of some always-true condition. Note this does not actually solve the root cause, which is that the coords we receive are bogus after clipping. This seems a difficult problem to solve. One issue is that due to float arithmetic, clip w may become 0 after clipping if the incoming geometry is "sufficiently degenerate", hence x/y/z ndc (and window) coords will be all inf (or nan). Even with w not quite 0, I believe it's possible we produce values which are actually outside the view volume. (Also, x=y=z=w=0 coords in clipspace would be not considered subject to clipping, and similarly result in all NaN coords.) We just hope for now other draw stages (and rasterizers) can handle those relatively safely (llvmpipe itself should be sort of robust against this, certainly converstion to fixed point will produce garbage, it might fail a couple assertions but should neither hang nor crash otherwise). Reviewed-by: Jose Fonseca --- .../auxiliary/draw/draw_pipe_stipple.c | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/gallium/auxiliary/draw/draw_pipe_stipple.c b/src/gallium/auxiliary/draw/draw_pipe_stipple.c index d30572cc61c..386b7649e4f 100644 --- a/src/gallium/auxiliary/draw/draw_pipe_stipple.c +++ b/src/gallium/auxiliary/draw/draw_pipe_stipple.c @@ -48,8 +48,8 @@ struct stipple_stage { struct draw_stage stage; float counter; - uint pattern; - uint factor; + ushort pattern; + ushort factor; bool smooth; }; @@ -110,7 +110,7 @@ emit_segment(struct draw_stage *stage, struct prim_header *header, static inline bool -stipple_test(int counter, ushort pattern, int factor) +stipple_test(int counter, ushort pattern, ushort factor) { int b = (counter / factor) & 0xf; return !!((1 << b) & pattern); @@ -136,6 +136,10 @@ stipple_line(struct draw_stage *stage, struct prim_header *header) float length; int i; + int intlength; + + if (header->flags & DRAW_PIPE_RESET_STIPPLE) + stipple->counter = 0; if (stipple->smooth) { float dx = x1 - x0; @@ -147,21 +151,21 @@ stipple_line(struct draw_stage *stage, struct prim_header *header) length = MAX2(dx, dy); } - if (header->flags & DRAW_PIPE_RESET_STIPPLE) - stipple->counter = 0; + if (util_is_inf_or_nan(length)) + intlength = 0; + else + intlength = ceilf(length); /* XXX ToDo: instead of iterating pixel-by-pixel, use a look-up table. */ - for (i = 0; i < length; i++) { + for (i = 0; i < intlength; i++) { bool result = stipple_test((int)stipple->counter + i, - (ushort)stipple->pattern, stipple->factor); + stipple->pattern, stipple->factor); if (result != state) { /* changing from "off" to "on" or vice versa */ if (state) { - if (start != i) { - /* finishing an "on" segment */ - emit_segment(stage, header, start / length, i / length); - } + /* finishing an "on" segment */ + emit_segment(stage, header, start / length, i / length); } else { /* starting an "on" segment */ -- 2.30.2