[cairo-commit] 4 commits - src/cairo-bentley-ottmann.c src/cairo-tee-surface.c test/bug-bo-ricotz.c test/cairo-test-trace.c test/Makefile.refs test/Makefile.sources test/reference util/cairo-fdr

Chris Wilson ickle at kemper.freedesktop.org
Fri Sep 16 05:50:26 PDT 2011


 src/cairo-bentley-ottmann.c                |   12 +++-
 src/cairo-tee-surface.c                    |   84 +++++++++++------------------
 test/Makefile.refs                         |    3 +
 test/Makefile.sources                      |    1 
 test/bug-bo-ricotz.c                       |   74 +++++++++++++++++++++++++
 test/cairo-test-trace.c                    |   39 ++++++++++---
 test/reference/bug-bo-ricotz.base.ref.png  |binary
 test/reference/bug-bo-ricotz.ref.png       |binary
 test/reference/bug-bo-ricotz.traps.ref.png |binary
 util/cairo-fdr/fdr.c                       |   26 ++++++++
 10 files changed, 174 insertions(+), 65 deletions(-)

New commits:
commit a0ec977944b7659a3fee7a97ce2e650d775bdf8e
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Fri Sep 16 12:35:45 2011 +0100

    bentley-ottman: End subsumed colinear traps
    
    I'm not quite sure how we end up with a pair of colinear edges both with
    a deferred trap...
    
    Fixes crash in bug-bo-ricotz
    
    Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>

