svg / svgo

⚙️ Node.js tool for optimizing SVG files
https://svgo.dev/
MIT License
20.87k stars 1.39k forks source link

Curve to arc optimization produces invalid results on multiple curves (regression) #1081

Open ChALkeR opened 5 years ago

ChALkeR commented 5 years ago

Input:

<svg viewBox="0 0 50 30">
  <rect width="50" height="30" fill="#000" />
  <g transform="translate(10, 10)">
    <path fill="#f00" d="m 0,0 c -0.556,0 -1.006,0.449 -1.006,1.005 0,0.361 0.188,0.677 0.472,0.855 0.037,-0.003 0.074,-0.005 0.112,-0.005 0.216,0 0.421,0.051 0.603,0.142 C 0.652,1.912 1.01,1.502 1.01,1.005 1.01,0.449 0.557,0 0,0" />
  </g>
  <g transform="translate(20, 10)">
    <path fill="#ff0" d="m 0,0 c -0.39,0.441 -0.961,0.718 -1.598,0.718 -0.126,0 -0.249,-0.011 -0.369,-0.032 -0.037,0.12 -0.057,0.248 -0.057,0.38 0,0.715 0.579,1.29 1.294,1.29 0.712,0 1.29,-0.575 1.29,-1.29 C 0.56,0.623 0.338,0.233 0,0" />
  </g>
  <g transform="translate(30, 10)">
    <path fill="#f0f" d="M 0,0 C -0.405,0 -0.731,0.33 -0.731,0.729 -0.731,1.134 -0.405,1.46 0,1.46 0.402,1.46 0.728,1.134 0.728,0.729 0.728,0.33 0.402,0 0,0" />
  </g>
  <g transform="matrix(2,0,0,2,40,10)">
    <path fill="#0ff" d="m 0,0 c 0,0 0,0 -0.071,0.001 c 1,1 1,1.592 -2.103,1.592 c -0.027,0 -0.054,-10e-4 -0.08,-0.002 c -0.089,0.25 -0.137,0.519 -0.137,0.799 0,0.544 0.182,1.045 0.487,1.446 0.112,-0.031 0.23,-0.048 0.352,-0.048 0.608,0 1.118,0.413 1.262,0.974 0.095,0.012 0.192,0.017 0.29,0.017 0.591,0 1.132,-0.213 1.549,-0.568 C 1.48,4.223 1.41,4.229 1.338,4.229 0.625,4.229 0.046,3.648 0.046,2.935 c 0,-0.713 0.579,-1.292 1.292,-1.292 0.427,0 0.807,0.209 1.043,0.531 C 2.272,0.955 1.249,0 0,0" />
  </g>
</svg>

Command:

svgo -i 0.svg --pretty --indent=1 -o 1.svg

Output:

<svg viewBox="0 0 50 30">
 <path d="M0 0h50v30H0z"/>
 <path fill="red" d="M10 10a1.006 1.006 0 1 0-.534 1.86 1.345 1.345 0 0 1 .715.137A1.007 1.007 0 0 0 10 10"/>
 <path fill="#ff0" d="M20 10a2.126 2.126 0 0 1-1.967.686 1.29 1.29 0 0 0 1.237 1.67A1.292 1.292 0 1 0 20 10"/>
 <path fill="#f0f" d="M30 10a.73.73 0 1 0 0 0"/>
 <path fill="#0ff" d="M40 10l-.142.002c2 2 2 3.184-4.206 3.184-.054 0-.108-.002-.16-.004a4.756 4.756 0 0 0-.274 1.598c0 1.088.364 2.09.974 2.892.224-.062.46-.096.704-.096 1.216 0 2.236.826 2.524 1.948.19.024.384.034.58.034a4.761 4.761 0 0 0 3.098-1.136 2.589 2.589 0 0 1-3.006-2.552 2.584 2.584 0 1 1 4.67-1.522A4.778 4.778 0 0 0 40 10"/>
</svg>

Input screenshot:

2019-02-24 6 55 58

Output screenshot:

2019-02-24 6 56 30

Each one of those four curves seems to break.

Version:

$ ./node_modules/.bin/svgo --version
1.1.1
ChALkeR commented 5 years ago

This is, in fact, a regression between v0.6.1 and v0.6.2/v0.6.3.

v0.6.1 produces correct results:

<svg viewBox="0 0 50 30">
    <path d="M0 0h50v30H0z"/>
    <path fill="red" d="M10 10a1.006 1.006 0 0 0-.534 1.86 1.345 1.345 0 0 1 .715.137A1.007 1.007 0 0 0 10 10"/>
    <path fill="#ff0" d="M20 10a2.126 2.126 0 0 1-1.967.686 1.29 1.29 0 0 0 1.237 1.67A1.292 1.292 0 0 0 20 10"/>
    <path fill="#f0f" d="M30 10a.73.73 0 1 0-.003 1.46A.73.73 0 0 0 30 10"/>
    <path fill="#0ff" d="M40 10l-.142.002c2 2 2 3.184-4.206 3.184-.054 0-.108-.002-.16-.004a4.756 4.756 0 0 0-.274 1.598c0 1.088.364 2.09.974 2.892.224-.062.46-.096.704-.096a2.6 2.6 0 0 1 2.524 1.948c.19.024.384.034.58.034a4.76 4.76 0 0 0 3.098-1.136 2.59 2.59 0 0 1-3.006-2.552 2.584 2.584 0 0 1 4.67-1.522A4.778 4.778 0 0 0 40 10"/>
