| Summary: | cairo_close_path should not change extents on a "empty" path | ||
|---|---|---|---|
| Product: | cairo | Reporter: | Jussi Kukkonen [inactive] <jussi.kukkonen> |
| Component: | general | Assignee: | Carl Worth <cworth> |
| Status: | RESOLVED FIXED | QA Contact: | cairo-bugs mailing list <cairo-bugs> |
| Severity: | normal | ||
| Priority: | medium | CC: | freedesktop |
| Version: | 1.12.2 | ||
| Hardware: | Other | ||
| OS: | All | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
| Attachments: |
path-fixed: do not let temporary lines affect path extents
path-fixed: do not let temporary lines affect path extents path-fixed: do not let temporary lines affect path extents Test case for degenerate closed path |
||
Created attachment 66672 [details] [review] path-fixed: do not let temporary lines affect path extents This seems to do the trick. Created attachment 66673 [details] [review] path-fixed: do not let temporary lines affect path extents Oops, fix style to match rest of file Found one more way to break this: data="M 1 1 Z M 1 2 Z" The Z in the middle screws things up. Not quite. The issue is that the zero-extents are wrong. The path exists but is degenerate (if you stroke it, it should generate a point for example).
So I think the fix is:
diff --git a/src/cairo-path-fixed.c b/src/cairo-path-fixed.c
index 459c680..c7b1cab 100644
--- a/src/cairo-path-fixed.c
+++ b/src/cairo-path-fixed.c
@@ -419,6 +419,7 @@ _cairo_path_fixed_move_to (cairo_path_fixed_t *path,
path->has_current_point = TRUE;
path->current_point.x = x;
path->current_point.y = y;
+ path->last_move_point = path->current_point;
return CAIRO_STATUS_SUCCESS;
}
Created attachment 66676 [details] [review] path-fixed: do not let temporary lines affect path extents ...and this one works for all the combinations of M and Z I could think of. > Not quite. The issue is that the zero-extents are wrong. The path exists but is
> degenerate (if you stroke it, it should generate a point for example).
Yeah, that looks more correct -- the zero sized extent is now in the correct location.
I confirm your patch passes the webkit tests I was looking at as well as the ones I cooked up. Thanks.
commit b0c466e27afcec230b2c9436eeb924c05123a544 Author: Chris Wilson <chris@chris-wilson.co.uk> Date: Wed Sep 5 14:55:55 2012 +0100 path: Update last_move_point after move-to Reported-and-tested-by: Jussi Kukkonen <jku@linux.intel.com> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=54549 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> I should also write a test-case. :| Created attachment 66684 [details] [review] Test case for degenerate closed path I can confirm that the patch from Chris is the right one (yes, in Cairo semantics, a degenerate path can have non-zero path extents). Here is the testcase. You should differentiate between stroke extents and clip/fill extents. |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.
This code: cairo_move_to (cr, -1, 1); cairo_path_extents (cr, &x1, &y1, &x2, &y2); printf ("extents before close_path: %f %f %f %f\n", x1,y1,x2,y2); cairo_close_path (cr); cairo_path_extents (cr, &x1, &y1, &x2, &y2); printf ("extents after close_path: %f %f %f %f\n", x1,y1,x2,y2); will output extents before close_path: 0.000000 0.000000 0.000000 0.000000 extents after close_path: -1.000000 0.000000 0.000000 1.000000 That seems like a bug to me: Shouldn't the boundingbox still be zero sized after cairo_close_path(). This started happening at commit 166453c1ab.