draw: fix infinite loop in line stippling
authorRoland Scheidegger <sroland@vmware.com>
Fri, 23 Nov 2018 01:31:24 +0000 (02:31 +0100)
committerRoland Scheidegger <sroland@vmware.com>
Thu, 29 Nov 2018 17:39:40 +0000 (18:39 +0100)
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 <jfonseca@vmware.com>
src/gallium/auxiliary/draw/draw_pipe_stipple.c

index d30572cc61c48f17b2adcd7dcbb667000daff842..386b7649e4fae50eb82f846921223689e69662a6 100644 (file)
@@ -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 */