GNOME Bugzilla – Bug 763386
painting-marker-properties-01-f.svg
Last modified: 2016-12-19 21:34:43 UTC
This patch shows one way to improve the output of the test painting-marker-properties-01-f of the svg 1.1 test suite diff --git a/rsvg-marker.c b/rsvg-marker.c index bcca6d7..9e32d85 100644 --- a/rsvg-marker.c +++ b/rsvg-marker.c @@ -354,7 +354,14 @@ path_to_segments (const cairo_path_t *path, cur_x = subpath_start_x; cur_y = subpath_start_y; - if (state == SEGMENT_START) { + if (state == SEGMENT_END) { + segment_num++; + g_assert (segment_num < max_segments); + + segments[segment_num].is_degenerate = FALSE; + + segments[segment_num].p1x = last_x; + segments[segment_num].p1y = last_y; segments[segment_num].is_degenerate = FALSE; segments[segment_num].p2x = cur_x; diff --git a/rsvg-path.c b/rsvg-path.c index ee32ee6..61cca2e 100644 --- a/rsvg-path.c +++ b/rsvg-path.c @@ -148,17 +148,17 @@ rsvg_path_builder_close_path (RsvgPathBuilder *builder) { cairo_path_data_t data; - rsvg_path_builder_ensure_capacity (builder, 1); + rsvg_path_builder_ensure_capacity (builder, 1 + (builder->last_move_to_index >= 0)); data.header.type = CAIRO_PATH_CLOSE_PATH; - data.header.length = 1; + data.header.length = 1 + (builder->last_move_to_index >= 0); rsvg_path_builder_add_element (builder, &data); /* Add a 'move-to' element */ if (builder->last_move_to_index >= 0) { cairo_path_data_t *moveto = &g_array_index (builder->path_data, cairo_path_data_t, builder->last_move_to_index); - rsvg_path_builder_move_to (builder, moveto[1].point.x, moveto[1].point.y); + rsvg_path_builder_add_element (builder, moveto + 1); } }
This is fixed in the rustification branch. It will get merged to master at some point.
I checked out and built the rustification branch and I don't think that representing all curves (linear included) as cubic is a great idea. For example marker directionality is still computed incorrectly in this case: <svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" viewBox="0 0 64 64"> <marker id="marker" viewBox="-4 -4 8 8" orient="auto"> <rect x="-1" y="-4" width="2" height="8" /> </marker> <path stroke-width="8" marker-start="url(#marker)" stroke="red" d="M 12,12 c 0,0 0,0 40,40"/> </svg>
Excellent test case, thank you! We didn't handle the case were three control points are coincident. This is fixed in commit f52d96f657f99c205f282fb9dec70459902bff48. I took the liberty of including your test case and modifying it a bit, so it has a) a triangular marker, so orientation can be seen easily; b) two paths, one where the first/second/third control points coincide, and another where the second/third/fourth coincide.
(In reply to Federico Mena Quintero from comment #3) > Excellent test case, thank you! We didn't handle the case were three > control points are coincident. This is fixed in commit > f52d96f657f99c205f282fb9dec70459902bff48. > > I took the liberty of including your test case and modifying it a bit, so it > has a) a triangular marker, so orientation can be seen easily; b) two paths, > one where the first/second/third control points coincide, and another where > the second/third/fourth coincide. I'm sorry, I feel I'm confusing more than helping, but it seems that the test suite should also include the case where P1 == P2 and P3 == P4 with P2 != P3 because it seems it is still not correct: <svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" viewBox="0 0 64 64"> <marker id="marker" viewBox="-4 -4 8 8" orient="auto"> <rect x="-1" y="-4" width="2" height="8" /> </marker> <path stroke-width="8" marker-end="url(#marker)" stroke="red" d="M 12,12 c 0,0 40,40 40,40"/> </svg>
(In reply to Massimo from comment #4) > > I'm sorry, I feel I'm confusing more than helping, but > it seems that the test suite should also include the case > where P1 == P2 and P3 == P4 with P2 != P3 because it seems > it is still not correct: Thanks again! Added this into the existing test case and fixed the code. ... do you have any other similar cases that don't pass? I *think* we handle all the possible cases now, but I haven't had nearly enough coffee today to be 100% sure.
(In reply to Federico Mena Quintero from comment #5) > (In reply to Massimo from comment #4) > > > > I'm sorry, I feel I'm confusing more than helping, but > > it seems that the test suite should also include the case > > where P1 == P2 and P3 == P4 with P2 != P3 because it seems > > it is still not correct: > > Thanks again! Added this into the existing test case and fixed the code. > > ... do you have any other similar cases that don't pass? I *think* we > handle all the possible cases now, but I haven't had nearly enough coffee > today to be 100% sure. Tests failing related to marker element/attribute/property? The 'marker' short-hand property is not handled as shown by the 'painting-marker-03-f.svg' test in the svg1.1 testsuite. Regarding computing marker orientation try this one: (git describe says: 2.40.16-237-gf6bf80e) <svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" viewBox="0 0 128 64"> <marker id="marker" viewBox="0 -1 4 2" orient="auto"> <path fill="blue" d="M 0 -1 L 4 0 0 1" /> </marker> <path stroke-width="4" marker-mid="url(#marker)" stroke="red" fill="none" d="M 12,12 l 40,0 0,40 -40,0 0,-40"/> <circle stroke-width="4" cx="32" cy="32" r="20" marker-mid="url(#marker)" stroke="red" fill="none"/> <rect stroke-width="4" x="76" y="12" width="40" height="40" marker-mid="url(#marker)" stroke="red" fill="none"/> </svg>
Moving that test file to a new bug.