[cairo-commit] 14 commits - boilerplate/cairo-boilerplate-pdf.c src/cairoint.h src/cairo-meta-surface.c src/cairo-path-stroke.c src/cairo-pen.c src/cairo-slope.c src/cairo-svg-surface.c test/clip-operator-pdf-argb32-ref.png test/degenerate-pen.c test/degenerate-pen-ps-argb32-ref.png test/degenerate-pen-ref.png test/gradient-alpha.c test/gradient-alpha-pdf-argb32-ref.png test/gradient-alpha-pdf-rgb24-ref.png test/linear-gradient.c test/linear-gradient-pdf-ref.png test/linear-gradient-reflect-pdf-argb32-ref.png test/linear-gradient-reflect-pdf-rgb24-ref.png test/meta-surface-pattern-svg-argb32-ref.png test/meta-surface-pattern-svg-rgb24-ref.png test/operator-clear-pdf-argb32-ref.png test/operator-clear-pdf-rgb24-ref.png test/paint-source-alpha-pdf-ref.png test/paint-with-alpha-pdf-ref.png test/push-group-pdf-ref.png test/push-group-pdf-rgb24-ref.png test/README test/rotate-image-surface-paint-pdf-rgb24-ref.png test/scale-source-surface-paint-pdf-rgb24-ref.png test/source-clip-scale-pdf-ref.png test/text-pattern.c test/text-pattern-pdf-argb32-ref.png test/text-pattern-pdf-rgb24-ref.png test/trap-clip.c test/trap-clip-pdf-argb32-ref.png test/trap-clip-pdf-rgb24-ref.png

Carl Worth cworth at kemper.freedesktop.org
Wed Feb 20 04:04:19 PST 2008


 boilerplate/cairo-boilerplate-pdf.c               |   55 -------------------
 src/cairo-meta-surface.c                          |   11 +--
 src/cairo-path-stroke.c                           |    2 
 src/cairo-pen.c                                   |   41 +++++++++-----
 src/cairo-slope.c                                 |   53 +++++++++---------
 src/cairo-svg-surface.c                           |   38 ++++++++-----
 src/cairoint.h                                    |    6 --
 test/README                                       |   63 ++++++++++++++++++++++
 test/clip-operator-pdf-argb32-ref.png             |binary
 test/degenerate-pen-ps-argb32-ref.png             |binary
 test/degenerate-pen-ref.png                       |binary
 test/degenerate-pen.c                             |    3 +
 test/gradient-alpha-pdf-argb32-ref.png            |binary
 test/gradient-alpha-pdf-rgb24-ref.png             |binary
 test/gradient-alpha.c                             |    3 -
 test/linear-gradient-pdf-ref.png                  |binary
 test/linear-gradient-reflect-pdf-argb32-ref.png   |binary
 test/linear-gradient-reflect-pdf-rgb24-ref.png    |binary
 test/linear-gradient.c                            |    3 -
 test/meta-surface-pattern-svg-argb32-ref.png      |binary
 test/meta-surface-pattern-svg-rgb24-ref.png       |binary
 test/operator-clear-pdf-argb32-ref.png            |binary
 test/operator-clear-pdf-rgb24-ref.png             |binary
 test/paint-source-alpha-pdf-ref.png               |binary
 test/paint-with-alpha-pdf-ref.png                 |binary
 test/push-group-pdf-ref.png                       |binary
 test/push-group-pdf-rgb24-ref.png                 |binary
 test/rotate-image-surface-paint-pdf-rgb24-ref.png |binary
 test/scale-source-surface-paint-pdf-rgb24-ref.png |binary
 test/source-clip-scale-pdf-ref.png                |binary
 test/text-pattern-pdf-argb32-ref.png              |binary
 test/text-pattern-pdf-rgb24-ref.png               |binary
 test/text-pattern.c                               |    3 -
 test/trap-clip-pdf-argb32-ref.png                 |binary
 test/trap-clip-pdf-rgb24-ref.png                  |binary
 test/trap-clip.c                                  |    3 -
 36 files changed, 155 insertions(+), 129 deletions(-)

New commits:
commit 78b270601ce54545a0c68075a60a3b85a2f37adb
Author: Carl Worth <cworth at cworth.org>
Date:   Wed Feb 20 03:49:16 2008 -0800

    Add poppler-specific reference image for source-clip-scale
    
    This one doesn't have any associated poppler bug report. The
    rendering by poppler is totally reasonable, and not problematic
    at all with respect to what the test is actually testing for.

diff --git a/test/source-clip-scale-pdf-ref.png b/test/source-clip-scale-pdf-ref.png
new file mode 100644
index 0000000..78c7710
Binary files /dev/null and b/test/source-clip-scale-pdf-ref.png differ
commit 64b7489a50808bd660a47615d0775842c77dd0f2
Author: Carl Worth <cworth at cworth.org>
Date:   Wed Feb 20 03:47:34 2008 -0800

    Add poppler-specific reference image for push-group
    
    This is justified by the following new bug report:
    
    	Incorrect clipping of group object (regression?)
    	https://bugs.freedesktop.org/show_bug.cgi?id=14580

diff --git a/test/README b/test/README
index 2bc06c7..988177d 100644
--- a/test/README
+++ b/test/README
@@ -152,3 +152,8 @@ paint-source-alpha
 paint-with-alpha
 rotate-image-surface-paint
 scale-source-surface-paint
