The 2Geom sync is almost done. So far I have identified two problem areas.
2Geom is now a lot more strict about what it considers a degenerate closing segment - it requires exact equality of endpoints, not just nearness to some arbitrary precision. This is of particular importance in paths where the last segment is curved. Because Inkscape loses precision when storing paths, bug #515237 now manifests on every closed path with a non-linear last segment.
https://bugs.launchpad.net/inkscape/+bug/515237
I intend to fix this in the following way: first, modify SVGPathParser so that it knows whether the last segment before 'z' was relative, and if it is, snap it to the final node with a precision set by another function call, 1e-6 by default; second, by making Inkscape output paths with the double-conversion library now integrated in 2Geom and always using absolute coordinates, so that there is perfect roundtrip between XML and internal representation. This will mean the removal of the Output Precision setting and a moderate increase in file sizes. The proper place for such functionality is in a separate export filter. As a bonus, setting the document unit to metres will no longer cause a variety of issues.
We discussed output precision briefly on the hackfest. Moving to full precision output with perfect roundtrip is the only correct solution and will enable us to avoid excessive recomputation of attributes in the SP tree, which is a performance win. If someone wants to get rid of extra precision - use an export filter. However, if anyone has a different opinion please speak up now.
The second problem is the transformation of elliptical arcs, as seen when manipulating ellipses in the selector tool. In some cases, the large arc / sweep flags are incorrectly set for transformed arcs. I am still looking for the cause of this bug.
The sync branch is at lp:~inkscape.dev/inkscape/lib2geom-sync - if anyone feels like looking for additional bugs, please do. I only did minimal testing on it.
Regards, Krzysztof
On May 7, 2015 10:18 PM, "Krzysztof Kosiński" <tweenk.pl@...400...> wrote:
We discussed output precision briefly on the hackfest. Moving to full precision output with perfect roundtrip is the only correct solution and will enable us to avoid excessive recomputation of attributes in the SP tree, which is a performance win. If someone wants to get rid of extra precision - use an export filter. However, if anyone has a different opinion please speak up now.
My opinion is in line with yours. I will however say if removing the precision preference, we need to make sure the export filter exists by the next release.
The second problem is the transformation of elliptical arcs, as seen when manipulating ellipses in the selector tool. In some cases, the large arc / sweep flags are incorrectly set for transformed arcs. I am still looking for the cause of this bug.
What's the bug report number on this one?
Cheers, Josh
2015-05-08 15:20 GMT+02:00 Josh Andler <scislac@...400...>:
The second problem is the transformation of elliptical arcs, as seen when manipulating ellipses in the selector tool. In some cases, the large arc / sweep flags are incorrectly set for transformed arcs. I am still looking for the cause of this bug.
What's the bug report number on this one?
Fixed that one a few minutes ago. I made two mistakes when simplifying the ellipse transform function in 2Geom, and there are no unit tests for ellipses yet...
At this point there are no blatantly obvious regressions in the sync branch, but my testing was rather limited - I only tested things which were likely to break based on the adjustments I made to Inkscape code. In particular, EMF / WMF import and export and some LPEs need to be checked.
Regards, Krzysztof
On Fri, May 8, 2015 at 6:57 AM, Krzysztof Kosiński <tweenk.pl@...1063....> wrote:
At this point there are no blatantly obvious regressions in the sync branch, but my testing was rather limited - I only tested things which were likely to break based on the adjustments I made to Inkscape code. In particular, EMF / WMF import and export and some LPEs need to be checked.
So... check it out and bang on stuff. Got it! :)
Cheers, Josh
I'm not sure if it's out of sync with trunk, but while messing around it crashes if I try to use the eraser tool on a closed path.
The taper stroke LPE is crashing for me too on a converted star object.
I'll hopefully be able to test more today.
Cheers, Josh
On Fri, May 8, 2015 at 7:01 AM, Josh Andler <scislac@...400...> wrote:
On Fri, May 8, 2015 at 6:57 AM, Krzysztof Kosiński <tweenk.pl@...2179......> wrote:
At this point there are no blatantly obvious regressions in the sync branch, but my testing was rather limited - I only tested things which were likely to break based on the adjustments I made to Inkscape code. In particular, EMF / WMF import and export and some LPEs need to be checked.
So... check it out and bang on stuff. Got it! :)
Cheers, Josh
2015-05-08 17:53 GMT+02:00 Josh Andler <scislac@...400...>:
I'm not sure if it's out of sync with trunk, but while messing around it crashes if I try to use the eraser tool on a closed path.
Now fixed, but the eraser tool is REALLY badly written...
- it creates a new XML object for the subtraction shape - it tries to create a "stroke" around the mouse path, but does it rather poorly, and the caps are not even approximately correct - it has a pre-existing bug where erasing a rectangle sometimes won't work
Some of these could be fixed by rewriting it to use the path outliner, but I'm not sure how robust that code is.
Regards, Krzysztof
2015-05-09 19:27 GMT+02:00 Liam White <inkscapebrony@...400...>:
On May 9, 2015 11:30 AM, "Krzysztof Kosiński" <tweenk.pl@...400...> wrote:
Some of these could be fixed by rewriting it to use the path outliner, but I'm not sure how robust that code is.
Regards, Krzysztof
Why don't we make the change and find out ;)
I'd rather keep the delta between the 2Geom sync branch and the trunk reasonably small. Further improvements can be made after the sync branch is merged.
Regards, Krzysztof
On Fri, 2015-05-08 at 07:17 +0200, Krzysztof Kosiński wrote:
The 2Geom sync is almost done. So far I have identified two problem areas.
2Geom is now a lot more strict about what it considers a degenerate closing segment - it requires exact equality of endpoints, not just nearness to some arbitrary precision. This is of particular importance in paths where the last segment is curved. Because Inkscape loses precision when storing paths, bug #515237 now manifests on every closed path with a non-linear last segment.
https://bugs.launchpad.net/inkscape/+bug/515237
I intend to fix this in the following way: first, modify SVGPathParser so that it knows whether the last segment before 'z' was relative, and if it is, snap it to the final node with a precision set by another function call, 1e-6 by default; second, by making Inkscape output paths with the double-conversion library now integrated in 2Geom and always using absolute coordinates, so that there is perfect roundtrip between XML and internal representation. This will mean the removal of the Output Precision setting and a moderate increase in file sizes. The proper place for such functionality is in a separate export filter. As a bonus, setting the document unit to metres will no longer cause a variety of issues.
We discussed output precision briefly on the hackfest. Moving to full precision output with perfect roundtrip is the only correct solution and will enable us to avoid excessive recomputation of attributes in the SP tree, which is a performance win. If someone wants to get rid of extra precision - use an export filter. However, if anyone has a different opinion please speak up now.
It's not clear to me what the plan is for enabling an export filter. We definitely need to offer our users the ability to limit the precision of the output so before we store double precision numbers we need to have this output filter in place. In addition, the ability to save as relative coordinates is important (I use it all the time and it has been requested by others; on the other hand, saving mixed absolute and relative coordinates to save a few bytes is probably not very useful).
Recall, the SVG 2 spec now allows 'z' to fill in missing points at the end of the path with the value of the first point so there is no need to create a 'zero' length line segment to close a path due to rounding errors. We should be able to parse this right away. Writing out this way should probably wait until the other renderers have caught up.
Tav
2015-05-08 18:04 GMT+02:00 Tavmjong Bah <tavmjong@...8...>:
It's not clear to me what the plan is for enabling an export filter. We definitely need to offer our users the ability to limit the precision of the output so before we store double precision numbers we need to have this output filter in place. In addition, the ability to save as relative coordinates is important (I use it all the time and it has been requested by others; on the other hand, saving mixed absolute and relative coordinates to save a few bytes is probably not very useful).
I can do some more work on Geom::SVGPathWriter so that it can be configured to do these things.
The export filter could be implemented as a visitor that creates a separate XML document and populates it by calling write() method on the SP tree. I'll look into this before changing over to full precision.
Recall, the SVG 2 spec now allows 'z' to fill in missing points at the end of the path with the value of the first point so there is no need to create a 'zero' length line segment to close a path due to rounding errors. We should be able to parse this right away. Writing out this way should probably wait until the other renderers have caught up.
Right now I 'snap' the final node to the initial node of the path when the initial moveto or the last segment before 'z' is given in relative coordinates. If both are given in absolute coordinates, they will maintain exact coincidence despite rounding and therefore no snapping is required.
Even when we move to extended 'z', the mandatory linear closing segment remains useful, because it ensures that the size of range begin(), end_closed() is equal to the number of nodes, i.e. there are as many nodes in the path as there are segments between begin(), end_closed(). I will probably exploit this when adding node-centric path operations to 2Geom. These operations are interactive, so they can be O(N) since rendering is also at the very least O(N).
PS: should the path parser in 2Geom have SVG 1.1 and SVG 2 versions, or just a single version that accepts everything? Is it useful to report errors for SVG 2 commands that are not in SVG 1.1?
Regards, Krzysztof
On Fri, 2015-05-08 at 19:45 +0200, Krzysztof Kosiński wrote:
2015-05-08 18:04 GMT+02:00 Tavmjong Bah <tavmjong@...8...>:
It's not clear to me what the plan is for enabling an export filter. We definitely need to offer our users the ability to limit the precision of the output so before we store double precision numbers we need to have this output filter in place. In addition, the ability to save as relative coordinates is important (I use it all the time and it has been requested by others; on the other hand, saving mixed absolute and relative coordinates to save a few bytes is probably not very useful).
I can do some more work on Geom::SVGPathWriter so that it can be configured to do these things.
Excellent
The export filter could be implemented as a visitor that creates a separate XML document and populates it by calling write() method on the SP tree. I'll look into this before changing over to full precision.
Recall, the SVG 2 spec now allows 'z' to fill in missing points at the end of the path with the value of the first point so there is no need to create a 'zero' length line segment to close a path due to rounding errors. We should be able to parse this right away. Writing out this way should probably wait until the other renderers have caught up.
Right now I 'snap' the final node to the initial node of the path when the initial moveto or the last segment before 'z' is given in relative coordinates. If both are given in absolute coordinates, they will maintain exact coincidence despite rounding and therefore no snapping is required.
Even when we move to extended 'z', the mandatory linear closing segment remains useful, because it ensures that the size of range begin(), end_closed() is equal to the number of nodes, i.e. there are as many nodes in the path as there are segments between begin(), end_closed(). I will probably exploit this when adding node-centric path operations to 2Geom. These operations are interactive, so they can be O(N) since rendering is also at the very least O(N).
OK, but we need to be careful about how markers are drawn. Having or not having a small closing segment can effect which markers are drawn where.
PS: should the path parser in 2Geom have SVG 1.1 and SVG 2 versions, or just a single version that accepts everything? Is it useful to report errors for SVG 2 commands that are not in SVG 1.1?
Just one version. It would be useful to report errors. There are a couple of new path commands 'r' and 'b' but I doubt anybody will implement them soon.
Tav
2015-05-08 20:18 GMT+02:00 Tavmjong Bah <tavmjong@...8...>:
PS: should the path parser in 2Geom have SVG 1.1 and SVG 2 versions, or just a single version that accepts everything? Is it useful to report errors for SVG 2 commands that are not in SVG 1.1?
Just one version. It would be useful to report errors. There are a couple of new path commands 'r' and 'b' but I doubt anybody will implement them soon.
So e.g. the path parser accepts everything but you can ask it whether what was read was SVG 1.1 compliant?
Regards, Krzysztof
participants (4)
-
Josh Andler -
Krzysztof Kosiński -
Liam White -
Tavmjong Bah