[cairo-commit] 4 commits - ROADMAP src/cairo.c src/cairo-path.c src/cairo-path-data.c test/close-path.c test/close-path-ps-argb32-ref.png test/close-path-ref.png test/.gitignore test/Makefile.am

Carl Worth cworth at kemper.freedesktop.org
Fri Aug 18 06:33:09 PDT 2006


 ROADMAP                           |    2 
 src/cairo-path-data.c             |    6 --
 src/cairo-path.c                  |   15 ++++--
 src/cairo.c                       |    8 +++
 test/.gitignore                   |    1 
 test/Makefile.am                  |    3 +
 test/close-path-ps-argb32-ref.png |binary
 test/close-path-ref.png           |binary
 test/close-path.c                 |   84 ++++++++++++++++++++++++++++++++++++++
 9 files changed, 108 insertions(+), 11 deletions(-)

New commits:
diff-tree c2d92d4397f7ed7a8b7fdfa24a4e339ecb0d6d69 (from c78c0110179f8f832e9096ad5e26f5887100cd59)
Author: Carl Worth <cworth at cworth.org>
Date:   Fri Aug 18 06:32:43 2006 -0700

    ROADMAP: Mark the close path bug as fixed.

diff --git a/ROADMAP b/ROADMAP
index 48c8299..065e7f3 100644
--- a/ROADMAP
+++ b/ROADMAP
@@ -38,7 +38,7 @@ cairo-1.2.4 (August 21, 2006): Fix build
 ✓- Type1 on Windows (Adrian has a patch)
 ✓- source-clip-scale
  - PS/PDF Type1/Type3 problem with rotated font_matrices
- - close_path behavior (T Rowley's mail)
+✓- close_path behavior (T Rowley's mail)
 
 cairo-1.4 (October 2006): Better performance
  - New tessellator
diff-tree c78c0110179f8f832e9096ad5e26f5887100cd59 (from 53f74e59faf1af78f2f0741ccf1f23aa5dad4efc)
Author: Carl Worth <cworth at cworth.org>
Date:   Fri Aug 18 06:31:15 2006 -0700

    Don't set current point to (0,0) in close_path.
    
    The setting of current point to (0,0) is actually harmless, but it
    definitely looks like a bug, (since after close_path the current point
    is really the last move point).
    
    We don't keep track of last move point here, nor do we even need to.
    So we can be consistent with _cairo_path_fixed_close_path by not
    adjusting current point at all, (the subsequent move_to coming right
    behind the close_path will fix up the current point).

diff --git a/src/cairo-path-data.c b/src/cairo-path-data.c
index 2407e72..cd15192 100644
--- a/src/cairo-path-data.c
+++ b/src/cairo-path-data.c
@@ -125,9 +125,6 @@ _cpdc_close_path (void *closure)
 
     cpdc->count += 1;
 
-    cpdc->current_point.x = 0;
-    cpdc->current_point.y = 0;
-
     return CAIRO_STATUS_SUCCESS;
 }
 
@@ -304,9 +301,6 @@ _cpdp_close_path (void *closure)
 
     cpdp->data += data->header.length;
 
-    cpdp->current_point.x = 0;
-    cpdp->current_point.y = 0;
-
     return CAIRO_STATUS_SUCCESS;
 }
 
diff-tree 53f74e59faf1af78f2f0741ccf1f23aa5dad4efc (from 200a2d811efab2e48d6b584b9da202effaddf99f)
Author: Carl Worth <cworth at cworth.org>
Date:   Fri Aug 18 06:27:45 2006 -0700

    Fix close-path failure by adding explicit move_to after close_path.
    
    Besides the bug fix, this is a user-visible change since the new
    move_to element after the close_path element can be seen in the
    results of cairo_copy_path, so we document that here.
    
    We are also careful to fix up _cairo_path_fixed_line_to to defer to
    _cairo_path_fixed_move_to to avoid letting the last_move_point state
    get stale. This avoids introducing the second bug that is also tested
    by the close-path test case.

