Performance issue

[LitoMore]

This library is relatively slow compared to svgpath and svg-path-bbox.

You can use these SVG images below to test the performance:

https://cdn.simpleicons.org/lerna
https://cdn.simpleicons.org/pubg

[thednp]

In what exactly?

[LitoMore]

All of them, like scale, translate, and getPathBBox.

[thednp]

Yea transforms are made by a DOMMatrix shim to allow 3D projections, where svgpath uses a 2D only model, if we were to use that, the performance would be within margin of error.

But as for the pathBBox that totally depends on many things, certainly something to look at in the future. Reminding myself what's what, I implemented a unified segment factory for each path command and that's not the most efficient way to find the pathBBox.

Thanks again for bringing these up.

[thednp]

I'm now working on a performance improvement for getPathBBox to not include pathLengthFactory which basically goes through every sub-pixel possible to determine segment and/or path length. I'm going to implement a similar solution as you suggested.

As for the transform situation it won't change, however with your use case where you combine transform and bbox the performance will be significantly improved.

Should I let you know when I have the new getPathBBox ready for a test?

[thednp]

Yo @LitoMore I've used your HUGE svg file, added some fixes and I've compiled a set of benchmarks, have a look:

I've analyzed the performance issues and made some improvements thanks to you, so I'm giving you a few notes:

• there are 11650 segments in that SVG you linked, that's a lot! 😄
• as you can see the getPathBBox is way faster as well as the initialization
• there was a problem with pathLengthFactory it was always looking for a point at length, even if no distance parameter was given, so that's nicely patched now;
• I've added a sampleSize parameter to all pathLengthFactory related methods, the default is 300 (unchanged), but you can work with 50-100 and still get decent results for another 2-3x performance;
• when SVGPathCommander initializes it will also calculate a shape width and height (via getPathBBox(sampleSize = 20)) to determine a transform origin for the path (an operation that costs about 60ms with your path); subsequent calls of the instance.transform will benefit massively from having an origin already processed and will improve performance further;
• even if we're using a DOMMatrix (with 3D projections), the performance of the transform is still really good don't you think?
• I've updated the wiki with the new changes, hope you can have a look!
• I've also analyzed the tool you linked me (svgpath + svg-path-bbox) and I'll try and implement a similar logic, our pathLengthFactory was doing 3 things at once: calculating length, finding a point at length and finding a min and a max
  for each segment, will have a look at that tomorrow 👍

Have a good one!

[thednp]

Yo @LitoMore, disregard my previous message, watch this:

I've implemented a series of changes that drastically improve both performance and accuracy of A, C and Q segments parsing/processing.

[thednp]

@LitoMore
There could be more performance gains to be achieved by removing the path check for each conversion tool (for instance pathToAbsolute uses isAbsolutePath which really iterates through all segments) but that will be detrimental to the type safety and I don't think this is what we want.

Yes, performance would be on par with SvgPath but is that worth it?

[LitoMore]

You could add a new option to the constructor. Like new SVGPathCommander('M0..', {fastMode: true}) to enable safe mode by default. Or use new SVGPathCommander('M0..', {safeMode: true}) to make the safe mode disabled by default.

The time it takes to process the image is so valuable that I don't mind if it fails because the browser won't throw an exception for a failed SVG image. The situations where I need to constantly check it are so rare that it's not worth it to keep running the check all the time.

[thednp]

Thanks for the suggestion, I'll come back to it as soon as I can.

[thednp]

The type safety is designed to eliminate the possibility of a crash, not just to show the dev some code marked with red in the editor. It might be a case of over checking, but for instance you want to transform a PathArray that is not normalized, something like a RelativeArray, the instance will not return an incomplete or invalid path string, but it will crash because it won't have enough data.

I'm thinking of a way to make the DX better as well, not just improve performance. I'm thinking of two main changes:

• The SVGPathCommander class should have some additional properties like isAbsolute, isRelative, isNormalized
• conversion methods should always receive compatible input values; if however the dev's code crashes, he has the readily available methods (isAbsolutePath, isRelativePath, etc) to perform checks themselves or create a custom logic.

Let's say we do the parsing and preparation faster, but on that 11650 segments path, transform will still take some ms, as explained above, but I think we can give this a try.

[thednp]

