Skip to content

Minor optimiztions to reduce regression test runtime #2135

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.

Sign up for GitHub

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 4 commits into
base: main
Choose a base branch
from
Open

Minor optimiztions to reduce regression test runtime #2135

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e9e0778
refactor(test-optimization): Reduced the regression test runtime by s…
Lorfdail May 17, 2025
809c589
refactor(optimization): Define and initalize the hasScripts event att…
Lorfdail May 17, 2025
a224a48
refactor(optimization): Prevent unecessary branch checks once a comma…
Lorfdail May 17, 2025
b0d1d01
refactor(optimization): Only compute element style when simple checks…
Lorfdail May 17, 2025
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
18 changes: 9 additions & 9 deletions lib/svgo/tools.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,14 @@ export const removeLeadingZero = (value) => {
return strValue;
};

const hasScriptsEventAttrs = [
...attrsGroups.animationEvent,
...attrsGroups.documentEvent,
...attrsGroups.documentElementEvent,
...attrsGroups.globalEvent,
...attrsGroups.graphicalEvent,
];

/**
* If the current node contains any scripts. This does not check parents or
* children of the node, only the properties and attributes of the node itself.
Expand All @@ -170,15 +178,7 @@ export const hasScripts = (node) => {
}
}

const eventAttrs = [
...attrsGroups.animationEvent,
...attrsGroups.documentEvent,
...attrsGroups.documentElementEvent,
...attrsGroups.globalEvent,
...attrsGroups.graphicalEvent,
];

return eventAttrs.some((attr) => node.attributes[attr] != null);
return hasScriptsEventAttrs.some((attr) => node.attributes[attr] != null);
};

/**
Expand Down
45 changes: 18 additions & 27 deletions plugins/convertPathData.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,8 +230,7 @@ const convertToRelative = (pathData) => {
cursor[1] += args[1];
start[0] = cursor[0];
start[1] = cursor[1];
}
if (command === 'M') {
} else if (command === 'M') {
// M → m
// skip first moveto
if (i !== 0) {
Expand All @@ -247,11 +246,10 @@ const convertToRelative = (pathData) => {
}

// lineto (x y)
if (command === 'l') {
else if (command === 'l') {
cursor[0] += args[0];
cursor[1] += args[1];
}
if (command === 'L') {
} else if (command === 'L') {
// L → l
command = 'l';
args[0] -= cursor[0];
Expand All @@ -261,33 +259,30 @@ const convertToRelative = (pathData) => {
}

// horizontal lineto (x)
if (command === 'h') {
else if (command === 'h') {
cursor[0] += args[0];
}
if (command === 'H') {
} else if (command === 'H') {
// H → h
command = 'h';
args[0] -= cursor[0];
cursor[0] += args[0];
}

// vertical lineto (y)
if (command === 'v') {
else if (command === 'v') {
cursor[1] += args[0];
}
if (command === 'V') {
} else if (command === 'V') {
// V → v
command = 'v';
args[0] -= cursor[1];
cursor[1] += args[0];
}

// curveto (x1 y1 x2 y2 x y)
if (command === 'c') {
else if (command === 'c') {
cursor[0] += args[4];
cursor[1] += args[5];
}
if (command === 'C') {
} else if (command === 'C') {
// C → c
command = 'c';
args[0] -= cursor[0];
Expand All @@ -301,11 +296,10 @@ const convertToRelative = (pathData) => {
}

// smooth curveto (x2 y2 x y)
if (command === 's') {
else if (command === 's') {
cursor[0] += args[2];
cursor[1] += args[3];
}
if (command === 'S') {
} else if (command === 'S') {
// S → s
command = 's';
args[0] -= cursor[0];
Expand All @@ -317,11 +311,10 @@ const convertToRelative = (pathData) => {
}

// quadratic Bézier curveto (x1 y1 x y)
if (command === 'q') {
else if (command === 'q') {
cursor[0] += args[2];
cursor[1] += args[3];
}
if (command === 'Q') {
} else if (command === 'Q') {
// Q → q
command = 'q';
args[0] -= cursor[0];
Expand All @@ -333,11 +326,10 @@ const convertToRelative = (pathData) => {
}

// smooth quadratic Bézier curveto (x y)
if (command === 't') {
else if (command === 't') {
cursor[0] += args[0];
cursor[1] += args[1];
}
if (command === 'T') {
} else if (command === 'T') {
// T → t
command = 't';
args[0] -= cursor[0];
Expand All @@ -347,11 +339,10 @@ const convertToRelative = (pathData) => {
}

// elliptical arc (rx ry x-axis-rotation large-arc-flag sweep-flag x y)
if (command === 'a') {
else if (command === 'a') {
cursor[0] += args[5];
cursor[1] += args[6];
}
if (command === 'A') {
} else if (command === 'A') {
// A → a
command = 'a';
args[5] -= cursor[0];
Expand All @@ -361,7 +352,7 @@ const convertToRelative = (pathData) => {
}

// closepath
if (command === 'Z' || command === 'z') {
else if (command === 'Z' || command === 'z') {
// reset cursor
cursor[0] = start[0];
cursor[1] = start[1];
Expand Down
116 changes: 58 additions & 58 deletions plugins/removeHiddenElems.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,38 +190,6 @@ export const fn = (root, params) => {
}
}

// Removes hidden elements
// https://www.w3schools.com/cssref/pr_class_visibility.asp
const computedStyle = computeStyle(stylesheet, node);
if (
isHidden &&
computedStyle.visibility &&
computedStyle.visibility.type === 'static' &&
computedStyle.visibility.value === 'hidden' &&
// keep if any descendant enables visibility
querySelector(node, '[visibility=visible]') == null
) {
removeElement(node, parentNode);
return;
}

// display="none"
//
// https://www.w3.org/TR/SVG11/painting.html#DisplayProperty
// "A value of display: none indicates that the given element
// and its children shall not be rendered directly"
if (
displayNone &&
computedStyle.display &&
computedStyle.display.type === 'static' &&
computedStyle.display.value === 'none' &&
// markers with display: none still rendered
node.name !== 'marker'
) {
removeElement(node, parentNode);
return;
}

// Circles with zero radius
//
// https://www.w3.org/TR/SVG11/shapes.html#CircleElementRAttribute
Expand Down Expand Up @@ -363,32 +331,6 @@ export const fn = (root, params) => {
return;
}

// Path with empty data
//
// https://www.w3.org/TR/SVG11/paths.html#DAttribute
//
// <path d=""/>
if (pathEmptyD && node.name === 'path') {
if (node.attributes.d == null) {
removeElement(node, parentNode);
return;
}
const pathData = parsePathData(node.attributes.d);
if (pathData.length === 0) {
removeElement(node, parentNode);
return;
}
// keep single point paths for markers
if (
pathData.length === 1 &&
computedStyle['marker-start'] == null &&
computedStyle['marker-end'] == null
) {
removeElement(node, parentNode);
return;
}
}

// Polyline with empty points
//
// https://www.w3.org/TR/SVG11/shapes.html#PolylineElementPointsAttribute
Expand Down Expand Up @@ -417,6 +359,64 @@ export const fn = (root, params) => {
return;
}

// Removes hidden elements
// https://www.w3schools.com/cssref/pr_class_visibility.asp
const computedStyle = computeStyle(stylesheet, node);
if (
isHidden &&
computedStyle.visibility &&
computedStyle.visibility.type === 'static' &&
computedStyle.visibility.value === 'hidden' &&
// keep if any descendant enables visibility
querySelector(node, '[visibility=visible]') == null
) {
removeElement(node, parentNode);
return;
}

// display="none"
//
// https://www.w3.org/TR/SVG11/painting.html#DisplayProperty
// "A value of display: none indicates that the given element
// and its children shall not be rendered directly"
if (
displayNone &&
computedStyle.display &&
computedStyle.display.type === 'static' &&
computedStyle.display.value === 'none' &&
// markers with display: none still rendered
node.name !== 'marker'
) {
removeElement(node, parentNode);
return;
}

// Path with empty data
//
// https://www.w3.org/TR/SVG11/paths.html#DAttribute
//
// <path d=""/>
if (pathEmptyD && node.name === 'path') {
if (node.attributes.d == null) {
removeElement(node, parentNode);
return;
}
const pathData = parsePathData(node.attributes.d);
if (pathData.length === 0) {
removeElement(node, parentNode);
return;
}
// keep single point paths for markers
if (
pathData.length === 1 &&
computedStyle['marker-start'] == null &&
computedStyle['marker-end'] == null
) {
removeElement(node, parentNode);
return;
}
}

for (const [name, value] of Object.entries(node.attributes)) {
const ids = findReferences(name, value);

Expand Down
20 changes: 13 additions & 7 deletions test/regression/compare.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ const HEIGHT = 720;
/** @type {import('playwright').PageScreenshotOptions} */
const screenshotOptions = {
omitBackground: true,
clip: { x: 0, y: 0, width: WIDTH, height: HEIGHT },
animations: 'disabled',
};