diff --git a/src/cairo-path.c b/src/cairo-path.c
index b0c094e..9549f8d 100644
--- a/src/cairo-path.c
+++ b/src/cairo-path.c
@@ -228,8 +228,13 @@ _cairo_path_fixed_line_to (cairo_path_fi
     point.x = x;
     point.y = y;
 
+    /* When there is not yet a current point, the line_to operation
+     * becomes a move_to instead. Note: We have to do this by
+     * explicitly calling into _cairo_path_fixed_line_to to ensure
+     * that the last_move_point state is updated properly.
+     */
     if (! path->has_current_point)
-	status = _cairo_path_fixed_add (path, CAIRO_PATH_OP_MOVE_TO, &point, 1);
+	status = _cairo_path_fixed_move_to (path, point.x, point.y);
     else
 	status = _cairo_path_fixed_add (path, CAIRO_PATH_OP_LINE_TO, &point, 1);
 
@@ -328,9 +333,11 @@ _cairo_path_fixed_close_path (cairo_path
     if (status)
 	return status;
 
-    path->current_point.x = path->last_move_point.x;
-    path->current_point.y = path->last_move_point.y;
-    path->has_current_point = TRUE;
+    status = _cairo_path_fixed_move_to (path,
+					path->last_move_point.x,
+					path->last_move_point.y);
+    if (status)
+	return status;
 
     return CAIRO_STATUS_SUCCESS;
 }
diff --git a/src/cairo.c b/src/cairo.c
index 837976f..928ebca 100644
--- a/src/cairo.c
+++ b/src/cairo.c
@@ -1645,6 +1645,14 @@ cairo_stroke_to_path (cairo_t *cr)
  *
  * If there is no current point before the call to cairo_close_path,
  * this function will have no effect.
+ *
+ * Note: As of cairo version 1.2.4 any call to cairo_close_path will
+ * place an explicit MOVE_TO element into the path immediately after
+ * the CLOSE_PATH element, (which can be seen in cairo_copy_path() for
+ * example). This can simplify path processing in some cases as it may
+ * not be necessary to save the "last move_to point" during processing
+ * as the MOVE_TO immediately after the CLOSE_PATH will provide that
+ * point.
  **/
 void
 cairo_close_path (cairo_t *cr)
diff-tree 200a2d811efab2e48d6b584b9da202effaddf99f (from 8330f4dbd123da57850756a194ba9f7558e6f9cc)
Author: Carl Worth <cworth at cworth.org>
Date:   Fri Aug 18 06:15:02 2006 -0700

    close-path: New test case to demonstrate corner case discovered by Tim Rowley
    
    The bug shows up when doing cairo_copy_path_flat for a path that has
    a curve_to immediately after a close_path. When the curve is flattened
    the flattener is using (0,0) as the initial point rather than the proper
    close_to point.
    
    This test also serves to ensure a similar bug doesn't crop up when
    closing a path that begins with an implicit move_to, (as from cairo_arc).
    In that bug the path state may have no last-move-point and the path
    is closed to (0,0). This bug is not present currently, but did appear
    during the development of a fix for the bug above.

diff --git a/test/.gitignore b/test/.gitignore
index 40332f9..f3e91bc 100644
--- a/test/.gitignore
+++ b/test/.gitignore
@@ -15,6 +15,7 @@ clip-fill-rule-pixel-aligned
 clip-nesting
 clip-operator
 clip-twice
+close-path
 composite-integer-translate-source
 composite-integer-translate-over
 composite-integer-translate-over-repeat
diff --git a/test/Makefile.am b/test/Makefile.am
index 87da1b8..e37449a 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -10,6 +10,7 @@ clip-fill-rule-pixel-aligned	\
 clip-nesting			\
 clip-operator			\
 clip-twice			\
+close-path			\
 composite-integer-translate-source \
 composite-integer-translate-over \
 composite-integer-translate-over-repeat \
@@ -174,6 +175,8 @@ clip-operator-rgb24-ref.png				\
 clip-twice-ref.png					\
 clip-twice-rgb24-ref.png				\
 clip-twice-ps-argb32-ref.png				\
+close-path-ref.png					\
+close-path-ps-argb32-ref.png				\
 composite-integer-translate-over-ref.png		\
 composite-integer-translate-over-pdf-argb32-ref.png	\
 composite-integer-translate-over-svg-ref.png		\
diff --git a/test/close-path-ps-argb32-ref.png b/test/close-path-ps-argb32-ref.png
new file mode 100644
index 0000000..1f5790c
Binary files /dev/null and b/test/close-path-ps-argb32-ref.png differ
diff --git a/test/close-path-ref.png b/test/close-path-ref.png
new file mode 100644
index 0000000..538d9c6
Binary files /dev/null and b/test/close-path-ref.png differ
diff --git a/test/close-path.c b/test/close-path.c
new file mode 100644
index 0000000..04658b2
--- /dev/null
+++ b/test/close-path.c
@@ -0,0 +1,84 @@
+/*
+ * Copyright © 2006 Red Hat, Inc.
+ *
+ * Permission to use, copy, modify, distribute, and sell this software
+ * and its documentation for any purpose is hereby granted without
+ * fee, provided that the above copyright notice appear in all copies
+ * and that both that copyright notice and this permission notice
+ * appear in supporting documentation, and that the name of
+ * Red Hat, Inc. not be used in advertising or publicity pertaining to
+ * distribution of the software without specific, written prior
+ * permission. Red Hat, Inc. makes no representations about the
+ * suitability of this software for any purpose.  It is provided "as
+ * is" without express or implied warranty.
+ *
+ * RED HAT, INC. DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS
+ * SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
+ * FITNESS, IN NO EVENT SHALL RED HAT, INC. BE LIABLE FOR ANY SPECIAL,
+ * INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER
+ * RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION
+ * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR
+ * IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ *
+ * Author: Carl D. Worth <cworth at cworth.org>
+ */
+
+#include <stdlib.h>
+#include "cairo-test.h"
+
+static cairo_test_draw_function_t draw;
+
+cairo_test_t test = {
+    "close-path",
+    "Test some corner cases related to cairo_close_path",
+    32, 16,
+    draw
+};
+
+static cairo_test_status_t
+draw (cairo_t *cr, int width, int height)
+{
+    cairo_path_t *path;
+
+    /* We draw in the default black, so paint white first. */
+    cairo_save (cr);
+    cairo_set_source_rgb (cr, 1.0, 1.0, 1.0); /* white */
+    cairo_paint (cr);
+    cairo_restore (cr);
+
+    /* This curious approach for drawing a circle (starting with a
+     * closed arc) exercises a bug in which the "last move point" was
+     * not being set so the close_path closes to (0,0). */
+    cairo_arc (cr, 8, 8, 4, 0, M_PI);
+    cairo_close_path (cr);
+    cairo_arc (cr, 8, 8, 4, M_PI, 2 * M_PI);
+
+    cairo_fill (cr);
+
+    cairo_translate (cr, 16, 0);
+
+    /* Here a curve immediately after a close_to will begin from (0,0)
+     * when the path is obtained with cairo_copy_path_flat. */
+    cairo_move_to (cr, 8, 4);
+    cairo_arc_negative (cr, 8, 8, 4, 3 * M_PI / 2.0, M_PI / 2.0);
+    cairo_close_path (cr);
+    cairo_curve_to (cr,
+		    12, 4,
+		    12, 12,
+		    8, 12);
+
+    path = cairo_copy_path_flat (cr);
+    cairo_new_path (cr);
+    cairo_append_path (cr, path);
+    cairo_path_destroy (path);
+
+    cairo_fill (cr);
+
+    return CAIRO_TEST_SUCCESS;
+}
+
+int
+main (void)
+{
+    return cairo_test (&test);
+}


More information about the cairo-commit mailing list