Skip to content

Rounding issues / bugfix #797

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions src/path.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,9 @@ function roundDecimal(float, places) {
const roundedDecimalPart = decimalRoundingCache[places][decimalPart];
return integerPart + roundedDecimalPart;
}

const roundedDecimalPart = +(Math.round(decimalPart + 'e+' + places) + 'e-' + places);

const roundedDecimalPart = Number((Math.round(decimalPart * Math.pow(10, places)) / Math.pow(10, places)).toFixed(places));

decimalRoundingCache[places][decimalPart] = roundedDecimalPart;

return integerPart + roundedDecimalPart;
Expand All @@ -52,7 +53,7 @@ function optimizeCommands(commands) {
const previousCommand = subpath[subpath.length - 1];
const nextCommand = commands[i + 1];
subpath.push(cmd);

if (cmd.type === 'M') {
startX = cmd.x;
startY = cmd.y;
Expand Down Expand Up @@ -713,7 +714,7 @@ Path.prototype.toDOMElement = function(options, pathData) {
newPath.setAttribute('fill', this.fill);
}
}

if (this.stroke) {
newPath.setAttribute('stroke', this.stroke);
newPath.setAttribute('stroke-width', this.strokeWidth);
Expand Down
53 changes: 38 additions & 15 deletions test/path.spec.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ describe('path.mjs', function() {
let testPath1;
let testPath2;


beforeEach(function() {
global.document = {
createElementNS: function (namespace, tagName) {
Expand All @@ -23,14 +23,14 @@ describe('path.mjs', function() {
};

emptyPath = new Path();

testPath1 = new Path();
testPath1.moveTo(1, 2);
testPath1.lineTo(3, 4);
testPath1.curveTo(5, 6, 7, 8, 9, 10);
testPath1.quadTo(11, 12, 13, 14, 15, 16);
testPath1.close();

testPath2 = new Path(); // two squares
testPath2.moveTo(0, 50);
testPath2.lineTo(0, 250);
Expand All @@ -51,7 +51,7 @@ describe('path.mjs', function() {
testPath2.lineTo(250, 50);
testPath2.close();
});

it('should set path commands correctly', function() {
const expectedCommands = [
{ type: 'M', x: 1, y: 2 },
Expand All @@ -64,14 +64,14 @@ describe('path.mjs', function() {
assert.deepEqual(testPath1.commands, expectedCommands);
assert.deepEqual(Path.fromSVG(svg, {flipY: false}).commands, expectedCommands);
});

it('should return a streamlined SVG path (no commas, no additional spaces, only absolute commands)', function() {
const input = 'M1,2 L 3 4Z M .5 6.7 L 8 9 l 2,1 m1 1 c 1 2,3 4 5, 6q-7.8-9.0 -1.011 12 m-13.99-28 h 13 15 V 17 19 v21 23 25 H27 V28 zzzZZzzz';
const expectedSVG = 'M1 2L3 4ZM0.50 6.70L8 9L10 10M11 11C12 13 14 15 16 17Q8.20 8 14.99 29M1 1L14 1L29 1L29 17L29 19L29 40L29 63L29 88L27 88L27 28Z';
const path = Path.fromSVG(input, {flipY: false});
assert.deepEqual(path.toPathData({flipY: false}), expectedSVG);
});

it('should accept integer or correct fallback for decimalPlaces backwards compatibility', function() {
const expectedResult = 'M0.58 0.75L1.76-1.25';
const expectedResult2 = 'M0.575 0.750L1.757-1.254';
Expand All @@ -82,18 +82,18 @@ describe('path.mjs', function() {
assert.equal(path.toPathData({optimize: true, flipY: false}), expectedResult);
assert.equal(path.toPathData(3), expectedResult2);
});

it('should not optimize SVG paths if parameter is set falsy', function() {
const unoptimizedResult = 'M0 50L0 250L50 250L100 250L150 250L200 250L200 50L0 50ZM250 50L250 250L300 250L350 250L400 250L450 250L450 50L250 50Z';
assert.equal(testPath2.toPathData({optimize: false, flipY: false}), unoptimizedResult);
});

it('should optimize SVG paths if path closing point matches starting point', function() {
const optimizedResult = 'M0 250L50 250L100 250L150 250L200 250L200 50L0 50ZM250 250L300 250L350 250L400 250L450 250L450 50L250 50Z';
assert.equal(testPath2.toPathData({flipY: false}), optimizedResult);
assert.equal(testPath2.toPathData({optimize: true, flipY: false}), optimizedResult);
});

it('should optimize SVG paths if they include unnecessary lineTo commands', function() {
const path = (new Path()).fromSVG(
'M199 97 L 199 97 L 313 97 L 313 97 Q 396 97 444 61 L 444 61 L 444 61 Q 493 25 493 -36 L 493 -36 L 493 -36' +
Expand All @@ -104,7 +104,7 @@ describe('path.mjs', function() {
assert.equal(path.toSVG({optimize: true}), expectedResult);
assert.equal(path.toDOMElement({optimize: true}).getAttribute('d'), expectedPath);
});

it('should calculate flipY from bounding box if set to true', function() {
const jNormal = 'M25 772C130 725 185 680 185 528L185 33L93 33L93 534C93 647 60 673-9 705ZM204-150' +
'C204-185 177-212 139-212C101-212 75-185 75-150C75-114 101-87 139-87C177-87 204-114 204-150Z';
Expand All @@ -114,14 +114,14 @@ describe('path.mjs', function() {
assert.equal(path.toPathData({flipY: false}), jUpsideDown);
assert.equal(path.toPathData(), jNormal);
});

it('should handle scaling and offset', function() {
const inputPath = 'M0 1L2 0L3 0L5 1L5 5L0 5Z';
const expectedPath = 'M1 4.50L6 2L8.50 2L13.50 4.50L13.50 14.50L1 14.50Z';
const path = Path.fromSVG(inputPath, { x: 1, y: 2, scale: 2.5 });
assert.equal(path.toPathData(), expectedPath);
});

it('should apply fill and stroke for toSVG()', function() {
assert.equal(emptyPath.toSVG(), '<path d=""/>');
emptyPath.fill = '#ffaa00';
Expand All @@ -135,13 +135,13 @@ describe('path.mjs', function() {
emptyPath.fill = 'black';
assert.equal(emptyPath.toSVG(), '<path d="" stroke="#0000ff" stroke-width="2"/>');
});

it('should apply fill and stroke for toDOMElement()', function() {
// in browser context these wouldn't be undefined, but we're only mocking it
assert.equal(emptyPath.toDOMElement().getAttribute('fill'), undefined);
assert.equal(emptyPath.toDOMElement().getAttribute('stroke'), undefined);
assert.equal(emptyPath.toDOMElement().getAttribute('stroke-width'), undefined);

emptyPath.fill = '#ffaa00';
assert.equal(emptyPath.toDOMElement().getAttribute('fill'), '#ffaa00');
emptyPath.stroke = '#0000ff';
Expand All @@ -154,7 +154,30 @@ describe('path.mjs', function() {
emptyPath.fill = 'black';
assert.equal(emptyPath.toDOMElement().getAttribute('fill'), undefined);
});


it('should not return NaNs in paths', function() {
let expectedResult = 'M0 349';
let path = new Path();
path.moveTo(0, 349.00000000000006);
assert.equal(path.toPathData({decimalPlaces: 2}), expectedResult);

expectedResult = 'M0 349.01';
path = new Path();
path.moveTo(0, 349.0060000000000);
assert.equal(path.toPathData({decimalPlaces: 2}), expectedResult);

expectedResult = 'M0 349.06';
path = new Path();
path.moveTo(0, 349.060000000000);
assert.equal(path.toPathData({decimalPlaces: 2}), expectedResult);

expectedResult = 'M0 349.00000000000006';
path = new Path();
path.moveTo(0, 349.00000000000006);
assert.equal(path.toPathData({decimalPlaces: 14}), expectedResult);

});

afterEach(() => {
delete global.document;
});
Expand Down