+
+Incorrect clipping of group object (regression?)
+https://bugs.freedesktop.org/show_bug.cgi?id=14580
+--------------------------------------------------
+push-group
diff --git a/test/push-group-pdf-ref.png b/test/push-group-pdf-ref.png
new file mode 100644
index 0000000..a7aace0
Binary files /dev/null and b/test/push-group-pdf-ref.png differ
diff --git a/test/push-group-pdf-rgb24-ref.png b/test/push-group-pdf-rgb24-ref.png
new file mode 100644
index 0000000..d45cf6e
Binary files /dev/null and b/test/push-group-pdf-rgb24-ref.png differ
commit 7aaa3e67c5016357e28476b10d768101e1e5bd41
Author: Carl Worth <cworth at cworth.org>
Date:   Wed Feb 20 03:38:07 2008 -0800

    Add poppler-specific reference images for tests needing EXTEND_PAD
    
    This is justified by the following new bug report:
    
    	Poppler should paint images with CAIRO_EXTEND_PAD
    	https://bugs.freedesktop.org/show_bug.cgi?id=14578
    
    The four affected tests are:
    
    	paint-source-alpha
    	paint-with-alpha
    	rotate-image-surface-paint
    	scale-source-surface-paint

diff --git a/test/README b/test/README
index 22322f2..2bc06c7 100644
--- a/test/README
+++ b/test/README
@@ -145,5 +145,10 @@ https://bugs.freedesktop.org/show_bug.cgi?id=14577
 --------------------------------------------------
 text-pattern
 
-
-
+Poppler should paint images with CAIRO_EXTEND_PAD
+https://bugs.freedesktop.org/show_bug.cgi?id=14578
+--------------------------------------------------
+paint-source-alpha
+paint-with-alpha
+rotate-image-surface-paint
+scale-source-surface-paint
diff --git a/test/paint-source-alpha-pdf-ref.png b/test/paint-source-alpha-pdf-ref.png
new file mode 100644
index 0000000..befab10
Binary files /dev/null and b/test/paint-source-alpha-pdf-ref.png differ
diff --git a/test/paint-with-alpha-pdf-ref.png b/test/paint-with-alpha-pdf-ref.png
new file mode 100644
index 0000000..9da63ad
Binary files /dev/null and b/test/paint-with-alpha-pdf-ref.png differ
diff --git a/test/rotate-image-surface-paint-pdf-rgb24-ref.png b/test/rotate-image-surface-paint-pdf-rgb24-ref.png
new file mode 100644
index 0000000..5cd7bf6
Binary files /dev/null and b/test/rotate-image-surface-paint-pdf-rgb24-ref.png differ
diff --git a/test/scale-source-surface-paint-pdf-rgb24-ref.png b/test/scale-source-surface-paint-pdf-rgb24-ref.png
new file mode 100644
index 0000000..3e873ab
Binary files /dev/null and b/test/scale-source-surface-paint-pdf-rgb24-ref.png differ
commit fdd046edccf283bfedb59733516f6110cc96ea82
Author: Carl Worth <cworth at cworth.org>
Date:   Wed Feb 20 03:23:38 2008 -0800

    Add poppler-specific reference image for text-pattern
    
    This is justified with the following new bug report:
    
    	Poppler renders patterned text as black
    	https://bugs.freedesktop.org/show_bug.cgi?id=14577

diff --git a/test/README b/test/README
index a603e98..22322f2 100644
--- a/test/README
+++ b/test/README
@@ -140,4 +140,10 @@ trap-clip
 linear-gradient
 linear-gradient-reflect
 
+Poppler renders patterned text as black
+https://bugs.freedesktop.org/show_bug.cgi?id=14577
+--------------------------------------------------
+text-pattern
+
+
 
diff --git a/test/text-pattern-pdf-argb32-ref.png b/test/text-pattern-pdf-argb32-ref.png
new file mode 100644
index 0000000..2a70deb
Binary files /dev/null and b/test/text-pattern-pdf-argb32-ref.png differ
diff --git a/test/text-pattern-pdf-rgb24-ref.png b/test/text-pattern-pdf-rgb24-ref.png
new file mode 100644
index 0000000..2933f34
Binary files /dev/null and b/test/text-pattern-pdf-rgb24-ref.png differ
commit 43302f0408d92138ac12afcd855d44c430074859
Author: Carl Worth <cworth at cworth.org>
Date:   Wed Feb 20 03:15:17 2008 -0800

    Re-enable pdf tests with gradients and add poppler-based reference images
    
    This freezes the current (buggy) poppler result in the test suite
    so that we will notice any future regressions.

diff --git a/boilerplate/cairo-boilerplate-pdf.c b/boilerplate/cairo-boilerplate-pdf.c
index c89f298..cc26d91 100644
--- a/boilerplate/cairo-boilerplate-pdf.c
+++ b/boilerplate/cairo-boilerplate-pdf.c
@@ -44,41 +44,6 @@ typedef struct _pdf_target_closure
 
 #define ARRAY_LENGTH(__array) ((int) (sizeof (__array) / sizeof (__array[0])))
 
