From e9e0778aed77bd5884f0a36d392b631047f4fb26 Mon Sep 17 00:00:00 2001 From: Lorf Date: Sun, 18 May 2025 00:51:18 +0200 Subject: [PATCH 1/4] refactor(test-optimization): Reduced the regression test runtime by simply limiting the screenshot area of the output to the rendered svg --- test/regression/compare.js | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/test/regression/compare.js b/test/regression/compare.js index 3a16d1e43..a09f586d5 100644 --- a/test/regression/compare.js +++ b/test/regression/compare.js @@ -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', }; @@ -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 From 809c5893a068dbc936a6a316cfcdaba3f8e6b621 Mon Sep 17 00:00:00 2001 From: Lorf Date: Sun, 18 May 2025 00:55:38 +0200 Subject: [PATCH 2/4] refactor(optimization): Define and initalize the hasScripts event attributes to check for once --- lib/svgo/tools.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/svgo/tools.js b/lib/svgo/tools.js index 439157a4f..b4aafb8c1 100644 --- a/lib/svgo/tools.js +++ b/lib/svgo/tools.js @@ -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. @@ -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); }; /** From a224a4868fd5b6bbd9ba718ce2cbf06d095b6ba2 Mon Sep 17 00:00:00 2001 From: Lorf Date: Sun, 18 May 2025 00:56:30 +0200 Subject: [PATCH 3/4] refactor(optimization): Prevent unecessary branch checks once a command has been matched successfully --- plugins/convertPathData.js | 45 +++++++++++++++----------------------- 1 file changed, 18 insertions(+), 27 deletions(-) diff --git a/plugins/convertPathData.js b/plugins/convertPathData.js index 1f032b3e9..36d2e2e33 100644 --- a/plugins/convertPathData.js +++ b/plugins/convertPathData.js @@ -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) { @@ -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]; @@ -261,10 +259,9 @@ 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]; @@ -272,10 +269,9 @@ const convertToRelative = (pathData) => { } // 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]; @@ -283,11 +279,10 @@ const convertToRelative = (pathData) => { } // 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]; @@ -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]; @@ -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]; @@ -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]; @@ -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]; @@ -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]; From b0d1d01a34ad72aec91165bebdb910c65d4f6c2a Mon Sep 17 00:00:00 2001 From: Lorf Date: Sun, 18 May 2025 00:57:13 +0200 Subject: [PATCH 4/4] refactor(optimization): Only compute element style when simple checks have not catched on --- plugins/removeHiddenElems.js | 116 +++++++++++++++++------------------ 1 file changed, 58 insertions(+), 58 deletions(-) diff --git a/plugins/removeHiddenElems.js b/plugins/removeHiddenElems.js index 0104543cd..4059e8090 100644 --- a/plugins/removeHiddenElems.js +++ b/plugins/removeHiddenElems.js @@ -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 @@ -363,32 +331,6 @@ export const fn = (root, params) => { return; } - // Path with empty data - // - // https://www.w3.org/TR/SVG11/paths.html#DAttribute - // - // - 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 @@ -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 + // + // + 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);