2Gom sync - high potential for regressions after r14226
Hello
I recently made a commit that finally synchronizes the 2Geom version used in Inkscape and the "upstream" version in 2Geom's Launchpad repository. It compiles and appears to work correctly, but since there were several potentially breaking changes in 2Geom, it has a high potential for regressions.
The most important changes in 2Geom are:
1. The sign convention for cross(Point, Point) was flipped to match the one for cross(D2<SBasis>, D2<SBasis>): it is a[X]*b[Y] - a[Y]*b[X], not the other way around. I think I converted all uses, but one can never know.
2. All nearestPoint() methods were renamed to nearestTime(), since they return time (parameter) values, not points. nearestTime() for Path and PathVector returns PathTime and PathVectorTime respectively. PathTime encapsulates a curve index and a time value, while PathVectorTime encapsulated a path index, a curve index and a time value. The reason for using these types instead of extending the range of the time parameter from [0, 1] to [0, N] is twofold:
- Packing these two parameters into the double type causes a loss of acuracy as the indices increase. For example, referencing a point that lies on the 32nd curve causes a loss of at least 5 bits of precision.
- For PathVector, there is an unnecessary O(N) step involved in mapping the [0, N] time value to path and curve indices.
In the future, there will be two additional families of methods: nearestPoint(), which will actually return the nearest point (I avoided adding until the sync is done to simplify porting to the new API), and nearestPosition(), which will compute both the nearest point and the time value. Doing the latter can often save a lot of calculations, since in many cases the point is computed from the time or vice versa.
3. SVGEllipticalArc was renamed to EllipticalArc, and the old EllipticalArc was removed. The only difference between them was that EllipticalArc would throw exceptions if the constructor parameters did not specify a valid arc, whereas SVGEllipticalArc just falls back to a straight line. Additionally, a number of other improvements were made, for example EllipticalArc no longer allocates a copy of itself when transforming it.
4. There was a major cleanup of Path:
- Iterating between begin() and end_closed() will skip the linear closing segment if and only if its endpoints coincide exactly. SVGPathParser was modified to retain the old behavior of considering the closing segment degenerate if it is shorter than some specified tolerance.
- Internally, the path is now stored as a copy-on-write boost::ptr_vector of Curve. This should give a performance boost, since we are no longer using boost::shared_ptr-based copy-on-write for each curve, but only for entire Paths.
- PathVector is now a real class, not a typedef of PathVector, This allows several things to be implemented as methods instead of as standalone functions.
- PathSink was improved. Instead of creating a switch-like structure for the result of dynamic_cast of the abstract curve type to more concrete types, please derive from PathSink.
- There is a new class called CairoPathSink for outputting 2Geom paths and shapes to Cairo.
5. Circle and Ellipse were fleshed out, and contain many of the methods found in Curve and Path.
6. There is a new intersection API, exposed as a method on shapes and returning a class that contains two time values and the point of intersection. There are methods for Line, Circle, Ellipse, Path and PathVector.
Please test and report and regressions, but please make sure to check whether they are really regressions - for example the eraser tool sometimes doesn't work and it has nothing to do with the 2Geom sync, but rather with some faulty code in livarot.
Regards, Krzysztof
On Sat, 2015-07-04 at 19:32 +0200, Krzysztof Kosiński wrote:
Hello
I recently made a commit that finally synchronizes the 2Geom version used in Inkscape and the "upstream" version in 2Geom's Launchpad repository. It compiles and appears to work correctly, but since there were several potentially breaking changes in 2Geom, it has a high potential for regressions.
Excellent!
...
Please test and report and regressions, but please make sure to check whether they are really regressions - for example the eraser tool sometimes doesn't work and it has nothing to do with the 2Geom sync, but rather with some faulty code in livarot.
Speaking of cheese... what needs to be done before we can remove livarot?
Tav
2015-07-04 21:12 GMT+02:00 Tavmjong Bah <tavmjong@...8...>:
Speaking of cheese... what needs to be done before we can remove livarot?
Quite a lot, unfortunately - my boolops work is just the start. For example, the entire tweak tool is based on Livarot.
Regards, Krzysztof
On 2015-07-04 19:32 (+0200), Krzysztof Kosiński wrote:
I recently made a commit that finally synchronizes the 2Geom version used in Inkscape and the "upstream" version in 2Geom's Launchpad repository. It compiles and appears to work correctly, but since there were several potentially breaking changes in 2Geom, it has a high potential for regressions.
Reported build breakages:
Bug #1471622 compile error on Windows XP, Inkscape rev 14232, in conicsec.cpp https://bugs.launchpad.net/inkscape/+bug/1471622
Bug #1471587 trunk: build failure with llvm-gcc-4.2 (rev >= 14226) https://bugs.launchpad.net/inkscape/+bug/1471587
Not filed as report fo far:
CMake-based build is broken after the 2geom update (r14226), tentative fix: https://gist.github.com/su-v/938a659f639b43007ff2 (needs review, and comparison with changes to Makefile_insert)
Regards, V
2015-07-07 1:45 GMT+02:00 su_v <suv-sf@...58...>:
On 2015-07-04 19:32 (+0200), Krzysztof Kosiński wrote:
I recently made a commit that finally synchronizes the 2Geom version used in Inkscape and the "upstream" version in 2Geom's Launchpad repository. It compiles and appears to work correctly, but since there were several potentially breaking changes in 2Geom, it has a high potential for regressions.
Reported build breakages:
Bug #1471622 compile error on Windows XP, Inkscape rev 14232, in conicsec.cpp https://bugs.launchpad.net/inkscape/+bug/1471622
Bug #1471587 trunk: build failure with llvm-gcc-4.2 (rev >= 14226) https://bugs.launchpad.net/inkscape/+bug/1471587
These should hopefully be fixed in r14237.
Not filed as report fo far:
CMake-based build is broken after the 2geom update (r14226), tentative fix: https://gist.github.com/su-v/938a659f639b43007ff2 (needs review, and comparison with changes to Makefile_insert)
I included this patch in r14237.
Regards, Krzysztof
On 2015-07-04 19:32 (+0200), Krzysztof Kosiński wrote:
I recently made a commit that finally synchronizes the 2Geom version used in Inkscape and the "upstream" version in 2Geom's Launchpad repository. It compiles and appears to work correctly, but since there were several potentially breaking changes in 2Geom, it has a high potential for regressions.
Reported regressions:
Bug #1472518 Powerstroke LPE renders with spurious circles https://bugs.launchpad.net/inkscape/+bug/1472518 (has proposed patches pending review)
Bug #1473317 Power Stroke LPE crashes with rectangles https://bugs.launchpad.net/inkscape/+bug/1473317
Bug #1473641 Regression problems on closed Spiro https://bugs.launchpad.net/inkscape/+bug/1473641
Build failures still open:
Bug #1471587 trunk: build failure with llvm-gcc-4.2 https://bugs.launchpad.net/inkscape/+bug/1471587
Regards, V
2015-07-13 20:20 GMT+02:00 su_v <suv@...2204...>:
On 2015-07-04 19:32 (+0200), Krzysztof Kosiński wrote:
I recently made a commit that finally synchronizes the 2Geom version used in Inkscape and the "upstream" version in 2Geom's Launchpad repository. It compiles and appears to work correctly, but since there were several potentially breaking changes in 2Geom, it has a high potential for regressions.
Reported regressions:
Bug #1472518 Powerstroke LPE renders with spurious circles https://bugs.launchpad.net/inkscape/+bug/1472518 (has proposed patches pending review)
Fixed the spurious circles, but the result still looks wrong, needs some more work.
Bug #1473317 Power Stroke LPE crashes with rectangles https://bugs.launchpad.net/inkscape/+bug/1473317
Should be fixed.
Bug #1473641 Regression problems on closed Spiro https://bugs.launchpad.net/inkscape/+bug/1473641
I know how to fix this, will get to that soon.
Build failures still open:
Bug #1471587 trunk: build failure with llvm-gcc-4.2 https://bugs.launchpad.net/inkscape/+bug/1471587
Should be fixed as of r14248, please test - I don't have that compiler version readily available.
Regards, Krzysztof
On 2015-07-04 19:32 (+0200), Krzysztof Kosiński wrote:
I recently made a commit that finally synchronizes the 2Geom version used in Inkscape and the "upstream" version in 2Geom's Launchpad repository. It compiles and appears to work correctly, but since there were several potentially breaking changes in 2Geom, it has a high potential for regressions.
Reported regressions related to r14226 (first 2geom update):
Bug #1478168 Bend LPE crash using a complex path as paste path https://bugs.launchpad.net/inkscape/+bug/1478168
Bug #1479167 Rotational node-snapping to path fails with rev >= 14226 https://bugs.launchpad.net/inkscape/+bug/1479167
Reported regressions related to r14299 (second 2geom update):
Bug #1487424 Powerstroke LPE hangs https://bugs.launchpad.net/inkscape/+bug/1487424
Regards, V
On 2015-08-27 09:55 (+0200), su_v wrote:
On 2015-07-04 19:32 (+0200), Krzysztof Kosiński wrote:
I recently made a commit that finally synchronizes the 2Geom version used in Inkscape and the "upstream" version in 2Geom's Launchpad repository. It compiles and appears to work correctly, but since there were several potentially breaking changes in 2Geom, it has a high potential for regressions.
Reported regressions related to r14226 (first 2geom update):
Bug #1478168 Bend LPE crash using a complex path as paste path https://bugs.launchpad.net/inkscape/+bug/1478168
Partially fixed (report still open).
Bug #1479167 Rotational node-snapping to path fails with rev >= 14226 https://bugs.launchpad.net/inkscape/+bug/1479167
Fixed.
NEW: Bug #1492153 CDR import crashes after lib2geom update https://bugs.launchpad.net/inkscape/+bug/1492153
The summary of this one might be somewhat misleading - the crash is related to a sequence of path commands (two consecutive 'Z' commands) produced by libcdr >= 0.0.15 which causes Inkscape to crash after the 2geom update in rev 14226; see test case attached in comment 6: https://bugs.launchpad.net/inkscape/+bug/1492153/+attachment/4457420/+files/...
Reported regressions related to r14299 (second 2geom update):
Bug #1487424 Powerstroke LPE hangs https://bugs.launchpad.net/inkscape/+bug/1487424
Fixed.
Regards, V
participants (3)
-
Krzysztof Kosiński
-
su_v
-
Tavmjong Bah