-/* We currently ignore tests that generate valid PDF output, but which
- * poppler currently misrenders. This lets us avoid false negatives in
- * the test suite, (at the significant cost that we won't notice any
- * regressions in the PDF output of these tests).
- *
- * So obviously, we have an interest in ensuring that the poppler bugs
- * get fixed sooner rather than later, so that we can re-enable these
- * tests. As such, we're trying to be good citizens by reporting all
- * such poppler bugs that we identify to the poppler bugzilla.
- *
- * Here's a tracking bug explaining the situation:
- *
- *	Poppler does not yet handle everything in the cairo test suite
- *	https://bugs.freedesktop.org/show_bug.cgi?id=12143
- *
- * Here's the rule: To add any test to this list (based on a poppler
- * bug), you must first create a new bug report against poppler,
- * marked as blocking 12143, and list that bug here in a comment for
- * the ignored test(s).
- *
- * And when this list shrinks to nothing, we can close bug 12143.
- */
-static const char *pdf_ignored_tests[] = {
-    /* These first four failures are due to:
-     *
-     *	Poppler doesn't correctly handle gradients with transparency
-     *	https://bugs.freedesktop.org/show_bug.cgi?id=12144
-     */
-    "gradient-alpha",
-    "linear-gradient",
-    "radial-gradient",
-    "text-pattern",
-    "trap-clip",
-};
-
 cairo_surface_t *
 _cairo_boilerplate_pdf_create_surface (const char		 *name,
 				       cairo_content_t		  content,
@@ -90,13 +55,6 @@ _cairo_boilerplate_pdf_create_surface (const char		 *name,
     pdf_target_closure_t *ptc;
     cairo_surface_t *surface;
 
-    if (mode == CAIRO_BOILERPLATE_MODE_TEST) {
-	int i;
-	for (i = 0; i < ARRAY_LENGTH (pdf_ignored_tests); i++)
-	    if (strcmp (name, pdf_ignored_tests[i]) == 0)
-		return NULL;
-    }
-
     /* Sanitize back to a real cairo_content_t value. */
     if (content == CAIRO_TEST_CONTENT_COLOR_ALPHA_FLATTENED)
 	content = CAIRO_CONTENT_COLOR_ALPHA;
diff --git a/test/README b/test/README
index 840e1ad..a603e98 100644
--- a/test/README
+++ b/test/README
@@ -94,3 +94,50 @@ message:
 
 	All XX tests passed
 
+Some notes on limitations in poppler
+------------------------------------
+One of the difficulties of our current test infrastructure is that we
+rely on external tools to convert cairo's vector output (PDF,
+PostScript, and SVG), into an image that can be used for the image
+comparison. This means that any bugs in that conversion tool will
+result in false negatives in the test suite.
+
+We've identified several such bugs in the poppler library which is
+used to convert PDF to an image. This is particularly discouraging
+because 1) poppler is free software that will be used by *many* cairo
+users, and 2) poppler calls into cairo for its rendering so it should
+be able to do a 100% faithful conversion.
+
+So we have an interest in ensuring that these poppler bugs get fixed
+sooner rather than later. As such, we're trying to be good citizens by
+reporting all such poppler bugs that we identify to the poppler
+bugzilla. Here's a tracking bug explaining the situation:
+
+	Poppler does not yet handle everything in the cairo test suite
+	https://bugs.freedesktop.org/show_bug.cgi?id=12143
+
+Here's the rule: If a cairo-pdf test reports a failure, but viewing
+the resulting PDF file with acroread suggests that the PDF itself is
+correct, then there's likely a bug in poppler. In this case, we can
+simply report the poppler bug, (making it block 12143 above), post the
+PDF result from the test suite, and list the bug in this file. Once
+we've done this, we can capture poppler's buggy output as a
+pdf-specific reference image so that the test suite will regard the
+test as passing, (and we'll ensure there is no regression).
+
+Once the poppler bug gets fixed, the test suite will start reporting a
+false negative again, and this will be easy to fix by simply removing
+the pdf-specific reference image.
+
+Here are the reported poppler bugs and the tests they affect:
+
+Poppler doesn't correctly handle gradients with transparency
+https://bugs.freedesktop.org/show_bug.cgi?id=12144
+--------------------------------------------------
+gradient-alpha
+linear-gradient
+trap-clip
+linear-gradient
+linear-gradient-reflect
+
+
diff --git a/test/gradient-alpha-pdf-argb32-ref.png b/test/gradient-alpha-pdf-argb32-ref.png
new file mode 100644
index 0000000..bf9c69a
Binary files /dev/null and b/test/gradient-alpha-pdf-argb32-ref.png differ
diff --git a/test/gradient-alpha-pdf-rgb24-ref.png b/test/gradient-alpha-pdf-rgb24-ref.png
new file mode 100644
index 0000000..7985273
Binary files /dev/null and b/test/gradient-alpha-pdf-rgb24-ref.png differ
diff --git a/test/gradient-alpha.c b/test/gradient-alpha.c
index 6646aec..6b76247 100644
--- a/test/gradient-alpha.c
+++ b/test/gradient-alpha.c
@@ -29,8 +29,7 @@ static cairo_test_draw_function_t draw;
 
 cairo_test_t test = {
     "gradient-alpha",
-    "Tests drawing of a gradient with various alpha values in the color stops\n"
-    "PDF backend will not be tested due to limitations in poppler.",
+    "Tests drawing of a gradient with various alpha values in the color stops\n",
     10, 10,
     draw
 };
diff --git a/test/linear-gradient-pdf-ref.png b/test/linear-gradient-pdf-ref.png
new file mode 100644
index 0000000..ede7c90
Binary files /dev/null and b/test/linear-gradient-pdf-ref.png differ
diff --git a/test/linear-gradient-reflect-pdf-argb32-ref.png b/test/linear-gradient-reflect-pdf-argb32-ref.png
new file mode 100644
index 0000000..46e1c0f
Binary files /dev/null and b/test/linear-gradient-reflect-pdf-argb32-ref.png differ
diff --git a/test/linear-gradient-reflect-pdf-rgb24-ref.png b/test/linear-gradient-reflect-pdf-rgb24-ref.png
new file mode 100644
index 0000000..46e1c0f
Binary files /dev/null and b/test/linear-gradient-reflect-pdf-rgb24-ref.png differ
diff --git a/test/linear-gradient.c b/test/linear-gradient.c
index 71a57f6..3dad477 100644
--- a/test/linear-gradient.c
+++ b/test/linear-gradient.c
@@ -58,8 +58,7 @@ static cairo_test_draw_function_t draw;
 
 cairo_test_t test = {
     "linear-gradient",
-    "Tests the drawing of linear gradients\n"
-    "PDF backend will not be tested due to limitations in poppler.",
+    "Tests the drawing of linear gradients\n",
     WIDTH, HEIGHT,
     draw
 };
diff --git a/test/text-pattern.c b/test/text-pattern.c
index 99ac9fd..15a909d 100644
--- a/test/text-pattern.c
+++ b/test/text-pattern.c
@@ -32,8 +32,7 @@ static cairo_test_draw_function_t draw;
 
 cairo_test_t test = {
     "text-pattern",
-    "Patterned Text\n"
-    "PDF backend will not be tested due to limitations in poppler.",
+    "Patterned Text\n",
     IMAGE_WIDTH, IMAGE_HEIGHT,
     draw
 };
diff --git a/test/trap-clip-pdf-argb32-ref.png b/test/trap-clip-pdf-argb32-ref.png
new file mode 100644
index 0000000..c3ee997
Binary files /dev/null and b/test/trap-clip-pdf-argb32-ref.png differ
diff --git a/test/trap-clip-pdf-rgb24-ref.png b/test/trap-clip-pdf-rgb24-ref.png
new file mode 100644
index 0000000..f787011
Binary files /dev/null and b/test/trap-clip-pdf-rgb24-ref.png differ
diff --git a/test/trap-clip.c b/test/trap-clip.c
index a33985f..3712c90 100644
--- a/test/trap-clip.c
+++ b/test/trap-clip.c
@@ -168,8 +168,7 @@ static cairo_test_draw_function_t draw;
 
 cairo_test_t test = {
     "trap-clip",
-    "Trapezoid clipping\n"
-    "PDF backend will not be tested due to limitations in poppler.",
+    "Trapezoid clipping\n",
     IMAGE_WIDTH, IMAGE_HEIGHT,
     draw
 };
commit 635fed77676f2446cbedfcf52a85f96e32c3a241
Author: Carl Worth <cworth at cworth.org>
Date:   Wed Feb 20 02:24:28 2008 -0800

    Re-enable some pdf tests now that poppler has knockout support
    
    We had several pdf tests disabled waiting for this bug fix:
    
    	Poppler does not correctly handle knockout groups
    	https://bugs.freedesktop.org/show_bug.cgi?id=12185
    
    That's in place for poppler now, so we're turning the tests
    back on. Some of the affected tests now pass perfectly:
    
    	over-above-source
    	over-around-source
    	over-below-source
    	over-between-source
    
    Some just needed new reference images:
    
    	operator-clear
    	clip-operator-pdf-argb32
    
    The remaining tests still fail, but none of the failures can
    obviously be ascribed to just poppler problems:
    
    	clip-operator-pdf-rgb24
    	operator-source
    	unbounded-operator
    
    The first two have some serious problems, while in the case
    of unbounded-operator the problem is extremely minor (a white
    grid appears in the background where the reference image is
    all black).

diff --git a/boilerplate/cairo-boilerplate-pdf.c b/boilerplate/cairo-boilerplate-pdf.c
index 499e1cc..c89f298 100644
--- a/boilerplate/cairo-boilerplate-pdf.c
+++ b/boilerplate/cairo-boilerplate-pdf.c
@@ -77,19 +77,6 @@ static const char *pdf_ignored_tests[] = {
     "radial-gradient",
     "text-pattern",
     "trap-clip",
-    /* These next 7 failures are due to:
-     *
-     *	Poppler does not correctly handle knockout groups
-     *	https://bugs.freedesktop.org/show_bug.cgi?id=12185
-     */
-    "clip-operator",
-    "operator-clear",
-    "operator-source",
-    "over-above-source",
-    "over-around-source",
-    "over-below-source",
-    "over-between-source",
-    "unbounded-operator"
 };
 
 cairo_surface_t *
diff --git a/test/clip-operator-pdf-argb32-ref.png b/test/clip-operator-pdf-argb32-ref.png
new file mode 100644
index 0000000..ecb56b4
Binary files /dev/null and b/test/clip-operator-pdf-argb32-ref.png differ
diff --git a/test/operator-clear-pdf-argb32-ref.png b/test/operator-clear-pdf-argb32-ref.png
new file mode 100644
index 0000000..d36de27
Binary files /dev/null and b/test/operator-clear-pdf-argb32-ref.png differ
diff --git a/test/operator-clear-pdf-rgb24-ref.png b/test/operator-clear-pdf-rgb24-ref.png
new file mode 100644
index 0000000..2dec75c
Binary files /dev/null and b/test/operator-clear-pdf-rgb24-ref.png differ
commit 98189d860e2c4219f994dd19fb199fcf4b457852
Author: Carl Worth <cworth at cworth.org>
Date:   Wed Feb 20 01:43:51 2008 -0800

    svg: Add support for flattening SOURCE painting when there's nothing beneath it
    
    This avoids unnecessary rasterization in many cases when using
    cairo_surface_create_similar with an SVG surface. Because of that
    it eliminates test-suite failures for the -similar cases where we
    have svg-specific reference images. Namely:
    
    	font-matrix-translation, ft-text-vertical-layout-type1,
    	ft-text-vertical-layout-type3, mask, meta-surface-pattern,
    	paint-source-alpha, paint-with-alpha, rotate-image-surface-paint,
    	scale-source-surface-paint, source-clip-scale, text-pattern,
    	text-rotate
    
    In all of these cases the test suite was kindly noticing that we
    weren't getting the same 'native' SVG output that was desired.

diff --git a/src/cairo-svg-surface.c b/src/cairo-svg-surface.c
index b2e65fb..895cb5e 100644
--- a/src/cairo-svg-surface.c
+++ b/src/cairo-svg-surface.c
@@ -752,11 +752,18 @@ _cairo_svg_surface_analyze_operation (cairo_svg_surface_t   *surface,
     if (cairo_svg_force_fallbacks)
 	return FALSE;
 
-    if (document->svg_version < CAIRO_SVG_VERSION_1_2)
-	if (op != CAIRO_OPERATOR_OVER)
-	    return CAIRO_INT_STATUS_UNSUPPORTED;
+    if (document->svg_version >= CAIRO_SVG_VERSION_1_2)
+	return CAIRO_STATUS_SUCCESS;
 
-    return CAIRO_STATUS_SUCCESS;
+    if (op == CAIRO_OPERATOR_OVER)
+	return CAIRO_STATUS_SUCCESS;
+
+    /* The SOURCE operator is onlysupported if there is nothing
+     * painted underneath. */
+    if (op == CAIRO_OPERATOR_SOURCE)
+	return CAIRO_INT_STATUS_FLATTEN_TRANSPARENCY;
+
+    return CAIRO_INT_STATUS_UNSUPPORTED;
 }
 
 static cairo_int_status_t
commit e7c6e291a64391004ae7bafc4d69830af5b0c8b1
Author: Carl Worth <cworth at cworth.org>
Date:   Wed Feb 20 01:43:22 2008 -0800

    Add svg-specific reference images for meta-surface-pattern test

diff --git a/test/meta-surface-pattern-svg-argb32-ref.png b/test/meta-surface-pattern-svg-argb32-ref.png
new file mode 100644
index 0000000..bd48555
Binary files /dev/null and b/test/meta-surface-pattern-svg-argb32-ref.png differ
diff --git a/test/meta-surface-pattern-svg-rgb24-ref.png b/test/meta-surface-pattern-svg-rgb24-ref.png
new file mode 100644
index 0000000..feda67d
Binary files /dev/null and b/test/meta-surface-pattern-svg-rgb24-ref.png differ
commit a1867412210db667a02e7b656729e2cf6a64a2d7
Author: Carl Worth <cworth at cworth.org>
Date:   Wed Feb 20 01:38:59 2008 -0800

    cairo-svg: Make operation_supported depend on analyze_operation instead of the other way around.
    
    This prepares for analyze_operation to be able to return more than
    just two values, (which will allow the svg backend to take advantage
    of CAIRO_INT_STATUS_FLATTEN_TRANSPARENCY).

diff --git a/src/cairo-svg-surface.c b/src/cairo-svg-surface.c
index 6cd5e08..b2e65fb 100644
--- a/src/cairo-svg-surface.c
+++ b/src/cairo-svg-surface.c
@@ -743,9 +743,9 @@ _cairo_svg_document_emit_font_subsets (cairo_svg_document_t *document)
 static cairo_bool_t cairo_svg_force_fallbacks = FALSE;
 
 static cairo_int_status_t
-_cairo_svg_surface_operation_supported (cairo_svg_surface_t	*surface,
-					cairo_operator_t	 op,
-					const cairo_pattern_t	*pattern)
+_cairo_svg_surface_analyze_operation (cairo_svg_surface_t   *surface,
+				      cairo_operator_t	     op,
+				      const cairo_pattern_t *pattern)
 {
     cairo_svg_document_t *document = surface->document;
 
@@ -754,20 +754,23 @@ _cairo_svg_surface_operation_supported (cairo_svg_surface_t	*surface,
 
     if (document->svg_version < CAIRO_SVG_VERSION_1_2)
 	if (op != CAIRO_OPERATOR_OVER)
-	    return FALSE;
+	    return CAIRO_INT_STATUS_UNSUPPORTED;
 
-    return TRUE;
+    return CAIRO_STATUS_SUCCESS;
 }
 
 static cairo_int_status_t
-_cairo_svg_surface_analyze_operation (cairo_svg_surface_t   *surface,
-				      cairo_operator_t	     op,
-				      const cairo_pattern_t *pattern)
+_cairo_svg_surface_operation_supported (cairo_svg_surface_t	*surface,
+					cairo_operator_t	 op,
+					const cairo_pattern_t	*pattern)
 {
-    if (_cairo_svg_surface_operation_supported (surface, op, pattern))
-	return CAIRO_STATUS_SUCCESS;
-    else
-	return CAIRO_INT_STATUS_UNSUPPORTED;
+    if (_cairo_svg_surface_analyze_operation (surface, op, pattern)
+	!= CAIRO_INT_STATUS_UNSUPPORTED)
+    {
+	return TRUE;
+    } else {
+	return FALSE;
+    }
 }
 
 static cairo_surface_t *
commit 8f11c4583f58f97c348aafcd71db059ac7791fe7
Author: Carl Worth <cworth at cworth.org>
Date:   Mon Feb 18 17:32:35 2008 -0800

    Revert "[meta-surface] Adjust tolerance when replaying to surfaces with device scale"
    
    This reverts commit 7f21bfb0a8265095d68bb5ff6afae7553683a776.
    
    We don't yet have consensus on whether this is a good change or not.
    So for now, we're favoring the existing behavior until we can work
    that out.

diff --git a/src/cairo-meta-surface.c b/src/cairo-meta-surface.c
index 152dd15..5248947 100644
--- a/src/cairo-meta-surface.c
+++ b/src/cairo-meta-surface.c
@@ -662,7 +662,6 @@ _cairo_meta_surface_replay_internal (cairo_surface_t	     *surface,
     cairo_bool_t has_device_transform = _cairo_surface_has_device_transform (target);
     cairo_matrix_t *device_transform = &target->device_transform;
     cairo_path_fixed_t path_copy, *dev_path;
-    double tolerance_multiplier = _cairo_matrix_transformed_circle_major_axis (device_transform, 1.0);
 
     if (surface->status)
 	return surface->status;
@@ -735,7 +734,7 @@ _cairo_meta_surface_replay_internal (cairo_surface_t	     *surface,
 					    &command->stroke.style,
 					    &dev_ctm,
 					    &dev_ctm_inverse,
-					    command->stroke.tolerance * tolerance_multiplier,
+					    command->stroke.tolerance,
 					    command->stroke.antialias);
 	    break;
 	}
@@ -772,7 +771,7 @@ _cairo_meta_surface_replay_internal (cairo_surface_t	     *surface,
 						     command->fill.op,
 						     &command->fill.source.base,
 						     command->fill.fill_rule,
-						     command->fill.tolerance * tolerance_multiplier,
+						     command->fill.tolerance,
 						     command->fill.antialias,
 						     dev_path,
 						     stroke_command->stroke.op,
@@ -780,7 +779,7 @@ _cairo_meta_surface_replay_internal (cairo_surface_t	     *surface,
 						     &stroke_command->stroke.style,
 						     &dev_ctm,
 						     &dev_ctm_inverse,
-						     stroke_command->stroke.tolerance * tolerance_multiplier,
+						     stroke_command->stroke.tolerance,
 						     stroke_command->stroke.antialias);
 		i++;
 		if (type == CAIRO_META_CREATE_REGIONS) {
@@ -797,7 +796,7 @@ _cairo_meta_surface_replay_internal (cairo_surface_t	     *surface,
 					      &command->fill.source.base,
 					      dev_path,
 					      command->fill.fill_rule,
-					      command->fill.tolerance * tolerance_multiplier,
+					      command->fill.tolerance,
 					      command->fill.antialias);
 	    break;
 	}
@@ -840,7 +839,7 @@ _cairo_meta_surface_replay_internal (cairo_surface_t	     *surface,
 	    else
 		status = _cairo_clip_clip (&clip, dev_path,
 					   command->intersect_clip_path.fill_rule,
-					   command->intersect_clip_path.tolerance * tolerance_multiplier,
+					   command->intersect_clip_path.tolerance,
 					   command->intersect_clip_path.antialias,
 					   target);
             assert (status == 0);
commit 1747718406b3450bc8869f0e050f9a18b3036030
Author: Carl Worth <cworth at cworth.org>
Date:   Thu Feb 14 12:27:47 2008 -0800

    Add a spline to the degenerate-pen test case
    
    This was triggering an infinite loop (with 24.8 fixed-point) just before
    the previous fix.
    
    NOTE: I usually put bug demonstrations just before the fixes, but this
    one was quite harsh---not only was cairo looping infinitely, but it
    was appending to an array on each iteration---so not kind at all.

diff --git a/test/degenerate-pen-ps-argb32-ref.png b/test/degenerate-pen-ps-argb32-ref.png
index 92ad91c..47b2900 100644
Binary files a/test/degenerate-pen-ps-argb32-ref.png and b/test/degenerate-pen-ps-argb32-ref.png differ
diff --git a/test/degenerate-pen-ref.png b/test/degenerate-pen-ref.png
index 0ad3325..539a325 100644
Binary files a/test/degenerate-pen-ref.png and b/test/degenerate-pen-ref.png differ
diff --git a/test/degenerate-pen.c b/test/degenerate-pen.c
index c92a689..858949f 100644
--- a/test/degenerate-pen.c
+++ b/test/degenerate-pen.c
@@ -66,6 +66,7 @@ draw (cairo_t *cr, int width, int height)
 
     /* First compress the pen to a vertical line. */
     cairo_rectangle (cr, 0, 0, SIZE, SIZE);
+    cairo_curve_to (cr, SIZE / 2, 0, SIZE, SIZE / 2, SIZE, SIZE);
     cairo_save (cr);
     {
 	cairo_scale (cr, 0.000001, 1.0);
@@ -77,6 +78,7 @@ draw (cairo_t *cr, int width, int height)
 
     /* Then compress the pen to a horizontal line. */
     cairo_rectangle (cr, 0, 0, SIZE, SIZE);
+    cairo_curve_to (cr, SIZE / 2, 0, SIZE, SIZE / 2, SIZE, SIZE);
     cairo_save (cr);
     {
 	cairo_scale (cr, 1.0, 0.000001);
@@ -88,6 +90,7 @@ draw (cairo_t *cr, int width, int height)
 
     /* Finally a line at an angle. */
     cairo_rectangle (cr, 0, 0, SIZE, SIZE);
+    cairo_curve_to (cr, SIZE / 2, 0, SIZE, SIZE / 2, SIZE, SIZE);
     cairo_save (cr);
     {
 	cairo_rotate (cr, M_PI / 4.0);
commit d6d81c92b501b32be6c48d134683ebd51f63c134
Author: Carl Worth <cworth at cworth.org>
Date:   Sat Feb 16 13:27:02 2008 -0800

    Eliminate a potential infinite loop in spline stroking
    
    Sometimes > rather than >= can make a bug difference. The infinite loop
    was noticed here:
    
    	Infinite loop when scaling very small values using 24.8
    	http://bugs.freedesktop.org/show_bug.cgi?id=14280
    
    Note that that particular test case only exposes the infinite
    loop when using 24.8 instead of 16.16 fixed-point values by
    setting CAIRO_FIXED_FRAC_BITS to 8.

diff --git a/src/cairo-pen.c b/src/cairo-pen.c
index fc01970..37456d9 100644
--- a/src/cairo-pen.c
+++ b/src/cairo-pen.c
@@ -417,7 +417,18 @@ _cairo_pen_stroke_spline_half (cairo_pen_t *pen,
 	    slope = final_slope;
 	else
 	    _cairo_slope_init (&slope, &point[i], &point[i+step]);
-	if (_cairo_slope_compare (&slope, &pen->vertices[active].slope_ccw) >= 0) {
+
+	/* The strict inequalities here ensure that if a spline slope
+	 * compares identically with either of the slopes of the
+	 * active vertex, then it remains the active vertex. This is
+	 * very important since otherwise we can trigger an infinite
+	 * loop in the case of a degenerate pen, (a line), where
+	 * neither vertex considers itself active for the slope---one
+	 * will consider it as equal and reject, and the other will
+	 * consider it unequal and reject. This is due to the inherent
+	 * ambiguity when comparing slopes that differ by exactly
+	 * pi. */
+	if (_cairo_slope_compare (&slope, &pen->vertices[active].slope_ccw) > 0) {
 	    if (++active == pen->num_vertices)
 		active = 0;
 	} else if (_cairo_slope_compare (&slope, &pen->vertices[active].slope_cw) < 0) {
commit 770b058c9e53c6d0e3a3f600cd5a9631eee39dff
Author: Carl Worth <cworth at cworth.org>
Date:   Sat Feb 16 13:17:49 2008 -0800

    Remove _cairo_slope_[counter_]clockwise
    
    These two functions were hiding away some important details
    about strictness of inequalities. Also, the callers differ
    on the strictness they need. Everything is cleaner and more
    flexible by making the callers just call _cairo_slope_compare
    directly.

diff --git a/src/cairo-path-stroke.c b/src/cairo-path-stroke.c
index 4344e6b..2c723f4 100644
--- a/src/cairo-path-stroke.c
+++ b/src/cairo-path-stroke.c
@@ -235,7 +235,7 @@ _cairo_stroker_face_clockwise (cairo_stroke_face_t *in, cairo_stroke_face_t *out
     _cairo_slope_init (&in_slope, &in->point, &in->cw);
     _cairo_slope_init (&out_slope, &out->point, &out->cw);
 
-    return _cairo_slope_clockwise (&in_slope, &out_slope);
+    return _cairo_slope_compare (&in_slope, &out_slope) < 0;
 }
 
 /**
diff --git a/src/cairo-pen.c b/src/cairo-pen.c
index cde129c..fc01970 100644
--- a/src/cairo-pen.c
+++ b/src/cairo-pen.c
@@ -302,13 +302,15 @@ _cairo_pen_compute_slopes (cairo_pen_t *pen)
 /*
  * Find active pen vertex for clockwise edge of stroke at the given slope.
  *
- * Note: The behavior of this function is sensitive to the sense of
- * the inequality within _cairo_slope_clockwise/_cairo_slope_counter_clockwise.
+ * The strictness of the inequalities here is delicate. The issue is
+ * that the slope_ccw member of one pen vertex will be equivalent to
+ * the slope_cw member of the next pen vertex in a counterclockwise
+ * order. However, for this function, we care strongly about which
+ * vertex is returned.
  *
- * The issue is that the slope_ccw member of one pen vertex will be
- * equivalent to the slope_cw member of the next pen vertex in a
- * counterclockwise order. However, for this function, we care
- * strongly about which vertex is returned.
+ * [I think the "care strongly" above has to do with ensuring that the
+ * pen's "extra points" from the spline's initial and final slopes are
+ * properly found when beginning the spline stroking.]
  */
 void
 _cairo_pen_find_active_cw_vertex_index (cairo_pen_t *pen,
@@ -318,8 +320,8 @@ _cairo_pen_find_active_cw_vertex_index (cairo_pen_t *pen,
     int i;
 
     for (i=0; i < pen->num_vertices; i++) {
-	if (_cairo_slope_clockwise (slope, &pen->vertices[i].slope_ccw)
-	    && _cairo_slope_counter_clockwise (slope, &pen->vertices[i].slope_cw))
+	if ((_cairo_slope_compare (slope, &pen->vertices[i].slope_ccw) < 0) &&
+	    (_cairo_slope_compare (slope, &pen->vertices[i].slope_cw) >= 0))
 	    break;
     }
 
@@ -336,8 +338,8 @@ _cairo_pen_find_active_cw_vertex_index (cairo_pen_t *pen,
 
 /* Find active pen vertex for counterclockwise edge of stroke at the given slope.
  *
- * Note: The behavior of this function is sensitive to the sense of
- * the inequality within _cairo_slope_clockwise/_cairo_slope_counter_clockwise.
+ * Note: See the comments for _cairo_pen_find_active_cw_vertex_index
+ * for some details about the strictness of the inequalities here.
  */
 void
 _cairo_pen_find_active_ccw_vertex_index (cairo_pen_t *pen,
@@ -352,8 +354,8 @@ _cairo_pen_find_active_ccw_vertex_index (cairo_pen_t *pen,
     slope_reverse.dy = -slope_reverse.dy;
 
     for (i=pen->num_vertices-1; i >= 0; i--) {
-	if (_cairo_slope_counter_clockwise (&pen->vertices[i].slope_ccw, &slope_reverse)
-	    && _cairo_slope_clockwise (&pen->vertices[i].slope_cw, &slope_reverse))
+	if ((_cairo_slope_compare (&pen->vertices[i].slope_ccw, &slope_reverse) >= 0) &&
+	    (_cairo_slope_compare (&pen->vertices[i].slope_cw, &slope_reverse) < 0))
 	    break;
     }
 
@@ -415,10 +417,10 @@ _cairo_pen_stroke_spline_half (cairo_pen_t *pen,
 	    slope = final_slope;
 	else
 	    _cairo_slope_init (&slope, &point[i], &point[i+step]);
-	if (_cairo_slope_counter_clockwise (&slope, &pen->vertices[active].slope_ccw)) {
+	if (_cairo_slope_compare (&slope, &pen->vertices[active].slope_ccw) >= 0) {
 	    if (++active == pen->num_vertices)
 		active = 0;
-	} else if (_cairo_slope_clockwise (&slope, &pen->vertices[active].slope_cw)) {
+	} else if (_cairo_slope_compare (&slope, &pen->vertices[active].slope_cw) < 0) {
 	    if (--active == -1)
 		active = pen->num_vertices - 1;
 	} else {
diff --git a/src/cairo-slope.c b/src/cairo-slope.c
index af97a63..d3f0db4 100644
--- a/src/cairo-slope.c
+++ b/src/cairo-slope.c
@@ -106,26 +106,3 @@ _cairo_slope_compare (cairo_slope_t *a, cairo_slope_t *b)
     /* Finally, for identical slopes, we obviously return 0. */
     return 0;
 }
-
-/* XXX: It might be cleaner to move away from usage of
-   _cairo_slope_clockwise/_cairo_slope_counter_clockwise in favor of
-   directly using _cairo_slope_compare.
-*/
-
-/* Is a clockwise of b?
- *
- * Note: The strict equality here is not significant in and of itself,
- * but there are functions up above that are sensitive to it,
- * (cf. _cairo_pen_find_active_cw_vertex_index).
- */
-int
-_cairo_slope_clockwise (cairo_slope_t *a, cairo_slope_t *b)
-{
-    return _cairo_slope_compare (a, b) < 0;
-}
-
-int
-_cairo_slope_counter_clockwise (cairo_slope_t *a, cairo_slope_t *b)
-{
-    return ! _cairo_slope_clockwise (a, b);
-}
diff --git a/src/cairoint.h b/src/cairoint.h
index 8f8685f..489cf40 100644
--- a/src/cairoint.h
+++ b/src/cairoint.h
@@ -2086,12 +2086,6 @@ _cairo_slope_init (cairo_slope_t *slope, cairo_point_t *a, cairo_point_t *b);
 cairo_private int
 _cairo_slope_compare (cairo_slope_t *a, cairo_slope_t *b);
 
-cairo_private int
-_cairo_slope_clockwise (cairo_slope_t *a, cairo_slope_t *b);
-
-cairo_private int
-_cairo_slope_counter_clockwise (cairo_slope_t *a, cairo_slope_t *b);
-
 /* cairo_pattern.c */
 
 cairo_private cairo_status_t
commit 2d22d698d32575fd883d2be072c041709254c86d
Author: Carl Worth <cworth at cworth.org>
Date:   Thu Feb 14 11:50:26 2008 -0800

    Make _cairo_slope_compare return a non-zero result for slopes that differ by pi
    
    This was an initial attempt to fix the infinite loop bug
    described here:
    
    	Infinite loop when scaling very small values using 24.8
    	http://bugs.freedesktop.org/show_bug.cgi?id=14280
    
    This doesn't actually fix that bug, but having a more robust
    comparison function can only be a good thing.

diff --git a/src/cairo-slope.c b/src/cairo-slope.c
index e8421fc..af97a63 100644
--- a/src/cairo-slope.c
+++ b/src/cairo-slope.c
@@ -47,8 +47,15 @@ _cairo_slope_init (cairo_slope_t *slope, cairo_point_t *a, cairo_point_t *b)
    positive X axis and increase in the direction of the positive Y
    axis.
 
-   WARNING: This function only gives correct results if the angular
-   difference between a and b is less than PI.
+   This function always compares the slope vectors based on the
+   smaller angular difference between them, (that is based on an
+   angular difference that is strictly less than pi). To break ties
+   when comparing slope vectors with an angular difference of exactly
+   pi, the vector with a positive dx (or positive dy if dx's are zero)
+   is considered to be more positive than the other.
+
+   Also, all slope vectors with both dx==0 and dy==0 are considered
+   equal and more positive than any non-zero vector.
 
    <  0 => a less positive than b
    == 0 => a equal to b
@@ -78,6 +85,25 @@ _cairo_slope_compare (cairo_slope_t *a, cairo_slope_t *b)
     if (b->dx == 0 && b->dy ==0)
 	return -1;
 
+    /* Finally, we're looking at two vectors that are either equal or
+     * that differ by exactly pi. We can identify the "differ by pi"
+     * case by looking for a change in sign in either dx or dy between
+     * a and b.
+     *
+     * And in these cases, we eliminate the ambiguity by reducing the angle
+     * of b by an infinitesimally small amount, (that is, 'a' will
+     * always be considered less than 'b').
+     */
+    if (((a->dx > 0) != (b->dx > 0)) ||
+	((a->dy > 0) != (b->dy > 0)))
+    {
+	if (a->dx > 0 || (a->dx == 0 && a->dy > 0))
+	    return +1;
+	else
+	    return -1;
+    }
+
+    /* Finally, for identical slopes, we obviously return 0. */
     return 0;
 }
 


More information about the cairo-commit mailing list