diff --git a/src/cairo-bentley-ottmann.c b/src/cairo-bentley-ottmann.c
index 634da6f..f30449e 100644
--- a/src/cairo-bentley-ottmann.c
+++ b/src/cairo-bentley-ottmann.c
@@ -1434,12 +1434,18 @@ _active_edges_to_traps (cairo_bo_edge_t	*pos,
     left = pos;
     while (pos != NULL) {
 	if (pos != left && pos->deferred_trap.right) {
-	    if (edges_colinear (left, pos)) {
+	    /* XXX It shouldn't be possible to here with 2 deferred traps
+	     * on colinear edges... See bug-bo-rictoz.
+	     */
+	    if (left->deferred_trap.right == NULL &&
+		edges_colinear (left, pos))
+	    {
 		/* continuation on left */
-		assert (left->deferred_trap.right == NULL);
 		left->deferred_trap = pos->deferred_trap;
 		pos->deferred_trap.right = NULL;
-	    } else {
+	    }
+	    else
+	    {
 		status = _cairo_bo_edge_end_trap (pos, top, traps);
 		if (unlikely (status))
 		    return status;
commit 0aabde231431d94eec50304b3f3b428965f83a39
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Fri Sep 16 13:46:09 2011 +0100

    test: Add bug-bo-ricotz
    
    Exercises an assertion failure found by Rico Tzschichholz.
    
    Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>

diff --git a/test/Makefile.refs b/test/Makefile.refs
index dc51278..80ddeef 100644
--- a/test/Makefile.refs
+++ b/test/Makefile.refs
@@ -201,6 +201,9 @@ REFERENCE_IMAGES = \
 	reference/bug-bo-rectangular.ref.png \
 	reference/bug-bo-rectangular.traps.argb32.ref.png \
 	reference/bug-bo-rectangular.traps.rgb24.ref.png \
+	reference/bug-bo-ricotz.base.ref.png \
+	reference/bug-bo-ricotz.ref.png \
+	reference/bug-bo-ricotz.traps.ref.png \
 	reference/bug-extents.base.argb32.ref.png \
 	reference/bug-extents.base.rgb24.ref.png \
 	reference/bug-extents.image16.ref.png \
diff --git a/test/Makefile.sources b/test/Makefile.sources
index 0f81c9e..9caa83a 100644
--- a/test/Makefile.sources
+++ b/test/Makefile.sources
@@ -23,6 +23,7 @@ test_sources = \
 	bilevel-image.c					\
 	bug-40410.c					\
 	bug-bo-rectangular.c				\
+	bug-bo-ricotz.c					\
 	bug-extents.c					\
 	bug-seams.c					\
 	caps.c						\
diff --git a/test/bug-bo-ricotz.c b/test/bug-bo-ricotz.c
new file mode 100644
index 0000000..9524d20
--- /dev/null
+++ b/test/bug-bo-ricotz.c
@@ -0,0 +1,74 @@
+/*
+ * Copyright © 2011 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use, copy,
+ * modify, merge, publish, distribute, sublicense, and/or sell copies
+ * of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ *
+ * Author: Chris Wilson <chris at chris-wilson.co.uk>
+ */
+
+#include "cairo-test.h"
+
+/* An assertion failure found by Rico Tzschichholz */
+
+static cairo_test_status_t
+draw (cairo_t *cr, int width, int height)
+{
+    cairo_set_source_rgb (cr, 1, 1, 1);
+    cairo_paint (cr);
+    cairo_set_source_rgb (cr, 0, 0, 0);
+
+    cairo_rectangle (cr, 10, 55,165, 1);
+    cairo_rectangle (cr, 174, 55,1, 413);
+    cairo_rectangle (cr, 10, 56, 1, 413);
+    cairo_rectangle (cr, 10, 469, 165, 1);
+    cairo_clip (cr);
+
+    cairo_set_fill_rule (cr, CAIRO_FILL_RULE_EVEN_ODD);
+
+    cairo_move_to (cr, 10, 57);
+    cairo_curve_to (cr, 10, 55.894531, 10.894531, 55, 12, 55);
+    cairo_line_to (cr, 173, 55);
+    cairo_curve_to (cr, 174.105469, 55, 175, 55.894531, 175, 57);
+    cairo_line_to (cr, 175, 468);
+    cairo_curve_to (cr, 175, 469.105469, 174.105469, 470, 173, 470);
+    cairo_line_to (cr, 12, 470);
+    cairo_curve_to (cr, 10.894531, 470, 10, 469.105469, 10, 468);
+
+    cairo_move_to (cr, 11, 57);
+    cairo_curve_to (cr, 11, 56.449219, 11.449219, 56, 12, 56);
+    cairo_line_to (cr, 173, 56);
+    cairo_curve_to (cr, 173.550781, 56, 174, 56.449219, 174, 57);
+    cairo_line_to (cr, 174, 468);
+    cairo_curve_to (cr, 174, 468.550781, 173.550781, 469, 173, 469);
+    cairo_line_to (cr, 12, 469);
+    cairo_curve_to (cr, 11.449219, 469, 11, 468.550781, 11, 468);
+
+    cairo_fill (cr);
+
+    return CAIRO_TEST_SUCCESS;
+}
+
+CAIRO_TEST (bug_bo_ricotz,
+	    "Exercises a bug discovered by Rico Tzschichholz",
+	    "clip, fill", /* keywords */
+	    NULL, /* requirements */
+	    649, 480,
+	    NULL, draw)
diff --git a/test/reference/bug-bo-ricotz.base.ref.png b/test/reference/bug-bo-ricotz.base.ref.png
new file mode 100644
index 0000000..ff7a552
Binary files /dev/null and b/test/reference/bug-bo-ricotz.base.ref.png differ
diff --git a/test/reference/bug-bo-ricotz.ref.png b/test/reference/bug-bo-ricotz.ref.png
new file mode 100644
index 0000000..51c7ccb
Binary files /dev/null and b/test/reference/bug-bo-ricotz.ref.png differ
diff --git a/test/reference/bug-bo-ricotz.traps.ref.png b/test/reference/bug-bo-ricotz.traps.ref.png
new file mode 100644
index 0000000..ff7a552
Binary files /dev/null and b/test/reference/bug-bo-ricotz.traps.ref.png differ
commit e7bcf1fd79ba96ef46ec297f82facee9c8d73e20
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Fri Sep 16 13:45:11 2011 +0100

    test: Hack cairo-test-trace to write at trace for all contexts
    
    Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>

diff --git a/test/cairo-test-trace.c b/test/cairo-test-trace.c
index 296a229..8773bd1 100644
--- a/test/cairo-test-trace.c
+++ b/test/cairo-test-trace.c
@@ -903,7 +903,7 @@ write_result (const char *trace, struct slave *slave)
 }
 
 static void
-write_trace (const char *trace, struct slave *slave)
+write_trace (const char *trace, const char *id, struct slave *slave)
 {
 #if CAIRO_HAS_SCRIPT_SURFACE
     cairo_device_t *script;
@@ -911,7 +911,7 @@ write_trace (const char *trace, struct slave *slave)
 
     assert (slave->is_recording);
 
-    xasprintf (&filename, "%s-fail.trace", trace);
+    xasprintf (&filename, "%s-%s.trace", trace, id);
 
     script = cairo_script_create (filename);
     cairo_script_from_recording_surface (script, slave->image);
@@ -1012,7 +1012,7 @@ test_run (void *base,
 {
     struct pollfd *pfd;
     int npfd, cnt, n, i;
-    int completion;
+    int completion, err = 0;
     cairo_bool_t ret = FALSE;
     unsigned long image;
 
@@ -1063,8 +1063,10 @@ test_run (void *base,
 	    if (! pfd[n].revents)
 		continue;
 
-	    if (pfd[n].revents & POLLHUP)
-		goto done;
+	    if (pfd[n].revents & POLLHUP) {
+		completion++;
+		continue;
+	    }
 
 	    for (i = 0; i < num_slaves; i++) {
 		if (slaves[i].fd == pfd[n].fd) {
@@ -1091,8 +1093,11 @@ test_run (void *base,
 			    allocate_image_for_slave (base,
 						      offset = image,
 						      &slaves[i]);
-			if (! writen (pfd[n].fd, &offset, sizeof (offset)))
-			    goto done;
+			if (! writen (pfd[n].fd, &offset, sizeof (offset))) {
+			    err = 1;
+			    completion++;
+			    continue;
+			}
 		    } else {
 			readn (pfd[n].fd,
 			       &slaves[i].image_ready,
@@ -1103,8 +1108,11 @@ test_run (void *base,
 				    slaves[i].image_ready,
 				    slaves[i].image_serial);
 			}
-			if (slaves[i].image_ready != slaves[i].image_serial)
-			    goto out;
+			if (slaves[i].image_ready != slaves[i].image_serial) {
+			    err = 1;
+			    completion++;
+			    continue;
+			}
 
 			/* Can anyone spell 'P·E·D·A·N·T'? */
 			if (! slaves[i].is_recording)
@@ -1120,6 +1128,12 @@ test_run (void *base,
 	}
 
 	if (completion == num_slaves) {
+	    if (err) {
+		if (DEBUG > 1)
+		    printf ("error detected\n");
+		goto out;
+	    }
+
 	    if (DEBUG > 1) {
 		printf ("all saves report completion\n");
 	    }
@@ -1139,12 +1153,17 @@ test_run (void *base,
 		write_images (trace, slaves, num_slaves);
 
 		if (slaves[0].is_recording)
-		    write_trace (trace, &slaves[0]);
+		    write_trace (trace, "fail", &slaves[0]);
 
 		goto out;
 	    }
 
 	    if (0) write_result (trace, &slaves[1]);
+	    if (0 && slaves[0].is_recording) {
+		char buf[80];
+		snprintf (buf, sizeof (buf), "%d", slaves[0].image_serial);
+		write_trace (trace, buf, &slaves[0]);
+	    }
 
 	    /* ack */
 	    for (i = 0; i < num_slaves; i++) {
commit 35f41d253ff09a7c144ace833901b28e961c8e14
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Fri Sep 16 13:47:17 2011 +0100

    fdr,tee: Reorder master/slave invocation to capture death-upon-signals
    
    Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>

diff --git a/src/cairo-tee-surface.c b/src/cairo-tee-surface.c
index 25b4208..4930a0b 100644
--- a/src/cairo-tee-surface.c
+++ b/src/cairo-tee-surface.c
@@ -209,10 +209,6 @@ _cairo_tee_surface_paint (void			*abstract_surface,
     int n, num_slaves;
     cairo_int_status_t status;
 
-    status = _cairo_surface_wrapper_paint (&surface->master, op, source, clip);
-    if (unlikely (status))
-	return status;
-
     num_slaves = _cairo_array_num_elements (&surface->slaves);
     slaves = _cairo_array_index (&surface->slaves, 0);
     for (n = 0; n < num_slaves; n++) {
@@ -221,7 +217,7 @@ _cairo_tee_surface_paint (void			*abstract_surface,
 	    return status;
     }
 
-    return CAIRO_STATUS_SUCCESS;
+    return _cairo_surface_wrapper_paint (&surface->master, op, source, clip);
 }
 
 static cairo_int_status_t
@@ -236,11 +232,6 @@ _cairo_tee_surface_mask (void			*abstract_surface,
     cairo_int_status_t status;
     int n, num_slaves;
 
-    status = _cairo_surface_wrapper_mask (&surface->master,
-					  op, source, mask, clip);
-    if (unlikely (status))
-	return status;
-
     num_slaves = _cairo_array_num_elements (&surface->slaves);
     slaves = _cairo_array_index (&surface->slaves, 0);
     for (n = 0; n < num_slaves; n++) {
@@ -250,7 +241,8 @@ _cairo_tee_surface_mask (void			*abstract_surface,
 	    return status;
     }
 
-    return CAIRO_STATUS_SUCCESS;
+    return _cairo_surface_wrapper_mask (&surface->master,
+					op, source, mask, clip);
 }
 
 static cairo_int_status_t
@@ -270,15 +262,6 @@ _cairo_tee_surface_stroke (void				*abstract_surface,
     cairo_int_status_t status;
     int n, num_slaves;
 
-    status = _cairo_surface_wrapper_stroke (&surface->master,
-					    op, source,
-					    path, style,
-					    ctm, ctm_inverse,
-					    tolerance, antialias,
-					    clip);
-    if (unlikely (status))
-	return status;
-
     num_slaves = _cairo_array_num_elements (&surface->slaves);
     slaves = _cairo_array_index (&surface->slaves, 0);
     for (n = 0; n < num_slaves; n++) {
@@ -292,7 +275,12 @@ _cairo_tee_surface_stroke (void				*abstract_surface,
 	    return status;
     }
 
-    return CAIRO_STATUS_SUCCESS;
+    return _cairo_surface_wrapper_stroke (&surface->master,
+					  op, source,
+					  path, style,
+					  ctm, ctm_inverse,
+					  tolerance, antialias,
+					  clip);
 }
 
 static cairo_int_status_t
@@ -310,14 +298,6 @@ _cairo_tee_surface_fill (void				*abstract_surface,
     cairo_int_status_t status;
     int n, num_slaves;
 
-    status = _cairo_surface_wrapper_fill (&surface->master,
-					  op, source,
-					  path, fill_rule,
-					  tolerance, antialias,
-					  clip);
-    if (unlikely (status))
-	return status;
-
     num_slaves = _cairo_array_num_elements (&surface->slaves);
     slaves = _cairo_array_index (&surface->slaves, 0);
     for (n = 0; n < num_slaves; n++) {
@@ -330,7 +310,11 @@ _cairo_tee_surface_fill (void				*abstract_surface,
 	    return status;
     }
 
-    return CAIRO_INT_STATUS_SUCCESS;
+    return _cairo_surface_wrapper_fill (&surface->master,
+					op, source,
+					path, fill_rule,
+					tolerance, antialias,
+					clip);
 }
 
 static cairo_bool_t
@@ -364,18 +348,6 @@ _cairo_tee_surface_show_text_glyphs (void		    *abstract_surface,
     if (unlikely (glyphs_copy == NULL))
 	return _cairo_error (CAIRO_STATUS_NO_MEMORY);
 
-    memcpy (glyphs_copy, glyphs, sizeof (cairo_glyph_t) * num_glyphs);
-    status = _cairo_surface_wrapper_show_text_glyphs (&surface->master, op,
-						      source,
-						      utf8, utf8_len,
-						      glyphs_copy, num_glyphs,
-						      clusters, num_clusters,
-						      cluster_flags,
-						      scaled_font,
-						      clip);
-    if (unlikely (status))
-	goto CLEANUP;
-
     num_slaves = _cairo_array_num_elements (&surface->slaves);
     slaves = _cairo_array_index (&surface->slaves, 0);
     for (n = 0; n < num_slaves; n++) {
@@ -392,7 +364,16 @@ _cairo_tee_surface_show_text_glyphs (void		    *abstract_surface,
 	    goto CLEANUP;
     }
 
-  CLEANUP:
+    memcpy (glyphs_copy, glyphs, sizeof (cairo_glyph_t) * num_glyphs);
+    status = _cairo_surface_wrapper_show_text_glyphs (&surface->master, op,
+						      source,
+						      utf8, utf8_len,
+						      glyphs_copy, num_glyphs,
+						      clusters, num_clusters,
+						      cluster_flags,
+						      scaled_font,
+						      clip);
+CLEANUP:
     free (glyphs_copy);
     return status;
 }
@@ -499,27 +480,26 @@ cairo_tee_surface_remove (cairo_surface_t *abstract_surface,
 {
     cairo_tee_surface_t *surface;
     cairo_surface_wrapper_t *slaves;
-    cairo_int_status_t status;
     int n, num_slaves;
 
     if (unlikely (abstract_surface->status))
 	return;
     if (unlikely (abstract_surface->finished)) {
-	status = _cairo_surface_set_error (abstract_surface,
-					   _cairo_error (CAIRO_STATUS_SURFACE_FINISHED));
+	_cairo_surface_set_error (abstract_surface,
+				  _cairo_error (CAIRO_STATUS_SURFACE_FINISHED));
 	return;
     }
 
     if (abstract_surface->backend != &cairo_tee_surface_backend) {
-	status = _cairo_surface_set_error (abstract_surface,
-					   _cairo_error (CAIRO_STATUS_SURFACE_TYPE_MISMATCH));
+	_cairo_surface_set_error (abstract_surface,
+				  _cairo_error (CAIRO_STATUS_SURFACE_TYPE_MISMATCH));
 	return;
     }
 
     surface = (cairo_tee_surface_t *) abstract_surface;
     if (target == surface->master.target) {
-	status = _cairo_surface_set_error (abstract_surface,
-					   _cairo_error (CAIRO_STATUS_INVALID_INDEX));
+	_cairo_surface_set_error (abstract_surface,
+				  _cairo_error (CAIRO_STATUS_INVALID_INDEX));
 	return;
     }
 
@@ -531,8 +511,8 @@ cairo_tee_surface_remove (cairo_surface_t *abstract_surface,
     }
 
     if (n == num_slaves) {
-	status = _cairo_surface_set_error (abstract_surface,
-					   _cairo_error (CAIRO_STATUS_INVALID_INDEX));
+	_cairo_surface_set_error (abstract_surface,
+				  _cairo_error (CAIRO_STATUS_INVALID_INDEX));
 	return;
     }
 
diff --git a/util/cairo-fdr/fdr.c b/util/cairo-fdr/fdr.c
index 209c380..08d9c01 100644
--- a/util/cairo-fdr/fdr.c
+++ b/util/cairo-fdr/fdr.c
@@ -85,6 +85,12 @@ fdr_sighandler (int sig)
 }
 
 static void
+fdr_urgent_sighandler (int sig)
+{
+    fdr_dump_ringbuffer ();
+}
+
+static void
 fdr_atexit (void)
 {
     if (fdr_dump)
@@ -100,6 +106,9 @@ fdr_pending_signals (void)
 	initialized = 1;
 
 	signal (SIGUSR1, fdr_sighandler);
+
+	signal (SIGSEGV, fdr_urgent_sighandler);
+	signal (SIGABRT, fdr_urgent_sighandler);
 	atexit (fdr_atexit);
     }
 
@@ -303,3 +312,20 @@ cairo_surface_create_similar (cairo_surface_t *surface,
     return DLCALL (cairo_surface_create_similar,
 		   surface, content, width, height);
 }
+
+cairo_surface_t *
+cairo_surface_create_for_rectangle (cairo_surface_t *surface,
+                                    double		 x,
+                                    double		 y,
+                                    double		 width,
+                                    double		 height)
+{
+    cairo_surface_t *tee;
+
+    tee = fdr_surface_get_tee (surface);
+    if (tee != NULL)
+	surface = tee;
+
+    return DLCALL (cairo_surface_create_for_rectangle,
+		   surface, x, y, width, height);
+}


More information about the cairo-commit mailing list