arc transform fixed

[Vrroom]

Hello, I found that the formula you were using to calculate the semi major and the semi minor axis of an arc after a transformation was wrong. Earlier, you were simply treating the radii as a vector and applying the transformation matrix directly to this vector. This is somewhat incorrect. (Because for example, the radii shouldn't change if the matrix is a rotation + translation matrix but in your case they do). I found the correct formula at here and have implemented it in this branch. I would appreciate any feedback and anything else required from my end to merge this with the master. Thanks for this awesome project!!!

[SebKuzminsky]

I tested this arcTransformFix branch and found that it still displays the problem in #111.

[mathandy]

Thanks @Vrroom @SebKuzminsky @matthijskooijman. @Vrroom, please address the concerns mentioned above.

[Vrroom]

Hello @matthijskooijman @SebKuzminsky @mathandy!

Sorry for such a late reply. Github's notification system really confuses me 😅. @matthijskooijman I commited your suggestion into this PR.

[matthijskooijman]

It's been a while since I looked at this, but re-reading my previous comment, I think you now ensured that incorrect arcs are no longer silently "corrected", but bail out instead. However, I think that elliptical arcs still do not produce correct results due to missing rotation, which was the other concern in my comment, which I think is still not addressed?

[Vrroom]

Can you share a graphic on which this issue arises? It would help me understand better what the issue is

[matthijskooijman]

Not easily, I'm no longer working with this code (and svg2mod which I worked on fixed things differently rather than switching to svgpathtools). But I think that it should be a matter of making an elliptical arc and rotating it.

[Vrroom]

Does this SVG work to illustrate your point @matthijskooijman

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<svg
   xmlns:dc="http://purl.org/dc/elements/1.1/"
   xmlns:cc="http://creativecommons.org/ns#"
   xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#"
   xmlns:svg="http://www.w3.org/2000/svg"
   xmlns="http://www.w3.org/2000/svg"
   id="svg8"
   version="1.1"
   viewBox="0 0 210 297"
   height="297mm"
   width="210mm">
  <defs
     id="defs2" />
  <metadata
     id="metadata5">
    <rdf:RDF>
      <cc:Work
         rdf:about="">
        <dc:format>image/svg+xml</dc:format>
        <dc:type
           rdf:resource="http://purl.org/dc/dcmitype/StillImage" />
        <dc:title></dc:title>
      </cc:Work>
    </rdf:RDF>
  </metadata>
  <g
     id="layer1">
    <g
       transform="rotate(-45,111.88095,134.60932)"
       id="g4750">
      <path
         style="opacity:1;fill:#00baff;fill-opacity:1;stroke:none;stroke-width:0.26499999;stroke-linecap:round;stroke-miterlimit:4;stroke-dasharray:none;stroke-dashoffset:0;stroke-opacity:1"
         id="path4747"
         d="m 182.94048,133.3363 a 71.059525,34.395832 0 0 1 -57.74432,33.78659 71.059525,34.395832 0 0 1 -79.384695,-21.12465 71.059525,34.395832 0 0 1 27.993926,-41.70331 71.059525,34.395832 0 0 1 89.875769,5.49583 l -51.80021,23.54554 z" />
    </g>
  </g>
</svg>

[matthijskooijman]

Looks like it should. Does svgpathtools crash on this? If not, then maybe I was mistaken or something was fixed in the mean time (you did merge the master branch).

[Vrroom]

It does seem to be working. I saved the above as drawing.svg.

>>> import svgpathtools as svg
>>> svg.Document('drawing.svg').paths()
[Path(Arc(start=(161.22746445634277+83.46248339448498j), radius=(71.05952500000001+34.395832j), rotation=0.0, large_arc=False, sweep=True, end=(144.28679111150643+148.18461054366054j)),
     Arc(start=(144.28679111150643+148.18461054366054j), radius=(71.05952500000001+34.395832j), rotation=0.0, large_arc=False, sweep=True, end=(73.2159516893882+189.38068343539396j)),
     Arc(start=(73.2159516893882+189.38068343539396j), radius=(71.05952500000001+34.395832j), rotation=0.0, large_arc=False, sweep=True, end=(63.52195329709785+140.0972952298348j)),
     Arc(start=(63.52195329709785+140.0972952298348j), radius=(71.05952500000001+34.395832j), rotation=0.0, large_arc=False, sweep=True, end=(130.95985768260203+80.43166816682755j)),
     Line(start=(130.95985768260203+80.43166816682755j), end=(110.9807889254139+133.70915892541385j)),
     Line(start=(110.9807889254139+133.70915892541385j), end=(161.22746445634277+83.46248339448498j)))]
>>> 

[matthijskooijman]

Hm, are you sure you're using the right version of the code? I just tried your branch with the above file:

$ python3 -c 'import svgpathtools as svg;svg.Document("drawing.svg").paths()'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/home/matthijs/docs/src/upstream/svgpathtools/svgpathtools/document.py", line 261, in paths
    return flattened_paths(self.tree.getroot(), group_filter,
  File "/home/matthijs/docs/src/upstream/svgpathtools/svgpathtools/document.py", line 143, in flattened_paths
    path = transform(parse_path(converter(path_elem)), path_tf)
  File "/home/matthijs/docs/src/upstream/svgpathtools/svgpathtools/path.py", line 304, in transform
    return transform_segments_together(curve, transformation)
  File "/home/matthijs/docs/src/upstream/svgpathtools/svgpathtools/path.py", line 187, in transform_segments_together
    transformed_segs = [transformation(seg) for seg in path]
  File "/home/matthijs/docs/src/upstream/svgpathtools/svgpathtools/path.py", line 187, in <listcomp>
    transformed_segs = [transformation(seg) for seg in path]
  File "/home/matthijs/docs/src/upstream/svgpathtools/svgpathtools/path.py", line 303, in <lambda>
    transformation = lambda seg: transform(seg, tf)
  File "/home/matthijs/docs/src/upstream/svgpathtools/svgpathtools/path.py", line 331, in transform
    return Arc(new_start, radius=new_radius, rotation=curve.rotation,
  File "/home/matthijs/docs/src/upstream/svgpathtools/svgpathtools/path.py", line 1431, in __init__
    self._parameterize()
  File "/home/matthijs/docs/src/upstream/svgpathtools/svgpathtools/path.py", line 1486, in _parameterize
    raise ValueError("No such elliptic arc exists.")
ValueError: No such elliptic arc exists.

[Vrroom]

I checked out the version d3961ea4d0 and re-ran the test and it gave me what I got earlier as well.

[Vrroom]

@matthijskooijman Do you want to hop on a quick call to see what is wrong?

[matthijskooijman]

Ok, I have the same commit checked out, so there must be some other difference. Interestingly, when I create a new virtualenv and run inside there, then it does work for me as well. Looking at dependencies, svgwrite is the same version inside and outside the venv, but numpy is newer. But downgrading numpy (from 1.20.3 to 1.17.4) inside the venv does not break it... Both use python 3.8.5.

With some prints I confirmed that the input of the path.transform() function is equal, but the radius it calculates is off (by factor of 2, 71-ish vs 34-ish). It looks like the numpy eigvals function returns different results. In both cases, the the D input variable is identical, but numpy produces different output. E.g. with this patch:

--- a/svgpathtools/path.py
+++ b/svgpathtools/path.py
@@ -319,11 +319,14 @@ def to_complex(v):
         D = invT.T @ Q @ invT

         eigvals = np.linalg.eigvals(D)
+        print(D)
+        print(eigvals)

         rx = 1 / np.sqrt(eigvals[0])
         ry = 1 / np.sqrt(eigvals[1])

         new_radius = complex(rx, ry)
+        print(new_radius)

         if new_radius.real == 0 or new_radius.imag == 0 :
             return Line(new_start, new_end)

I get this output:

$ python3 -c 'import svgpathtools as svg;print(svg.Document("drawing.svg").paths())'
[[0.00052165 0.00032361]
 [0.00032361 0.00052165]]
[0.00084526 0.00019804]
(34.395832+71.059525j)
(...more output and exception snipped...)

$ ./venv/bin/python3 -c 'import svgpathtools as svg;print(svg.Document("drawing.svg").paths())'
[[0.00052165 0.00032361]
 [0.00032361 0.00052165]]
[0.00019804 0.00084526]
(71.05952500000001+34.395832j)
(...more output snipped...)

Note that the D value is equal, but the resulting eigvalues is different. Also, the transform function runs a 4 times, of which the second one fails with an exception, but the values printed are the same all 4 times, so I'm showing only the first one. I also just noted that the eigenvalues, and thus also the radius values are reversed between the working and non-working cases.

But then, the plot thickens... Running just the eigenvalues calculation, shows no difference, and returns the broken (wrong?) value in both cases.

$ python3 -c 'import numpy; print(numpy.linalg.eigvals([[0.00052165,0.00032361],[0.00032361,0.00052165]]))'
[0.00084526 0.00019804]
$ venv/bin/python3 -c 'import numpy; print(numpy.linalg.eigvals([[0.00052165,0.00032361],[0.00032361,0.00052165]]))'
[0.00084526 0.00019804]

Maybe this is due to rounding (I've copied the printed values for D, which might have been rounded?).

I'm not sure what is going on here... Are you familiar enough with the math to check that 1) the value for D shown here is correct and 2) what the correct eigenvalue and radius should be? The input to the transform function that generates the above output is Arc(start=(144.28679111150643+148.18461054366054j), radius=(71.05952500000001+34.395832j), rotation=0.0, large_arc=False, sweep=True, end=(73.21595168938822+189.38068343539396j)).

@matthijskooijman Do you want to hop on a quick call to see what is wrong?

I really should be doing other things, this problem is something that was relevant to me a while back but not really anymore right now. However, since I'm the only one that can reproduce this problem, I'll try to produce enough info to at least figure this out. Let's see if you can find anything from my above comments, if you then think it is useful to do some more interactive debugging in a call, I'm open to that.

[Vrroom]

@matthijskooijman First of all I'm sorry that I didn't read your first comment properly. It pointed out a serious bug with rotation. I only realized this after re-reading my code. Regarding your concern about the D matrix and the eigenvalues, you might have noticed that in both cases, the eigenvalues are the same, just that their order is reversed. In one case you got [0.00084526 0.00019804] and in the other, [0.00019804 0.00084526]. I checked the numpy documentation and it confirms that:

The eigenvalues, each repeated according to its multiplicity. The eigenvalues are not necessarily ordered.

So how is that relevant to us? After a transformation, we are free to decide which is rx and ry among the two eigenvalues. But once we fix them, we have to pick the eigenvector corresponding to the eigenvalue chosen for rx to calculate curve rotation.

In my latest commit e5520b7e0, I maintain this consistency by picking the first eigenvalue for rx and then using the first eigenvector to calculate the rotation of the x-axis. This is then added to curve.rotation.

        rx = 1 / np.sqrt(eigvals[0])
        ry = 1 / np.sqrt(eigvals[1])

        new_radius = complex(rx, ry)

        xeigvec = eigvecs[:, 0]
        rot = np.degrees(np.arccos(xeigvec[0]))

I used the following test file to try out the new code and the path in output.svg matched that in drawing.svg.

import svgpathtools as svg

doc = svg.Document('./drawing.svg')
vbox = doc.tree.getroot().attrib['viewBox']
paths = doc.paths()
svg.wsvg(paths, stroke_widths=[1], viewbox=vbox, filename='./output.svg')

Thanks for this discussion. Please try it out now and see if any new bugs crop up!

[matthijskooijman]

Sounds like a good analysis and fix, even though I do not follow the math completely. I can confirm that the code no longer raises an exception, so that's good. I don't have any readily available files or setup to test with, though.

[Vrroom]

@mathandy @matthijskooijman Do you need me to add automated tests or something? Can we go ahead and merge this?

[matthijskooijman]

I'm just a passer-by that happened to run in a similar issue, so I'm leaving that question for @mathandy.

[mathandy]

So when I first wrote the code to handle arcs I realized there are some discrepancies between how the math would suggest arcs would work, how the SVG docs on W3 suggested they should work, and how they actually rendered in standard browsers (e.g. Chrome).

Can you give me a visual example of what this code fixes?

BTW I tend to think of Chrome as the standard to go by, but really it depends on your application and I'm open to arguments to thinking otherwise.

[Vrroom]

Consider the test function I provided earlier. Copying here for convenience:

import svgpathtools as svg

doc = svg.Document('./drawing.svg')
vbox = doc.tree.getroot().attrib['viewBox']
paths = doc.paths()
svg.wsvg(paths, stroke_widths=[1], viewbox=vbox, filename='./output.svg')

drawing.svg

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<svg
   xmlns:dc="http://purl.org/dc/elements/1.1/"
   xmlns:cc="http://creativecommons.org/ns#"
   xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#"
   xmlns:svg="http://www.w3.org/2000/svg"
   xmlns="http://www.w3.org/2000/svg"
   id="svg8"
   version="1.1"
   viewBox="0 0 210 297"
   height="297mm"
   width="210mm">
  <g transform="rotate(-45,111.88095,134.60932)" fill="none" stroke="black">
    <path
       d="m 182.94048,133.3363 a 71.059525,34.395832 0 0 1 -57.74432,33.78659 71.059525,34.395832 0 0 1 -79.384695,-21.12465 71.059525,34.395832 0 0 1 27.993926,-41.70331 71.059525,34.395832 0 0 1 89.875769,5.49583" />
  </g>
</svg>

This graphic looks like:

The input drawing.svg and the output output.svg, produced by my test code, should have identical geometries if the transformation handling for arcs is correct. Without this fix, this is not the case. The output with the current master is:

With my contribution, it becomes:

[Vrroom]

@mathandy Are we going to go forward with this?

[mathandy]

Thanks @Vrroom !