mathandy / svgpathtools

A collection of tools for manipulating and analyzing SVG Path objects and Bezier curves.
MIT License
557 stars 142 forks source link

Bugfix for rotated ellipse #221

Open kennyweiss opened 5 months ago

kennyweiss commented 5 months ago

Thanks for this great library!

I was playing with rotated ellipses and noticed a bug in how the rotation angle is extracted from the transform function. Essentially, arccos wasn't computing the right sign for some angles, so I replaced it with arctan2 and added a bunch of unit tests.

Reproducer:

Before this PR, the following input

<svg width="200" height="200" xmlns="http://www.w3.org/2000/svg" style="fill:green;stroke:black;stroke-width:1.5">
    <ellipse cx="40" cy="80" rx="15" ry="20" transform="rotate(-45 40 80)"/>
</svg>

image

generated these arc paths:

<?xml version="1.0" ?>
<svg:svg xmlns:svg="http://www.w3.org/2000/svg" width="200" height="200" style="fill:red;stroke:black;stroke-width:1.5">
    <path d="M 29.393398282201787,90.60660171779821 
             A 15.0,20.0 45.0 1,0 50.60660171779821,69.39339828220179 
             A 15.0,20.0 45.0 1,0 29.393398282201787,90.60660171779821"/>
</svg:svg>

Note: The angle is 45 degrees instead of -45 degrees, which is rendered as: image (note: I changed the fill color for easier comparison).

After this PR, the correct arc paths are produced:

<?xml version="1.0" ?>
<svg:svg xmlns:svg="http://www.w3.org/2000/svg" width="200" height="200" style="fill:red;stroke:black;stroke-width:1.5">
    <path d="M 29.393398282201787,90.60660171779821 
             A 15.0,20.0 -44.99999999999999 1,0 50.60660171779821,69.39339828220179 
             A 15.0,20.0 -44.99999999999999 1,0 29.393398282201787,90.60660171779821"/>
</svg:svg>

image

kennyweiss commented 5 months ago

Note: This bugfix seems related to the discussion in https://github.com/mathandy/svgpathtools/pull/98 (specifically, this comment from @Vrroom -- https://github.com/mathandy/svgpathtools/pull/98#issuecomment-841042615)