Skip to content

Commit

Permalink
Fix error computing cubic Bézier curves bounding boxes (#95)
Browse files Browse the repository at this point in the history
* Fix error computing bounding boxes with cubic Bézier commands

* Refactor

* Bump version

* Minor change

* Refactor in tests

* Minor change
  • Loading branch information
mondeja authored May 12, 2022
1 parent dfde704 commit 3980277
Show file tree
Hide file tree
Showing 9 changed files with 50 additions and 49 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ deps.txt
emsdk
*.wasm
*.tgz
/*.mjs
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# CHANGELOG

## [1.2.1] - 2022-05-12

- Fixed error computing cubic Bézier curves bounding boxes.

## [1.2.0] - 2022-05-07

- Use default export for better interoperability.
Expand Down
6 changes: 3 additions & 3 deletions dist/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@ function minmaxQ(A) {
}
// https://github.com/kpym/SVGPathy/blob/acd1a50c626b36d81969f6e98e8602e128ba4302/lib/box.js#L127
function minmaxC(A) {
// if the polynomial is (almost) quadratic and not cubic
var K = A[0] - 3 * A[1] + 3 * A[2] - A[3];
if (K === 0 && A[0] === A[1] && A[0] === A[3]) {
if (A[0] === A[1] && A[0] === A[3]) {
// no curve, point targeting same location
return [A[0], A[3]];
}
// if the polynomial is (almost) quadratic and not cubic
var K = A[0] - 3 * A[1] + 3 * A[2] - A[3];
if (Math.abs(K) < CBEZIER_MINMAX_EPSILON) {
return minmaxQ([
A[0],
Expand Down
6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "svg-path-bbox",
"version": "1.2.0",
"version": "1.2.1",
"description": "Compute bounding boxes of SVG paths.",
"keywords": [
"svg",
Expand All @@ -9,7 +9,7 @@
],
"main": "dist/wrapper.js",
"browser": "dist/wrapper.js",
"module": "dist/index.js",
"module": "dist/wrapper.js",
"types": "dist/index.d.ts",
"type": "commonjs",
"bin": {
Expand All @@ -25,7 +25,7 @@
"example:cjs": "node examples/common.js",
"example:esm": "node examples/esm.mjs",
"example:ts": "ts-node examples/typescript.ts",
"lint": "eslint src/index.ts tests scripts examples/typescript.ts",
"lint": "eslint --max-warnings 0 src/index.ts tests scripts examples/typescript.ts",
"lint:fix": "npm run lint -- --fix",
"dist:prepare": "run-s dist:clean dist:create",
"dist:create": "mkdir dist",
Expand Down
5 changes: 2 additions & 3 deletions scripts/benchmark.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,14 @@ import * as svgPathBoundingBox from "svg-path-bounding-box";
import type { BBox } from "../src";
import type { BoundingBoxView } from "svg-path-bounding-box";

type Library = "svg-path-bbox" | "svg-path-bounding-box";
type LibraryAdapter = {
func: typeof svgPathBbox | typeof svgPathBoundingBox;
resultParser?: (result: BoundingBoxView) => BBox;
};

/* eslint-disable-next-line @typescript-eslint/no-explicit-any */
const LIBRARIES: {
[key in Library]: LibraryAdapter;
[key: string]: LibraryAdapter;
} = {
"svg-path-bbox": {
func: svgPathBbox,
Expand Down Expand Up @@ -70,7 +69,7 @@ const runLibrariesBenchmark = (

const resultParser = LIBRARIES[library].resultParser;
if (resultParser !== undefined) {
result = resultParser(result);
result = resultParser(result as BoundingBoxView);
}
if (result) {
console.log(" + result:", result);
Expand Down
8 changes: 4 additions & 4 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,13 @@ function minmaxQ(A: [number, number, number]): minMax {

// https://github.com/kpym/SVGPathy/blob/acd1a50c626b36d81969f6e98e8602e128ba4302/lib/box.js#L127
function minmaxC(A: [number, number, number, number]): minMax {
// if the polynomial is (almost) quadratic and not cubic
const K = A[0] - 3 * A[1] + 3 * A[2] - A[3];

if (K === 0 && A[0] === A[1] && A[0] === A[3]) {
if (A[0] === A[1] && A[0] === A[3]) {
// no curve, point targeting same location
return [A[0], A[3]];
}

// if the polynomial is (almost) quadratic and not cubic
const K = A[0] - 3 * A[1] + 3 * A[2] - A[3];
if (Math.abs(K) < CBEZIER_MINMAX_EPSILON) {
return minmaxQ([
A[0],
Expand Down
47 changes: 12 additions & 35 deletions tests/browser.test.ts
Original file line number Diff line number Diff line change
@@ -1,49 +1,26 @@
import { jest } from "@jest/globals";
import { linealCases, pathologicalCases } from "./cases/bbox";
import cases from "./cases/bbox";
import type { BBox } from "../src";

jest.retryTimes(3);
jest.setTimeout(2000);

const rounding = 5;

const round = (num: number): number => {
const result = parseFloat(num.toFixed(rounding));
return result == 0 ? 0 : result;
};

describe("Consistency with browser's element.getBBox()", () => {
beforeAll(async () => {
await page.goto("http://localhost:8080");
});

const cases = [...linealCases, ...pathologicalCases];

test.each(cases)(
"browserSvgPathBbox(%p) ⇢ %p",
async (d: string, libBbox: BBox) => {
const browserBbox = await page.evaluate(
(d, rounding) => {
const round = (num: number): number =>
parseFloat(num.toFixed(rounding));
const browserBbox = await page.evaluate((d) => {
document.body.innerHTML = `<svg width="800" height="800"><path d="${d}"/></svg>`;
const bbox = (
document.querySelector("path") as SVGPathElement
).getBBox();
return [bbox.x, bbox.y, bbox.x + bbox.width, bbox.y + bbox.height];
}, d);

document.body.innerHTML =
`<svg width="800" height='800'>` +
`<path fill='black' d="${d}"/></svg>`;
const bbox = (
document.querySelector("path") as SVGPathElement
).getBBox();
return [
round(bbox.x),
round(bbox.y),
round(bbox.x + bbox.width),
round(bbox.y + bbox.height),
];
},
d,
rounding
);
expect(browserBbox).toEqual(libBbox.map((num) => round(num)));
expect(browserBbox[0]).toBeCloseTo(libBbox[0], 3);
expect(browserBbox[1]).toBeCloseTo(libBbox[1], 3);
expect(browserBbox[2]).toBeCloseTo(libBbox[2], 3);
expect(browserBbox[3]).toBeCloseTo(libBbox[3], 3);
}
);
});
11 changes: 10 additions & 1 deletion tests/cases/bbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,15 @@ export const pathologicalCases: CasesTuple = [
[
"M 21.1456 21.1456 C 21.1456 21.1456 21.1456 21.1456 200.0000 0.0000 200.0000 0.0000 200.0000 0.0000 221.1456 221.1456 221.1456 221.1456 221.1456 221.1456 21.1456 221.1456 21.1456 221.1456 21.1456 221.1456 21.1456 21.1456",
[21.1456, 0, 221.1456, 221.1456],
// svg-path-bounding-box: [21.1456, 0, 221.1456, 221.1456],
// svg-path-bounding-box: [ 21.1456, 0, 221.1456, 221.1456 ],
],

// https://github.com/mondeja/svg-path-bbox/issues/91
[
"M436.79 417.31C435.89 415.01 438.59 410.91 439.69 409.31C439.89 408.91 439.69 408.41 439.39 408.31C438.09 407.41 435.19 404.71 435.09 401.61C434.99 399.71 437.39 396.41 438.49 394.91C439.19 393.71 437.69 393.11 437.09 392.31C435.69 390.81 433.99 388.31 434.09 385.71C434.19 382.71 436.69 379.51 440.79 377.01C440.89 376.91 440.99 376.71 440.89 376.61C439.79 374.91 439.29 374.01 439.19 371.41C438.99 367.91 440.79 364.81 444.19 362.41C446.59 360.81 449.89 359.81 452.79 359.21C452.79 359.01 452.79 349.41 452.79 348.21V348.01V340.21C452.79 340.01 452.69 339.91 452.49 339.91L209.89 340.21C209.69 340.21 209.59 340.31 209.59 340.51L209.69 896.1H452.49C452.69 896.1 452.79 896 452.79 895.8V420.31C449.79 421.41 446.59 422.11 444.19 422.11C440.79 422.01 437.69 419.51 436.79 417.31Z",
[209.59000000000003, 339.90999999999764, 452.7900000000486, 896.1],
// svg-path-bounding-box: [ 209.59, 339.91, 452.79, 896.1 ]
],
];

export default [...linealCases, ...pathologicalCases];
11 changes: 11 additions & 0 deletions tests/changelog.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import * as fs from "node:fs";

test("Latest version has a CHANGELOG entry", () => {
const changelog = fs.readFileSync("CHANGELOG.md", "utf8");
const packageJson = JSON.parse(fs.readFileSync("package.json", "utf8"));

const changelogEntry = changelog
.split("\n")
.find((line: string) => line.startsWith(`## [${packageJson.version}]`));
expect(changelogEntry).toBeDefined();
});

0 comments on commit 3980277

Please sign in to comment.