Expand Down Expand Up @@ -63,21 +62,28 @@ const runTests = async (list) => {
*/
const processFile = async (page, name) => {
await page.goto(`file://${path.join(REGRESSION_FIXTURES_PATH, name)}`);
const originalBuffer = await page.screenshot(screenshotOptions);
let element = await page.waitForSelector('svg');
const originalBuffer = await element.screenshot(screenshotOptions);

await page.goto(`file://${path.join(REGRESSION_OPTIMIZED_PATH, name)}`);
const optimizedBufferPromise = page.screenshot(screenshotOptions);
element = await page.waitForSelector('svg');
const optimizedBufferPromise = element.screenshot(screenshotOptions);

const writeDiffs = process.env.NO_DIFF == null;
const diff = writeDiffs ? new PNG({ width: WIDTH, height: HEIGHT }) : null;
const originalPng = PNG.sync.read(originalBuffer);
const optimizedPng = PNG.sync.read(await optimizedBufferPromise);
const writeDiffs = process.env.NO_DIFF == null;
const diff = writeDiffs
? new PNG({ width: originalPng.width, height: originalPng.height })
: null;

// TODO: Maybe we would also like to manually compare orig and optimized size otherwise we *may* crash the entire test process otherwise.
// But at the same time it is much more unlikely to have a change in the
const matched = pixelmatch(
originalPng.data,
optimizedPng.data,
diff?.data,
WIDTH,
HEIGHT,
originalPng.width,
originalPng.height,
);

// ignore small aliasing issues
Expand Down
Loading