svgdotjs / svgdom

Straightforward DOM implementation to make SVG.js run headless on Node.js
MIT License
274 stars 54 forks source link

Arc.length(): Maximum call stack size exceeded #54

Closed martinleopold closed 4 years ago

martinleopold commented 4 years ago

Run in Node.js v12.15.0:

const { SVG, registerWindow } = require('@svgdotjs/svg.js');
const window = require('svgdom');
const document = window.document;
registerWindow(window , window.document);

const svg = SVG(document.documentElement);

let path = svg.path('M 11.938 22.013 A 0.125 0.125 0 1 1 11.791 21.913');

let len = path.length();

Last line produces the following error:

/Users/mlg/code/ldb-streamline-data/node_modules/svgdom/class/Point.js:5
  constructor (x, y) {
              ^

RangeError: Maximum call stack size exceeded
    at new Point (/Users/mlg/code/ldb-streamline-data/node_modules/svgdom/class/Point.js:5:15)
    at Point.clone (/Users/mlg/code/ldb-streamline-data/node_modules/svgdom/class/Point.js:24:12)
    at new Arc (/Users/mlg/code/ldb-streamline-data/node_modules/svgdom/utils/pathUtils.js:171:18)
    at Arc.splitAt (/Users/mlg/code/ldb-streamline-data/node_modules/svgdom/utils/pathUtils.js:275:7)
    at Arc.length (/Users/mlg/code/ldb-streamline-data/node_modules/svgdom/utils/pathUtils.js:258:20)
    at Arc.length (/Users/mlg/code/ldb-streamline-data/node_modules/svgdom/utils/pathUtils.js:266:19)
    at Arc.length (/Users/mlg/code/ldb-streamline-data/node_modules/svgdom/utils/pathUtils.js:266:19)
    at Arc.length (/Users/mlg/code/ldb-streamline-data/node_modules/svgdom/utils/pathUtils.js:266:19)
    at Arc.length (/Users/mlg/code/ldb-streamline-data/node_modules/svgdom/utils/pathUtils.js:266:19)
    at Arc.length (/Users/mlg/code/ldb-streamline-data/node_modules/svgdom/utils/pathUtils.js:266:19)

As far as I can see it's a valid arc command, though.

EDIT: "@svgdotjs/svg.js": "^3.0.16", "svgdom": "0.0.21"

EDIT 2: running the same calculation in the browser is sucessful and returns a length of 0.5878579616546631

<!DOCTYPE html>
<html lang="en">
<head>
  <meta charset="UTF-8">
  <meta name="viewport" content="width=device-width, initial-scale=1.0">
  <meta http-equiv="X-UA-Compatible" content="ie=edge">
  <title>Document</title>
</head>
<body>
  <script type="module">
    import { SVG } from './node_modules/@svgdotjs/svg.js/dist/svg.esm.js';

    let svg = SVG();

    let path = svg.path('M 11.938 22.013 A 0.125 0.125 0 1 1 11.791 21.913');

    let len = path.length();

    console.log(len);
  </script>
</body>
</html>
Fuzzyma commented 4 years ago

To calculate the length of an arc, its splitted again and again so that when you combine the start-end points by a line you come close to the real length. This is done up to an error delta. Seems like the delta is too low for your arc OR we have a bug so that the condition is never reached. Have to check that...

martinleopold commented 4 years ago

Thanks for looking into this! I'm working with ~10K SVG icons at the moment, converting them to series of points. The error happens with about 200 of them - every time with Arc.length. I've tried to up the delta successively, which makes the error go away for SOME icons but I'm getting to 0.1, 1 and even 10 and still getting errors with others. This makes me believe theres a bug in the length function. I'll post an arc command later that won't work even with much higher delta. That should be a better test case.

Fuzzyma commented 4 years ago

Yes, that would help. Thank you!

martinleopold commented 4 years ago

I found two more types of arcs that produce the error, and don't go away with a different delta value:

"M 18.75 23.25 A 0 0 0 0 1 18.75 23.25"
"M 15.372999999999998 10.5 V 10.5 A 0 0 0 0 1 15.373 10.5"

These are degenerate arcs (same start and end point, ie. a point, arc length 0) or in the second case, an almost degenerate arc, with very short length.

I wonder how the length calculation could be done differently, eg. by using an exact formula.

As a side note: I'm also getting (different) errors with cubic curves - they also seem to be degenerate ones with length 0.

EDIT: It looks like there is not 'exact' formula... https://math.stackexchange.com/questions/433094/how-to-determine-the-arc-length-of-ellipse

EDIT 2: Here is one possibility, worth considering (Use a fixed number of steps along the arc): https://github.com/rveciana/svg-path-properties/blob/master/src/arc.ts#L242

Fuzzyma commented 4 years ago

Jep I dont think there is any exact formular :D. Having only fixed points gives you different precision for different arcs. And I think thats a bad thing because you cannot count on the accuracy. I am pretty sure that solution would be faster though... I dont even know why this part of the code has this problems. I often had stack overflows there but the all turned out to be an error in the correct creation of the subarcs. However, I thought I finally fixed that and had a robust way to create arcs... well...

Fuzzyma commented 4 years ago

So I've finally resolved that issue. First of all degenerated cases were just not considered. A lot of NaN here and there. Extra ifs to get around that. The other case was trickier. It basically occured because I rounded a value on path creation. I am pretty sure I did that because other parts of the code didnt work (because they test for 0 and its 7.3333333E-123 or similar). I am not sure if these problems will pop up again. I think it was only an issue for bbox calculation and that part actually still has rounding applied. So hopefully that works out.