Hey @LitoMore how do you like this? (Haven't committed yet)

Transform is still slower, but it does so much more it doesn't matter really for just a couple of milliseconds IMO.

What do you think? Would you like to play?

[LitoMore]

OK, I will try it out tomorrow.

[LitoMore]

I just played the code on master branch and the 2.1.0.

Here is my playground:

{
  "type": "module",
  "description": "",
  "dependencies": {
    "svg-path-commander-master": "file:///Users/litomore/Git/svg-path-commander",
    "simple-icons": "13.14.0",
    "svg-path-commander": "2.1.0"
  }
}

import * as si from "simple-icons";
import SVGPathCommander from "svg-path-commander";
import Commander from "svg-path-commander-master";

const transform = {
  translate: 15, // X axis translation
  rotate: 15, // Z axis rotation
  scale: 0.75, // uniform scale on X, Y, Z axis
  skew: 15, // skew 15deg on the X axis
  origin: [15, 0], // if not specified, it will use the default origin value [0, 0]
};

const run = (CMD) => {
  const start = performance.now();

  for (const icon of Object.values(si)) {
    try {
      const cmd = new CMD(icon.path);
      const bbox = cmd.getBBox();
      cmd.transform(transform).toString();
    } catch (error) {
      console.log(icon.title, error.toString());
    }
  }

  const end = performance.now();
  console.log(end - start, "ms");
};

console.log("[email protected]");
run(SVGPathCommander);

console.log();

console.log("svg-path-commander@master");
run(Commander);

It returns:

[email protected]
boulanger TypeError: SVGPathCommander Error: Invalid path value "C" is not a MoveTo path command at index 192
Cocos TypeError: SVGPathCommander Error: Invalid path value "c" is not a MoveTo path command at index 15
Create React App TypeError: SVGPathCommander Error: Invalid path value "a" is not a MoveTo path command at index 5176
Google News TypeError: SVGPathCommander Error: Invalid path value "a" is not a MoveTo path command at index 1016
Renren TypeError: SVGPathCommander Error: Invalid path value "c" is not a MoveTo path command at index 358
Sat.1 TypeError: SVGPathCommander Error: Invalid path value "l" is not a MoveTo path command at index 691
1763.852834 ms

svg-path-commander@master
boulanger TypeError: SVGPathCommander Error: Invalid path value "C" is not a MoveTo path command at index 192
Cocos TypeError: SVGPathCommander Error: Invalid path value "c" is not a MoveTo path command at index 15
Create React App TypeError: SVGPathCommander Error: Invalid path value "a" is not a MoveTo path command at index 5176
Google News TypeError: SVGPathCommander Error: Invalid path value "a" is not a MoveTo path command at index 1016
Renren TypeError: SVGPathCommander Error: Invalid path value "c" is not a MoveTo path command at index 358
Sat.1 TypeError: SVGPathCommander Error: Invalid path value "l" is not a MoveTo path command at index 691
814.9035000000001 ms

Let's ignore those path errors. Whatever, it's actually much faster now.

[thednp]

@LitoMore
Get the latest v2.1.1 and try it out. You need to fix those path strings manually. I'm curious same operation how long it takes with SVGPath.

[LitoMore]

The v2.1.1 work as good as the master build.

BTW, I used the SVG Path Visualizer (boulanger) to inspect the path values. I'm not a path pro. But the value looks fine.

[thednp]

Thank you for your contribution. Yes the values are correct, I checked with the older 2.0.x version and a 100k resolution (every segment length calculated at 100.000 sample size).

In the next couple of days I'm going to work on other utils to improve performance a bit further, and just to give you a hint on what changed:

• I tried to keep things simple and not add additional parameter complexity;
• I've used a similar stacking mechanism as SvgPath in most important utils, so that segment conversion, calculation and all that's specific to the task run in a single array iteration while skipping task compatible segments;
• path factory tools have been broken into smaller and more dedicated tools to execute as fast as possible for each task;
• I've fixed some old issues while reworking the code, which is also a great outcome.

Let me know if you encounter more issues, I will provide an update on the progress.

[thednp]

@LitoMore
I have a small update to report, I found that SvgPath doesn't actually take 0ms to do transform, for instance svgPathInstance.scale(0.8) doesn't do anything unless you invoke .__evaluateStack() or toString(), it actually takes 10-15ms and our SVGPathCommander instance takes 35-45ms, we know where additional time is spent on, so we're going to town.

I've optimized other tools like optimizePath, getPointAtLength, roundPath and currently working on reversePath, we're to expect a 2-4X performance boost in all these.

Ttyl

[thednp]

Yo @LitoMore I'm going to commit a new version as soon as i finish some tooling updates.

Basically everything has been reworked / optimized for faster execution. I can't strip any more code just to get the bbox faster without breaking other tools. There's more to the performance because of Arc segments, if your shape didn't have Arc segments or were converted to CubicBezier, the string would get a little bigger, but the processing would be faster overall.

I think this really good now. What do you think?

UPDATE: v2.1.2 is out there now.

[thednp]

Hey @LitoMore, please check v2.1.3

Now both transform and bbox under 30ms. Please note the Date().getTime() is kinda wacky.