</svg>

v0.6.2 (in https://github.com/svg/svgo/commit/1b0051ad1ca410f040b7bd6af7a6e5ec9c1370c1) breaks the third curve, producing the following result:

<svg viewBox="0 0 50 30">
    <path d="M0 0h50v30H0z"/>
    <path fill="red" d="M10 10a1.006 1.006 0 0 0-.534 1.86 1.345 1.345 0 0 1 .715.137A1.007 1.007 0 0 0 10 10"/>
    <path fill="#ff0" d="M20 10a2.126 2.126 0 0 1-1.967.686 1.29 1.29 0 0 0 1.237 1.67A1.292 1.292 0 0 0 20 10"/>
    <path fill="#f0f" d="M30 10a.73.73 0 1 0 0 0"/>
    <path fill="#0ff" d="M40 10l-.142.002c2 2 2 3.184-4.206 3.184-.054 0-.108-.002-.16-.004a4.756 4.756 0 0 0-.274 1.598c0 1.088.364 2.09.974 2.892.224-.062.46-.096.704-.096a2.6 2.6 0 0 1 2.524 1.948c.19.024.384.034.58.034a4.76 4.76 0 0 0 3.098-1.136 2.59 2.59 0 0 1-3.006-2.552 2.584 2.584 0 0 1 4.67-1.522A4.778 4.778 0 0 0 40 10"/>
</svg>

v0.6.3 (in https://github.com/svg/svgo/commit/0f599b670da77de81eeacacd71dc335f9bdab1a8) breaks the remaining 3 curves (swapping some 0s with 1s):

<svg viewBox="0 0 50 30">
    <path d="M0 0h50v30H0z"/>
    <path fill="red" d="M10 10a1.006 1.006 0 1 0-.534 1.86 1.345 1.345 0 0 1 .715.137A1.007 1.007 0 0 0 10 10"/>
    <path fill="#ff0" d="M20 10a2.126 2.126 0 0 1-1.967.686 1.29 1.29 0 0 0 1.237 1.67A1.292 1.292 0 1 0 20 10"/>
    <path fill="#f0f" d="M30 10a.73.73 0 1 0 0 0"/>
    <path fill="#0ff" d="M40 10l-.142.002c2 2 2 3.184-4.206 3.184-.054 0-.108-.002-.16-.004a4.756 4.756 0 0 0-.274 1.598c0 1.088.364 2.09.974 2.892.224-.062.46-.096.704-.096 1.216 0 2.236.826 2.524 1.948.19.024.384.034.58.034a4.761 4.761 0 0 0 3.098-1.136 2.589 2.589 0 0 1-3.006-2.552 2.584 2.584 0 1 1 4.67-1.522A4.778 4.778 0 0 0 40 10"/>
</svg>
ChALkeR commented 5 years ago

For M 0,0 C -0.405,0 -0.731,0.33 -0.731,0.729 -0.731,1.134 -0.405,1.46 0,1.46 0.402,1.46 0.728,1.134 0.728,0.729 0.728,0.33 0.402,0 0,0, the returned angles are:

[1.5666969669518132, 1.5752774941371457, 1.568056459652197, 1.5707955025384717, 1.5663198734556092]

Replacing those with Math.PI / 2, which is very close:

[1.5707963267948966, 1.5707963267948966, 1.5707963267948966, 1.5707963267948966, 1.5707963267948966]

Solves the issue for the third curve. Perhaps something sign-related or otherwise caused by rounding is going wrong?

Commenting out this line: https://github.com/svg/svgo/blob/ee065be5a88c85b976db601c13eb6e55650a7d68/plugins/convertPathData.js#L343 fixes the remaining 3 curves (and #1074).

I understand that this is not the real fix, but this is an indication of what exactly is going wrong.

himedlooff commented 5 years ago

Disabling if (angle > Math.PI) arc.data[3] = 1; fixes a similar issue I have as well. Hope that helps validate your theory.

ChALkeR commented 5 years ago

@himedlooff as a better workaround, just using --config='{"plugins":[{"convertPathData":{"makeArcs":false}}]} (or specifying equivalent option in the configuration file) would also fix the problem with less risks of introducing more potential issues. It would disable the curve-to-arc transform completely though.

himedlooff commented 5 years ago

Yeah it's definitely not a good way to go forward but it was the only way I could test to make sure that my issue was with svg and not something local. Also for some reason I couldn't get the makeArcs property work in my set up:

image

☝️ that does not fix the issue for me. 🤔