Compute glyph path only once per glyph

zbream commented 3 years ago

Fixes #133

Proposed changes

Previously, we run glyphPath.round() multiple times for multiple codepoints. Since glyphPath is mutable, there's a chance for rounding and precision errors to accumulate, causing the computed path to vary for the same glyph. See #133 for details.

Now, we compute the glyph's path only once, using that for each unicode codepoint.

Code quality

I did not include an explicit test for this change. As a minor optimization that happens to fix the linked issue, it is a little difficult to test, IMO, without arbitrarily specific icons and configuration. I'd be happy to add this if the maintainers think it would be useful in the long run.

License

To get your contribution merged, you must check the following.

[x] I read the project license in the LICENSE file
[x] I agree with publishing under this project license

Join

[ ] I wish to join the core team
[ ] I agree that with great powers comes responsibilities
[ ] I'm a nice person

My NPM username:

nfroidure commented 3 years ago

@pioug @NaridaL any advice on it? Feels like it won't hurt but maybe you are more aware of the potential backward compatibily issues.

pioug commented 3 years ago

Sorry @NaridaL, I am bypassing your review so I can release this patch and move on to other things 😶 I am available for reverting the changes if anything 🙏

nfroidure / svgicons2svgfont