From 124f09ab29f6c113f3320360533875449ec0b201 Mon Sep 17 00:00:00 2001 From: John Kenny Date: Thu, 3 Oct 2024 18:06:04 -0700 Subject: [PATCH 01/26] Rewrite removeHiddenElems. --- plugins/removeHiddenElems-orig.js | 514 ++++++++++++++++ plugins/removeHiddenElems.js | 549 +++--------------- test/plugins/_index.test.js | 1 + ...t => removeHiddenElems.10.failing.svg.txt} | 0 ...t => removeHiddenElems.12.failing.svg.txt} | 0 ...t => removeHiddenElems.13.failing.svg.txt} | 0 test/plugins/removeHiddenElems.14.svg.txt | 14 - test/plugins/removeHiddenElems.16.svg.txt | 4 +- ...t => removeHiddenElems.19.failing.svg.txt} | 0 test/plugins/removeHiddenElems.20.svg.txt | 1 + 10 files changed, 612 insertions(+), 471 deletions(-) create mode 100644 plugins/removeHiddenElems-orig.js rename test/plugins/{removeHiddenElems.10.svg.txt => removeHiddenElems.10.failing.svg.txt} (100%) rename test/plugins/{removeHiddenElems.12.svg.txt => removeHiddenElems.12.failing.svg.txt} (100%) rename test/plugins/{removeHiddenElems.13.svg.txt => removeHiddenElems.13.failing.svg.txt} (100%) delete mode 100644 test/plugins/removeHiddenElems.14.svg.txt rename test/plugins/{removeHiddenElems.19.svg.txt => removeHiddenElems.19.failing.svg.txt} (100%) diff --git a/plugins/removeHiddenElems-orig.js b/plugins/removeHiddenElems-orig.js new file mode 100644 index 0000000..42891d9 --- /dev/null +++ b/plugins/removeHiddenElems-orig.js @@ -0,0 +1,514 @@ +/** + * @typedef {import('../lib/types.js').XastChild} XastChild + * @typedef {import('../lib/types.js').XastElement} XastElement + * @typedef {import('../lib/types.js').XastParent} XastParent + */ + +import { elemsGroups } from './_collections.js'; +import { querySelector, detachNodeFromParent } from '../lib/xast.js'; +import { parsePathData } from '../lib/path.js'; +import { findReferences } from '../lib/svgo/tools.js'; + +const nonRendering = elemsGroups.nonRendering; + +export const name = 'removeHiddenElems'; +export const description = + 'removes hidden elements (zero sized, with absent attributes)'; + +/** + * Remove hidden elements with disabled rendering: + * - display="none" + * - opacity="0" + * - circle with zero radius + * - ellipse with zero x-axis or y-axis radius + * - rectangle with zero width or height + * - pattern with zero width or height + * - image with zero width or height + * - path with empty data + * - polyline with empty points + * - polygon with empty points + * + * @author Kir Belevich + * + * @type {import('./plugins-types.js').Plugin<'removeHiddenElems'>} + */ +export const fn = (root, params, info) => { + const { + isHidden = true, + displayNone = true, + opacity0 = true, + circleR0 = true, + ellipseRX0 = true, + ellipseRY0 = true, + rectWidth0 = true, + rectHeight0 = true, + patternWidth0 = true, + patternHeight0 = true, + imageWidth0 = true, + imageHeight0 = true, + pathEmptyD = true, + polylineEmptyPoints = true, + polygonEmptyPoints = true, + } = params; + + const styleData = info.docData.getStyles(); + if ( + info.docData.hasScripts() || + styleData === null || + !styleData.hasOnlyFeatures(['simple-selectors', 'attribute-selectors']) + ) { + return; + } + + let inNonRenderingNode = 0; + + /** + * Skip non-rendered nodes initially, and only detach if they have no ID, or + * their ID is not referenced by another node. + * + * @type {Set} + */ + const nonRenderedNodes = new Set(); + + /** + * IDs for removed hidden definitions. + * + * @type {Set} + */ + const removedDefIds = new Set(); + + /** @type {Set} */ + const defNodesToRemove = new Set(); + + /** + * @type {Set} + */ + const allDefs = new Set(); + + /** @type {Map>} */ + const allReferences = new Map(); + + /** @type {Map} */ + const referencedIdsByNode = new Map(); + + /** + * @type {Map} + */ + const referencesById = new Map(); + + /** + * Nodes can't be removed if they or any of their children have an id attribute that is referenced. + * @param {XastElement} node + * @returns boolean + */ + function canRemoveNonRenderingNode(node) { + const refs = allReferences.get(node.attributes.id); + if (refs && refs.size) { + return false; + } + for (const child of node.children) { + if (child.type === 'element' && !canRemoveNonRenderingNode(child)) { + return false; + } + } + return true; + } + + /** + * Retrieve information about all IDs referenced by an element and its children. + * @param {XastElement} node + * @returns {XastElement[]} + */ + function getNodesReferencedByBranch(node) { + const allIds = []; + const thisNodeIds = referencedIdsByNode.get(node); + if (thisNodeIds) { + allIds.push(node); + } + for (const child of node.children) { + if (child.type === 'element') { + allIds.push(...getNodesReferencedByBranch(child)); + } + } + return allIds; + } + + /** + * @param {string} referencedId + * @param {XastElement} referencingElement + */ + function recordReference(referencedId, referencingElement) { + const refs = allReferences.get(referencedId); + if (refs) { + refs.add(referencingElement); + } else { + allReferences.set(referencedId, new Set([referencingElement])); + } + } + + /** + * @param {XastElement} node + * @param {XastParent} parentNode + */ + function removeElement(node, parentNode) { + if ( + node.type === 'element' && + node.attributes.id != null && + parentNode.type === 'element' && + parentNode.name === 'defs' + ) { + removedDefIds.add(node.attributes.id); + } + + defNodesToRemove.add(node); + } + + // Record all references in the style element. + const styleElement = styleData.getFirstStyleElement(); + if (styleElement) { + for (const id of styleData.getIdsReferencedByProperties()) { + recordReference(id, styleElement); + } + } + + return { + element: { + enter: (node, parentNode, parentInfo) => { + if (nonRendering.has(node.name)) { + nonRenderedNodes.add(node); + inNonRenderingNode++; + } + const computedStyle = styleData.computeStyle(node, parentInfo); + const opacity = computedStyle.get('opacity'); + // https://www.w3.org/TR/SVG11/masking.html#ObjectAndGroupOpacityProperties + if (opacity0 && opacity === '0') { + if (!inNonRenderingNode) { + if (node.name === 'path') { + // It's possible this will be referenced in a . + nonRenderedNodes.add(node); + } else { + removeElement(node, parentNode); + return; + } + } + } + + if (node.name === 'defs') { + allDefs.add(node); + } + + if (node.name === 'use') { + for (const attr of Object.keys(node.attributes)) { + if (attr !== 'href' && !attr.endsWith(':href')) continue; + const value = node.attributes[attr]; + const id = value.slice(1); + + let refs = referencesById.get(id); + if (!refs) { + refs = []; + referencesById.set(id, refs); + } + refs.push(node); + } + } + + // Removes hidden elements + // https://www.w3schools.com/cssref/pr_class_visibility.asp + const visibility = computedStyle.get('visibility'); + if ( + isHidden && + visibility === '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" + const display = computedStyle.get('display'); + if ( + displayNone && + display === '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 + // "A value of zero disables rendering of the element" + // + // + if ( + circleR0 && + node.name === 'circle' && + node.children.length === 0 && + node.attributes.r === '0' + ) { + removeElement(node, parentNode); + return; + } + + // Ellipse with zero x-axis radius + // + // https://www.w3.org/TR/SVG11/shapes.html#EllipseElementRXAttribute + // "A value of zero disables rendering of the element" + // + // + if ( + ellipseRX0 && + node.name === 'ellipse' && + node.children.length === 0 && + node.attributes.rx === '0' + ) { + removeElement(node, parentNode); + return; + } + + // Ellipse with zero y-axis radius + // + // https://www.w3.org/TR/SVG11/shapes.html#EllipseElementRYAttribute + // "A value of zero disables rendering of the element" + // + // + if ( + ellipseRY0 && + node.name === 'ellipse' && + node.children.length === 0 && + node.attributes.ry === '0' + ) { + removeElement(node, parentNode); + return; + } + + // Rectangle with zero width + // + // https://www.w3.org/TR/SVG11/shapes.html#RectElementWidthAttribute + // "A value of zero disables rendering of the element" + // + // + if ( + rectWidth0 && + node.name === 'rect' && + node.children.length === 0 && + node.attributes.width === '0' + ) { + removeElement(node, parentNode); + return; + } + + // Rectangle with zero height + // + // https://www.w3.org/TR/SVG11/shapes.html#RectElementHeightAttribute + // "A value of zero disables rendering of the element" + // + // + if ( + rectHeight0 && + rectWidth0 && + node.name === 'rect' && + node.children.length === 0 && + node.attributes.height === '0' + ) { + removeElement(node, parentNode); + return; + } + + // Pattern with zero width + // + // https://www.w3.org/TR/SVG11/pservers.html#PatternElementWidthAttribute + // "A value of zero disables rendering of the element (i.e., no paint is applied)" + // + // + if ( + patternWidth0 && + node.name === 'pattern' && + node.attributes.width === '0' + ) { + removeElement(node, parentNode); + return; + } + + // Pattern with zero height + // + // https://www.w3.org/TR/SVG11/pservers.html#PatternElementHeightAttribute + // "A value of zero disables rendering of the element (i.e., no paint is applied)" + // + // + if ( + patternHeight0 && + node.name === 'pattern' && + node.attributes.height === '0' + ) { + removeElement(node, parentNode); + return; + } + + // Image with zero width + // + // https://www.w3.org/TR/SVG11/struct.html#ImageElementWidthAttribute + // "A value of zero disables rendering of the element" + // + // + if ( + imageWidth0 && + node.name === 'image' && + node.attributes.width === '0' + ) { + removeElement(node, parentNode); + return; + } + + // Image with zero height + // + // https://www.w3.org/TR/SVG11/struct.html#ImageElementHeightAttribute + // "A value of zero disables rendering of the element" + // + // + if ( + imageHeight0 && + node.name === 'image' && + node.attributes.height === '0' + ) { + 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.has('marker-start') && + !computedStyle.has('marker-end') + ) { + removeElement(node, parentNode); + return; + } + } + + // Polyline with empty points + // + // https://www.w3.org/TR/SVG11/shapes.html#PolylineElementPointsAttribute + // + // + if ( + polylineEmptyPoints && + node.name === 'polyline' && + node.attributes.points == null + ) { + removeElement(node, parentNode); + return; + } + + // Polygon with empty points + // + // https://www.w3.org/TR/SVG11/shapes.html#PolygonElementPointsAttribute + // + // + if ( + polygonEmptyPoints && + node.name === 'polygon' && + node.attributes.points == null + ) { + removeElement(node, parentNode); + return; + } + + const allIds = []; + for (const [name, value] of Object.entries(node.attributes)) { + const ids = findReferences(name, value); + + // Record which other nodes are referenced by this node. + for (const id of ids) { + allIds.push(id); + recordReference(id, node); + } + } + + // Record which ids are referenced by this node. + if (allIds.length) { + referencedIdsByNode.set(node, allIds); + } + }, + exit: (node) => { + if (nonRendering.has(node.name)) { + inNonRenderingNode--; + } + }, + }, + root: { + exit: () => { + for (const child of defNodesToRemove) { + detachNodeFromParent(child); + nonRenderedNodes.delete(child); + } + + for (const id of removedDefIds) { + const refs = referencesById.get(id); + if (refs) { + for (const node of refs) { + detachNodeFromParent(node); + } + } + } + + let tryAgain; + do { + tryAgain = false; + for (const nonRenderedNode of nonRenderedNodes) { + if (canRemoveNonRenderingNode(nonRenderedNode)) { + detachNodeFromParent(nonRenderedNode); + nonRenderedNodes.delete(nonRenderedNode); + + // For any elements referenced by the just-deleted node and its children, remove the node from the list of referencing nodes. + const deletedReferenceNodes = + getNodesReferencedByBranch(nonRenderedNode); + for (const deletedNode of deletedReferenceNodes) { + const referencedIds = referencedIdsByNode.get(deletedNode); + if (referencedIds) { + for (const id of referencedIds) { + const referencingNodes = allReferences.get(id); + if (referencingNodes) { + referencingNodes.delete(deletedNode); + if (referencingNodes.size === 0) { + tryAgain = true; + } + } + } + } + } + } + } + } while (tryAgain); + + for (const node of allDefs) { + if (node.children.length === 0) { + detachNodeFromParent(node); + } + } + }, + }, + }; +}; diff --git a/plugins/removeHiddenElems.js b/plugins/removeHiddenElems.js index 42891d9..5f4e046 100644 --- a/plugins/removeHiddenElems.js +++ b/plugins/removeHiddenElems.js @@ -1,56 +1,15 @@ -/** - * @typedef {import('../lib/types.js').XastChild} XastChild - * @typedef {import('../lib/types.js').XastElement} XastElement - * @typedef {import('../lib/types.js').XastParent} XastParent - */ - +import { getReferencedIds } from '../lib/svgo/tools.js'; +import { visitSkip } from '../lib/xast.js'; import { elemsGroups } from './_collections.js'; -import { querySelector, detachNodeFromParent } from '../lib/xast.js'; -import { parsePathData } from '../lib/path.js'; -import { findReferences } from '../lib/svgo/tools.js'; - -const nonRendering = elemsGroups.nonRendering; export const name = 'removeHiddenElems'; export const description = - 'removes hidden elements (zero sized, with absent attributes)'; + 'removes non-rendered elements that are not referenced'; /** - * Remove hidden elements with disabled rendering: - * - display="none" - * - opacity="0" - * - circle with zero radius - * - ellipse with zero x-axis or y-axis radius - * - rectangle with zero width or height - * - pattern with zero width or height - * - image with zero width or height - * - path with empty data - * - polyline with empty points - * - polygon with empty points - * - * @author Kir Belevich - * * @type {import('./plugins-types.js').Plugin<'removeHiddenElems'>} */ export const fn = (root, params, info) => { - const { - isHidden = true, - displayNone = true, - opacity0 = true, - circleR0 = true, - ellipseRX0 = true, - ellipseRY0 = true, - rectWidth0 = true, - rectHeight0 = true, - patternWidth0 = true, - patternHeight0 = true, - imageWidth0 = true, - imageHeight0 = true, - pathEmptyD = true, - polylineEmptyPoints = true, - polygonEmptyPoints = true, - } = params; - const styleData = info.docData.getStyles(); if ( info.docData.hasScripts() || @@ -60,453 +19,133 @@ export const fn = (root, params, info) => { return; } - let inNonRenderingNode = 0; - - /** - * Skip non-rendered nodes initially, and only detach if they have no ID, or - * their ID is not referenced by another node. - * - * @type {Set} - */ - const nonRenderedNodes = new Set(); - - /** - * IDs for removed hidden definitions. - * - * @type {Set} - */ - const removedDefIds = new Set(); - - /** @type {Set} */ - const defNodesToRemove = new Set(); + /** @type {Map>} */ + const referencedIds = new Map(); + for (const id of styleData.getReferencedIds().keys()) { + addIdReference(id, undefined); + } - /** - * @type {Set} - */ - const allDefs = new Set(); + /** @type {Map>} */ + const nonRenderedElements = new Map(); - /** @type {Map>} */ - const allReferences = new Map(); + /** Associate children of non-rendering elements with the top-level element. */ + /** @type {Map} */ - const referencedIdsByNode = new Map(); + /** @type {Set} */ + const nonRenderedReferencingChildren = new Set(); /** - * @type {Map} + * @param {string} id + * @param {import('../lib/types.js').XastElement|undefined} element */ - const referencesById = new Map(); - - /** - * Nodes can't be removed if they or any of their children have an id attribute that is referenced. - * @param {XastElement} node - * @returns boolean - */ - function canRemoveNonRenderingNode(node) { - const refs = allReferences.get(node.attributes.id); - if (refs && refs.size) { - return false; - } - for (const child of node.children) { - if (child.type === 'element' && !canRemoveNonRenderingNode(child)) { - return false; - } + function addIdReference(id, element) { + let referencingElements = referencedIds.get(id); + if (!referencingElements) { + referencingElements = new Set(); + referencedIds.set(id, referencingElements); } - return true; + referencingElements.add(element); } /** - * Retrieve information about all IDs referenced by an element and its children. - * @param {XastElement} node - * @returns {XastElement[]} + * @param {import('../lib/types.js').XastElement} topElement */ - function getNodesReferencedByBranch(node) { - const allIds = []; - const thisNodeIds = referencedIdsByNode.get(node); - if (thisNodeIds) { - allIds.push(node); - } - for (const child of node.children) { - if (child.type === 'element') { - allIds.push(...getNodesReferencedByBranch(child)); + function addNonRenderedElement(topElement) { + /** + * @param {import('../lib/types.js').XastElement} element + */ + function processElement(element) { + if (element.attributes.id) { + ids.add(element.attributes.id); + nonRenderedReferencingChildren.add(element); + nonRenderingChildren.set(element, topElement); + } + for (const child of element.children) { + if (child.type === 'element') { + processElement(child); + } } } - return allIds; - } - /** - * @param {string} referencedId - * @param {XastElement} referencingElement - */ - function recordReference(referencedId, referencingElement) { - const refs = allReferences.get(referencedId); - if (refs) { - refs.add(referencingElement); - } else { - allReferences.set(referencedId, new Set([referencingElement])); - } + const ids = new Set(); + processElement(topElement); + nonRenderedElements.set(topElement, ids); } /** - * @param {XastElement} node - * @param {XastParent} parentNode + * @param {import('../lib/types.js').XastElement} element */ - function removeElement(node, parentNode) { - if ( - node.type === 'element' && - node.attributes.id != null && - parentNode.type === 'element' && - parentNode.name === 'defs' - ) { - removedDefIds.add(node.attributes.id); + function isReferenced(element) { + const ids = nonRenderedElements.get(element); + if (!ids) { + throw new Error(); } - - defNodesToRemove.add(node); - } - - // Record all references in the style element. - const styleElement = styleData.getFirstStyleElement(); - if (styleElement) { - for (const id of styleData.getIdsReferencedByProperties()) { - recordReference(id, styleElement); - } - } - - return { - element: { - enter: (node, parentNode, parentInfo) => { - if (nonRendering.has(node.name)) { - nonRenderedNodes.add(node); - inNonRenderingNode++; - } - const computedStyle = styleData.computeStyle(node, parentInfo); - const opacity = computedStyle.get('opacity'); - // https://www.w3.org/TR/SVG11/masking.html#ObjectAndGroupOpacityProperties - if (opacity0 && opacity === '0') { - if (!inNonRenderingNode) { - if (node.name === 'path') { - // It's possible this will be referenced in a . - nonRenderedNodes.add(node); - } else { - removeElement(node, parentNode); - return; - } + for (const id of ids) { + const referencingEls = referencedIds.get(id); + if (referencingEls) { + // See if any of the referencing nodes are referenced. + for (const el of referencingEls) { + if (el === undefined) { + // Referenced in a + + + + + + +@@@ + + + + + + + + From 5d679972829687e003a04ef57e305273cbfa69fd Mon Sep 17 00:00:00 2001 From: John Kenny Date: Fri, 4 Oct 2024 14:34:09 -0700 Subject: [PATCH 06/26] Remove elements with display=none. --- plugins/removeHiddenElems.js | 57 ++++++++++++++++++++++++++++-------- 1 file changed, 44 insertions(+), 13 deletions(-) diff --git a/plugins/removeHiddenElems.js b/plugins/removeHiddenElems.js index 4db5779..a651caf 100644 --- a/plugins/removeHiddenElems.js +++ b/plugins/removeHiddenElems.js @@ -19,6 +19,10 @@ export const fn = (root, params, info) => { return; } + // Record which elements to delete, sorted by parent. + /** @type {Map>} */ + const childrenToDeleteByParent = new Map(); + /** @type {Map>} */ const referencedIds = new Map(); for (const id of styleData.getReferencedIds().keys()) { @@ -113,6 +117,18 @@ export const fn = (root, params, info) => { return false; } + /** + * @param {import('../lib/types.js').XastElement} element + */ + function removeElement(element) { + let childrenToDelete = childrenToDeleteByParent.get(element.parentNode); + if (!childrenToDelete) { + childrenToDelete = new Set(); + childrenToDeleteByParent.set(element.parentNode, childrenToDelete); + } + childrenToDelete.add(element); + } + /** * @param {import('../lib/types.js').XastElement} element */ @@ -126,7 +142,7 @@ export const fn = (root, params, info) => { return { element: { - enter: (element) => { + enter: (element, parentNode, parentInfo) => { // Record any ids referenced by this element. recordReferencedIds(element); @@ -148,28 +164,43 @@ export const fn = (root, params, info) => { addNonRenderedElement(element); return visitSkip; } + + // Remove any rendering elements which are not visible. + + const properties = styleData.computeStyle(element, parentInfo); + if (!properties) { + return; + } + + // Remove empty paths. + if (element.name === 'path' && !element.attributes.d) { + removeElement(element); + return; + } + + // 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" + const display = properties.get('display'); + if ( + display === 'none' && + // markers with display: none still rendered + element.name !== 'marker' + ) { + removeElement(element); + return; + } }, exit: () => {}, }, root: { exit: () => { - // Record which elements to delete, sorted by parent. - /** @type {Map>} */ - const childrenToDeleteByParent = new Map(); - for (const element of nonRenderedElements.keys()) { if (isReferenced(element)) { continue; } - let childrenToDelete = childrenToDeleteByParent.get( - element.parentNode, - ); - if (!childrenToDelete) { - childrenToDelete = new Set(); - childrenToDeleteByParent.set(element.parentNode, childrenToDelete); - } - childrenToDelete.add(element); + removeElement(element); } // For each parent, delete no longer needed children. From 8e3ab16356acd0bf91cae9155d4d9aa0cd8e4fe7 Mon Sep 17 00:00:00 2001 From: John Kenny Date: Fri, 4 Oct 2024 14:56:18 -0700 Subject: [PATCH 07/26] Restored test files. --- ...iddenElems.12.failing.svg.txt => removeHiddenElems.12.svg.txt} | 0 ...iddenElems.13.failing.svg.txt => removeHiddenElems.13.svg.txt} | 0 ...iddenElems.19.failing.svg.txt => removeHiddenElems.19.svg.txt} | 0 3 files changed, 0 insertions(+), 0 deletions(-) rename test/plugins/{removeHiddenElems.12.failing.svg.txt => removeHiddenElems.12.svg.txt} (100%) rename test/plugins/{removeHiddenElems.13.failing.svg.txt => removeHiddenElems.13.svg.txt} (100%) rename test/plugins/{removeHiddenElems.19.failing.svg.txt => removeHiddenElems.19.svg.txt} (100%) diff --git a/test/plugins/removeHiddenElems.12.failing.svg.txt b/test/plugins/removeHiddenElems.12.svg.txt similarity index 100% rename from test/plugins/removeHiddenElems.12.failing.svg.txt rename to test/plugins/removeHiddenElems.12.svg.txt diff --git a/test/plugins/removeHiddenElems.13.failing.svg.txt b/test/plugins/removeHiddenElems.13.svg.txt similarity index 100% rename from test/plugins/removeHiddenElems.13.failing.svg.txt rename to test/plugins/removeHiddenElems.13.svg.txt diff --git a/test/plugins/removeHiddenElems.19.failing.svg.txt b/test/plugins/removeHiddenElems.19.svg.txt similarity index 100% rename from test/plugins/removeHiddenElems.19.failing.svg.txt rename to test/plugins/removeHiddenElems.19.svg.txt From c3808a71eb03e83be9c46d4a019cd75133e575be Mon Sep 17 00:00:00 2001 From: John Kenny Date: Fri, 4 Oct 2024 15:20:26 -0700 Subject: [PATCH 08/26] Remove elements with opacity=0. --- plugins/removeHiddenElems.js | 12 ++++++++++ ...g.svg.txt => removeHiddenElems.08.svg.txt} | 0 test/plugins/removeHiddenElems.27.svg.txt | 24 +++++++++++++++++++ 3 files changed, 36 insertions(+) rename test/plugins/{removeHiddenElems.08.failing.svg.txt => removeHiddenElems.08.svg.txt} (100%) create mode 100644 test/plugins/removeHiddenElems.27.svg.txt diff --git a/plugins/removeHiddenElems.js b/plugins/removeHiddenElems.js index a651caf..182cd64 100644 --- a/plugins/removeHiddenElems.js +++ b/plugins/removeHiddenElems.js @@ -190,6 +190,18 @@ export const fn = (root, params, info) => { removeElement(element); return; } + + const opacity = properties.get('opacity'); + // https://www.w3.org/TR/SVG11/masking.html#ObjectAndGroupOpacityProperties + if (opacity === '0') { + if (element.name === 'path') { + // It's possible this will be referenced in a ; treat it as a non-rendered element. + addNonRenderedElement(element); + } else { + removeElement(element); + return; + } + } }, exit: () => {}, }, diff --git a/test/plugins/removeHiddenElems.08.failing.svg.txt b/test/plugins/removeHiddenElems.08.svg.txt similarity index 100% rename from test/plugins/removeHiddenElems.08.failing.svg.txt rename to test/plugins/removeHiddenElems.08.svg.txt diff --git a/test/plugins/removeHiddenElems.27.svg.txt b/test/plugins/removeHiddenElems.27.svg.txt new file mode 100644 index 0000000..2620b6e --- /dev/null +++ b/test/plugins/removeHiddenElems.27.svg.txt @@ -0,0 +1,24 @@ +Preserve referenced path, even when path has opacity=0 and is not in . + +=== + + + + + + this is path 2 + + + + + +@@@ + + + + + + this is path 2 + + + From c5944017eb6ebe57c670fa3a8e33e266e8bb3bf5 Mon Sep 17 00:00:00 2001 From: John Kenny Date: Fri, 4 Oct 2024 17:11:20 -0700 Subject: [PATCH 09/26] Delete with 0 width or height. --- plugins/removeHiddenElems.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/plugins/removeHiddenElems.js b/plugins/removeHiddenElems.js index 182cd64..5fe0ede 100644 --- a/plugins/removeHiddenElems.js +++ b/plugins/removeHiddenElems.js @@ -178,6 +178,20 @@ export const fn = (root, params, info) => { return; } + // + // https://svgwg.org/svg2-draft/shapes.html#RectElement + if ( + element.name === 'rect' && + element.children.length === 0 && + (!element.attributes.width || + !element.attributes.height || + element.attributes.width === '0' || + element.attributes.height === '0') + ) { + removeElement(element); + return; + } + // 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" From f14cae4d9a7bdd23aebba2dd81ee77dbe92fc8bb Mon Sep 17 00:00:00 2001 From: John Kenny Date: Fri, 4 Oct 2024 17:49:48 -0700 Subject: [PATCH 10/26] Remove empty shapes from within non-rendered elements. --- plugins/removeHiddenElems.js | 55 +++++++++++++++++++++++------------- 1 file changed, 35 insertions(+), 20 deletions(-) diff --git a/plugins/removeHiddenElems.js b/plugins/removeHiddenElems.js index 5fe0ede..3fcc0b0 100644 --- a/plugins/removeHiddenElems.js +++ b/plugins/removeHiddenElems.js @@ -57,6 +57,11 @@ export const fn = (root, params, info) => { * @param {import('../lib/types.js').XastElement} element */ function processElement(element) { + // If the element is an empty shape, remove it and don't process any children. + if (removeEmptyShapes(element)) { + return; + } + if (element.attributes.id) { ids.add(element.attributes.id); } @@ -140,6 +145,32 @@ export const fn = (root, params, info) => { return ids.length !== 0; } + /** + * @param {import('../lib/types.js').XastElement} element + */ + function removeEmptyShapes(element) { + // Remove empty paths. + if (element.name === 'path' && !element.attributes.d) { + removeElement(element); + return true; + } + + // https://svgwg.org/svg2-draft/shapes.html#RectElement + if ( + element.name === 'rect' && + element.children.length === 0 && + (!element.attributes.width || + !element.attributes.height || + element.attributes.width === '0' || + element.attributes.height === '0') + ) { + removeElement(element); + return true; + } + + return false; + } + return { element: { enter: (element, parentNode, parentInfo) => { @@ -165,30 +196,14 @@ export const fn = (root, params, info) => { return visitSkip; } - // Remove any rendering elements which are not visible. - - const properties = styleData.computeStyle(element, parentInfo); - if (!properties) { + if (removeEmptyShapes(element)) { return; } - // Remove empty paths. - if (element.name === 'path' && !element.attributes.d) { - removeElement(element); - return; - } + // Remove any rendering elements which are not visible. - // - // https://svgwg.org/svg2-draft/shapes.html#RectElement - if ( - element.name === 'rect' && - element.children.length === 0 && - (!element.attributes.width || - !element.attributes.height || - element.attributes.width === '0' || - element.attributes.height === '0') - ) { - removeElement(element); + const properties = styleData.computeStyle(element, parentInfo); + if (!properties) { return; } From 3ba4d364543a723e36045d0429b97644187d4d7e Mon Sep 17 00:00:00 2001 From: John Kenny Date: Fri, 4 Oct 2024 19:12:09 -0700 Subject: [PATCH 11/26] Remove paths with length < 2. --- lib/path.js | 1 + plugins/minifyPathData.js | 6 +++- plugins/removeHiddenElems.js | 66 +++++++++++++++++++++--------------- 3 files changed, 44 insertions(+), 29 deletions(-) diff --git a/lib/path.js b/lib/path.js index 6ea533c..0cb2a5a 100644 --- a/lib/path.js +++ b/lib/path.js @@ -128,6 +128,7 @@ const readNumber = (string, cursor) => { /** * @param {string} string * @returns {PathDataItem[]} + * @deprecated */ export const parsePathData = (string) => { /** @type {PathDataItem[]} */ diff --git a/plugins/minifyPathData.js b/plugins/minifyPathData.js index 7365b48..25810ff 100644 --- a/plugins/minifyPathData.js +++ b/plugins/minifyPathData.js @@ -787,9 +787,10 @@ function stringifyCommand(cmdCode, prevCmdChar, lastNumber, args) { /** * @param {string} path + * @param {number} [maxCommands] * @returns {PathCommand[]} */ -export function parsePathCommands(path) { +export function parsePathCommands(path, maxCommands = Number.MAX_SAFE_INTEGER) { function addArg() { if (currentArg !== '') { args.push(currentArg); @@ -831,6 +832,9 @@ export function parsePathCommands(path) { switch (getCharType(c)) { case 'command': addCurrentCommand(c); + if (commands.length >= maxCommands) { + return commands; + } continue; case '.': if (currentCommand === '') { diff --git a/plugins/removeHiddenElems.js b/plugins/removeHiddenElems.js index 3fcc0b0..363c749 100644 --- a/plugins/removeHiddenElems.js +++ b/plugins/removeHiddenElems.js @@ -1,6 +1,7 @@ import { getReferencedIds } from '../lib/svgo/tools.js'; import { visitSkip } from '../lib/xast.js'; import { elemsGroups } from './_collections.js'; +import { parsePathCommands } from './minifyPathData.js'; export const name = 'removeHiddenElems'; export const description = @@ -122,18 +123,6 @@ export const fn = (root, params, info) => { return false; } - /** - * @param {import('../lib/types.js').XastElement} element - */ - function removeElement(element) { - let childrenToDelete = childrenToDeleteByParent.get(element.parentNode); - if (!childrenToDelete) { - childrenToDelete = new Set(); - childrenToDeleteByParent.set(element.parentNode, childrenToDelete); - } - childrenToDelete.add(element); - } - /** * @param {import('../lib/types.js').XastElement} element */ @@ -148,24 +137,45 @@ export const fn = (root, params, info) => { /** * @param {import('../lib/types.js').XastElement} element */ - function removeEmptyShapes(element) { - // Remove empty paths. - if (element.name === 'path' && !element.attributes.d) { - removeElement(element); - return true; + function removeElement(element) { + let childrenToDelete = childrenToDeleteByParent.get(element.parentNode); + if (!childrenToDelete) { + childrenToDelete = new Set(); + childrenToDeleteByParent.set(element.parentNode, childrenToDelete); } + childrenToDelete.add(element); + } - // https://svgwg.org/svg2-draft/shapes.html#RectElement - if ( - element.name === 'rect' && - element.children.length === 0 && - (!element.attributes.width || - !element.attributes.height || - element.attributes.width === '0' || - element.attributes.height === '0') - ) { - removeElement(element); - return true; + /** + * @param {import('../lib/types.js').XastElement} element + */ + function removeEmptyShapes(element) { + switch (element.name) { + case 'path': { + if (!element.attributes.d) { + removeElement(element); + return true; + } + const commands = parsePathCommands(element.attributes.d, 2); + if (commands.length < 2) { + removeElement(element); + return true; + } + return false; + } + case 'rect': + // https://svgwg.org/svg2-draft/shapes.html#RectElement + if ( + element.children.length === 0 && + (!element.attributes.width || + !element.attributes.height || + element.attributes.width === '0' || + element.attributes.height === '0') + ) { + removeElement(element); + return true; + } + break; } return false; From cde7b9cd8dd796f6a6f3873a792629e6c1ba64c8 Mon Sep 17 00:00:00 2001 From: John Kenny Date: Fri, 4 Oct 2024 19:49:02 -0700 Subject: [PATCH 12/26] Remove zero-radius ellipses. --- plugins/removeHiddenElems.js | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/plugins/removeHiddenElems.js b/plugins/removeHiddenElems.js index 363c749..c995359 100644 --- a/plugins/removeHiddenElems.js +++ b/plugins/removeHiddenElems.js @@ -148,9 +148,21 @@ export const fn = (root, params, info) => { /** * @param {import('../lib/types.js').XastElement} element + * @returns {boolean} */ function removeEmptyShapes(element) { switch (element.name) { + case 'ellipse': + // Ellipse with zero radius + // https://svgwg.org/svg2-draft/geometry.html#RxProperty + if ( + element.children.length === 0 && + (element.attributes.rx === '0' || element.attributes.ry === '0') + ) { + removeElement(element); + return true; + } + return false; case 'path': { if (!element.attributes.d) { removeElement(element); @@ -217,9 +229,6 @@ export const fn = (root, params, info) => { return; } - // 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" const display = properties.get('display'); if ( display === 'none' && @@ -231,7 +240,6 @@ export const fn = (root, params, info) => { } const opacity = properties.get('opacity'); - // https://www.w3.org/TR/SVG11/masking.html#ObjectAndGroupOpacityProperties if (opacity === '0') { if (element.name === 'path') { // It's possible this will be referenced in a ; treat it as a non-rendered element. From f567fc7e99218d7e554818c7af031af36eb3e953 Mon Sep 17 00:00:00 2001 From: John Kenny Date: Sat, 5 Oct 2024 07:39:19 -0700 Subject: [PATCH 13/26] Cleaned up test cases. --- plugins/minifyPathData.js | 2 +- plugins/removeHiddenElems.js | 18 ++++++++--- ....txt => minifyPathData.03.failing.svg.txt} | 0 ....txt => minifyPathData.04.failing.svg.txt} | 0 ....txt => minifyPathData.05.failing.svg.txt} | 0 ....txt => minifyPathData.06.failing.svg.txt} | 0 ...t => removeHiddenElems.03.failing.svg.txt} | 0 ...t => removeHiddenElems.07.failing.svg.txt} | 0 ...t => removeHiddenElems.08.failing.svg.txt} | 0 ...t => removeHiddenElems.12.failing.svg.txt} | 0 ...t => removeHiddenElems.13.failing.svg.txt} | 0 .../removeHiddenElems.28.failing.svg.txt | 32 +++++++++++++++++++ 12 files changed, 46 insertions(+), 6 deletions(-) rename test/plugins/{minifyPathData.03.svg.failing.txt => minifyPathData.03.failing.svg.txt} (100%) rename test/plugins/{minifyPathData.04.svg.failing.txt => minifyPathData.04.failing.svg.txt} (100%) rename test/plugins/{minifyPathData.05.svg.failing.txt => minifyPathData.05.failing.svg.txt} (100%) rename test/plugins/{minifyPathData.06.svg.failing.txt => minifyPathData.06.failing.svg.txt} (100%) rename test/plugins/{removeHiddenElems.03.svg.txt => removeHiddenElems.03.failing.svg.txt} (100%) rename test/plugins/{removeHiddenElems.07.svg.txt => removeHiddenElems.07.failing.svg.txt} (100%) rename test/plugins/{removeHiddenElems.08.svg.txt => removeHiddenElems.08.failing.svg.txt} (100%) rename test/plugins/{removeHiddenElems.12.svg.txt => removeHiddenElems.12.failing.svg.txt} (100%) rename test/plugins/{removeHiddenElems.13.svg.txt => removeHiddenElems.13.failing.svg.txt} (100%) create mode 100644 test/plugins/removeHiddenElems.28.failing.svg.txt diff --git a/plugins/minifyPathData.js b/plugins/minifyPathData.js index 25810ff..7ec3946 100644 --- a/plugins/minifyPathData.js +++ b/plugins/minifyPathData.js @@ -1092,7 +1092,7 @@ class ExactNum { } } -class PathParseError extends Error { +export class PathParseError extends Error { /** * @param {string} msg */ diff --git a/plugins/removeHiddenElems.js b/plugins/removeHiddenElems.js index c995359..b8e1411 100644 --- a/plugins/removeHiddenElems.js +++ b/plugins/removeHiddenElems.js @@ -1,7 +1,7 @@ import { getReferencedIds } from '../lib/svgo/tools.js'; import { visitSkip } from '../lib/xast.js'; import { elemsGroups } from './_collections.js'; -import { parsePathCommands } from './minifyPathData.js'; +import { parsePathCommands, PathParseError } from './minifyPathData.js'; export const name = 'removeHiddenElems'; export const description = @@ -168,10 +168,18 @@ export const fn = (root, params, info) => { removeElement(element); return true; } - const commands = parsePathCommands(element.attributes.d, 2); - if (commands.length < 2) { - removeElement(element); - return true; + try { + const commands = parsePathCommands(element.attributes.d, 2); + if (commands.length < 2) { + removeElement(element); + return true; + } + } catch (error) { + if (error instanceof PathParseError) { + console.warn(error.message); + return false; + } + throw error; } return false; } diff --git a/test/plugins/minifyPathData.03.svg.failing.txt b/test/plugins/minifyPathData.03.failing.svg.txt similarity index 100% rename from test/plugins/minifyPathData.03.svg.failing.txt rename to test/plugins/minifyPathData.03.failing.svg.txt diff --git a/test/plugins/minifyPathData.04.svg.failing.txt b/test/plugins/minifyPathData.04.failing.svg.txt similarity index 100% rename from test/plugins/minifyPathData.04.svg.failing.txt rename to test/plugins/minifyPathData.04.failing.svg.txt diff --git a/test/plugins/minifyPathData.05.svg.failing.txt b/test/plugins/minifyPathData.05.failing.svg.txt similarity index 100% rename from test/plugins/minifyPathData.05.svg.failing.txt rename to test/plugins/minifyPathData.05.failing.svg.txt diff --git a/test/plugins/minifyPathData.06.svg.failing.txt b/test/plugins/minifyPathData.06.failing.svg.txt similarity index 100% rename from test/plugins/minifyPathData.06.svg.failing.txt rename to test/plugins/minifyPathData.06.failing.svg.txt diff --git a/test/plugins/removeHiddenElems.03.svg.txt b/test/plugins/removeHiddenElems.03.failing.svg.txt similarity index 100% rename from test/plugins/removeHiddenElems.03.svg.txt rename to test/plugins/removeHiddenElems.03.failing.svg.txt diff --git a/test/plugins/removeHiddenElems.07.svg.txt b/test/plugins/removeHiddenElems.07.failing.svg.txt similarity index 100% rename from test/plugins/removeHiddenElems.07.svg.txt rename to test/plugins/removeHiddenElems.07.failing.svg.txt diff --git a/test/plugins/removeHiddenElems.08.svg.txt b/test/plugins/removeHiddenElems.08.failing.svg.txt similarity index 100% rename from test/plugins/removeHiddenElems.08.svg.txt rename to test/plugins/removeHiddenElems.08.failing.svg.txt diff --git a/test/plugins/removeHiddenElems.12.svg.txt b/test/plugins/removeHiddenElems.12.failing.svg.txt similarity index 100% rename from test/plugins/removeHiddenElems.12.svg.txt rename to test/plugins/removeHiddenElems.12.failing.svg.txt diff --git a/test/plugins/removeHiddenElems.13.svg.txt b/test/plugins/removeHiddenElems.13.failing.svg.txt similarity index 100% rename from test/plugins/removeHiddenElems.13.svg.txt rename to test/plugins/removeHiddenElems.13.failing.svg.txt diff --git a/test/plugins/removeHiddenElems.28.failing.svg.txt b/test/plugins/removeHiddenElems.28.failing.svg.txt new file mode 100644 index 0000000..566bb68 --- /dev/null +++ b/test/plugins/removeHiddenElems.28.failing.svg.txt @@ -0,0 +1,32 @@ +If a non-rendering element doesn't have an id, treat it the same as - remove any unreferenced children. + +=== + + + + + + + + + + + + + + + + + +@@@ + + + + + + + + + + + From 44fdad09e3ce0f0cf6e71027a14a6854ce295cd1 Mon Sep 17 00:00:00 2001 From: John Kenny Date: Sat, 5 Oct 2024 08:26:40 -0700 Subject: [PATCH 14/26] Hoist children so all have an id. --- plugins/removeHiddenElems.js | 60 +++++++++++++++++++---- test/plugins/removeHiddenElems.29.svg.txt | 36 ++++++++++++++ 2 files changed, 87 insertions(+), 9 deletions(-) create mode 100644 test/plugins/removeHiddenElems.29.svg.txt diff --git a/plugins/removeHiddenElems.js b/plugins/removeHiddenElems.js index b8e1411..6368896 100644 --- a/plugins/removeHiddenElems.js +++ b/plugins/removeHiddenElems.js @@ -81,6 +81,30 @@ export const fn = (root, params, info) => { nonRenderedElements.set(topElement, ids); } + /** + * @param {import('../lib/types.js').XastChild} child + * @returns {import('../lib/types.js').XastChild[]} + */ + function getChildrenWithIds(child) { + if (child.type !== 'element' || child.attributes.id) { + return [child]; + } + + // Preserve styles and scripts with no id. + switch (child.name) { + case 'script': + case 'style': + return [child]; + } + + // It's an element with no id; return its children which have ids. + const children = []; + for (const grandchild of child.children) { + children.push(...getChildrenWithIds(grandchild)); + } + return children; + } + /** * @param {import('../lib/types.js').XastElement} topElement * @param {Set} [checkedElements] @@ -123,6 +147,32 @@ export const fn = (root, params, info) => { return false; } + /** + * @param {import('../lib/types.js').XastElement} element + */ + function processDefsChildren(element) { + /** @type {import('../lib/types.js').XastChild[]} */ + const children = []; + + // Make sure all children of have an id; otherwise they can't be rendered. If a child doesn't have an id, delete it and move up + // its children so they are immediate children of the . + for (const child of element.children) { + children.push(...getChildrenWithIds(child)); + } + children.forEach((c) => (c.parentNode = element)); + element.children = children; + + // Any children of are hidden, regardless of whether they are non-rendering. + for (const child of children) { + if (child.type === 'element') { + if (child.name === 'style') { + continue; + } + addNonRenderedElement(child); + } + } + } + /** * @param {import('../lib/types.js').XastElement} element */ @@ -208,15 +258,7 @@ export const fn = (root, params, info) => { recordReferencedIds(element); if (element.name === 'defs') { - // Any children of are hidden, regardless of whether they are non-rendering. - for (const child of element.children) { - if (child.type === 'element') { - if (child.name === 'style') { - continue; - } - addNonRenderedElement(child); - } - } + processDefsChildren(element); return visitSkip; } diff --git a/test/plugins/removeHiddenElems.29.svg.txt b/test/plugins/removeHiddenElems.29.svg.txt new file mode 100644 index 0000000..a05c901 --- /dev/null +++ b/test/plugins/removeHiddenElems.29.svg.txt @@ -0,0 +1,36 @@ +Hoist children of so all children have id attributes. + +=== + + + + + + + + + + + + + + + + + + +@@@ + + + + + + + + + + + + + + From b347dd872e7c7734839366f8c6ddb573ce0d8df0 Mon Sep 17 00:00:00 2001 From: John Kenny Date: Sat, 5 Oct 2024 09:01:39 -0700 Subject: [PATCH 15/26] Convert non-rendering elements without an id to . --- lib/parser.js | 2 +- plugins/removeHiddenElems.js | 8 +++++++- test/plugins/inlineStyles.03.svg.txt | 2 +- test/plugins/inlineStyles.18.svg.txt | 2 +- test/plugins/prefixIds.12.svg.txt | 2 +- test/plugins/prefixIds.13.svg.txt | 4 ++-- ...ms.06.svg.txt => removeHiddenElems.06.failing.svg.txt} | 0 test/plugins/removeHiddenElems.16.svg.txt | 4 ++-- ...ms.28.failing.svg.txt => removeHiddenElems.28.svg.txt} | 4 ++-- test/plugins/removeHiddenElems.29.svg.txt | 2 ++ test/svg2js/_index.test.js | 2 +- test/svgo/entities.svg.txt | 2 +- 12 files changed, 21 insertions(+), 13 deletions(-) rename test/plugins/{removeHiddenElems.06.svg.txt => removeHiddenElems.06.failing.svg.txt} (100%) rename test/plugins/{removeHiddenElems.28.failing.svg.txt => removeHiddenElems.28.svg.txt} (98%) diff --git a/lib/parser.js b/lib/parser.js index 8d156c6..44a420e 100644 --- a/lib/parser.js +++ b/lib/parser.js @@ -167,7 +167,7 @@ export const parseSvg = (data, from) => { const node = { type: 'comment', parentNode: current, - value: foreignLevel > 0 ? comment : comment.trim(), + value: comment, }; pushToContent(node); }; diff --git a/plugins/removeHiddenElems.js b/plugins/removeHiddenElems.js index 6368896..869fb07 100644 --- a/plugins/removeHiddenElems.js +++ b/plugins/removeHiddenElems.js @@ -264,7 +264,13 @@ export const fn = (root, params, info) => { // Process non-rendering elements outside of a . if (elemsGroups.nonRendering.has(element.name)) { - addNonRenderedElement(element); + if (element.attributes.id) { + addNonRenderedElement(element); + return visitSkip; + } + // If the element doesn't have an id, it can't be referenced; but it may contain referenced elements. Change it to . + element.name = 'defs'; + processDefsChildren(element); return visitSkip; } diff --git a/test/plugins/inlineStyles.03.svg.txt b/test/plugins/inlineStyles.03.svg.txt index eb5ef2d..b3bd74a 100644 --- a/test/plugins/inlineStyles.03.svg.txt +++ b/test/plugins/inlineStyles.03.svg.txt @@ -16,7 +16,7 @@ @@@ - + diff --git a/test/plugins/removeHiddenElems.28.failing.svg.txt b/test/plugins/removeHiddenElems.28.svg.txt similarity index 98% rename from test/plugins/removeHiddenElems.28.failing.svg.txt rename to test/plugins/removeHiddenElems.28.svg.txt index 566bb68..1e7ef3e 100644 --- a/test/plugins/removeHiddenElems.28.failing.svg.txt +++ b/test/plugins/removeHiddenElems.28.svg.txt @@ -21,12 +21,12 @@ If a non-rendering element doesn't have an id, treat it the same as - rem @@@ - + - + diff --git a/test/plugins/removeHiddenElems.29.svg.txt b/test/plugins/removeHiddenElems.29.svg.txt index a05c901..c3883da 100644 --- a/test/plugins/removeHiddenElems.29.svg.txt +++ b/test/plugins/removeHiddenElems.29.svg.txt @@ -8,6 +8,7 @@ Hoist children of so all children have id attributes. + @@ -25,6 +26,7 @@ Hoist children of so all children have id attributes. + diff --git a/test/svg2js/_index.test.js b/test/svg2js/_index.test.js index bb859a2..8ef386b 100644 --- a/test/svg2js/_index.test.js +++ b/test/svg2js/_index.test.js @@ -64,7 +64,7 @@ describe('svg2js', function () { expect(root.children[1]).toMatchObject({ type: 'comment', value: - 'Generator: Adobe Illustrator 15.0.0, SVG Export Plug-In . SVG Version: 6.00 Build 0)', + ' Generator: Adobe Illustrator 15.0.0, SVG Export Plug-In . SVG Version: 6.00 Build 0) ', }); }); diff --git a/test/svgo/entities.svg.txt b/test/svgo/entities.svg.txt index cc85db3..0d7a02a 100644 --- a/test/svgo/entities.svg.txt +++ b/test/svgo/entities.svg.txt @@ -15,7 +15,7 @@ @@@ - + From dbebfa7bded98511c55f2a8af8d1d86209fc4ede Mon Sep 17 00:00:00 2001 From: John Kenny Date: Sat, 5 Oct 2024 10:11:25 -0700 Subject: [PATCH 16/26] Deprecate removeUselessDefs. --- lib/xast.js | 1 + plugins/preset-default.js | 2 -- plugins/preset-next.js | 2 -- plugins/removeUselessDefs.js | 9 +++++++++ 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/lib/xast.js b/lib/xast.js index a947f9e..64786a6 100644 --- a/lib/xast.js +++ b/lib/xast.js @@ -100,6 +100,7 @@ const visitChild = (node, visitor, parents) => { /** * @param {XastChild} node * @param {XastParent} [parentNode] + * @deprecated */ // Disable no-unused-vars until all calls to detachNodeFromParent() are updated. // eslint-disable-next-line no-unused-vars diff --git a/plugins/preset-default.js b/plugins/preset-default.js index c6313f4..7145db2 100644 --- a/plugins/preset-default.js +++ b/plugins/preset-default.js @@ -26,7 +26,6 @@ import * as removeMetadata from './removeMetadata.js'; import * as removeNonInheritableGroupAttrs from './removeNonInheritableGroupAttrs.js'; import * as removeUnknownsAndDefaults from './removeUnknownsAndDefaults.js'; import * as removeUnusedNS from './removeUnusedNS.js'; -import * as removeUselessDefs from './removeUselessDefs.js'; import * as removeUselessStrokeAndFill from './removeUselessStrokeAndFill.js'; import * as removeXMLProcInst from './removeXMLProcInst.js'; @@ -45,7 +44,6 @@ const presetDefault = createPreset({ inlineStyles, minifyStyles, cleanupIds, - removeUselessDefs, convertColors, removeUnknownsAndDefaults, removeNonInheritableGroupAttrs, diff --git a/plugins/preset-next.js b/plugins/preset-next.js index 3f05717..df3a768 100644 --- a/plugins/preset-next.js +++ b/plugins/preset-next.js @@ -26,7 +26,6 @@ import * as removeMetadata from './removeMetadata.js'; import * as removeNonInheritableGroupAttrs from './removeNonInheritableGroupAttrs.js'; import * as removeUnknownsAndDefaults from './removeUnknownsAndDefaults.js'; import * as removeUnusedNS from './removeUnusedNS.js'; -import * as removeUselessDefs from './removeUselessDefs.js'; import * as removeUselessStrokeAndFill from './removeUselessStrokeAndFill.js'; import * as removeXMLProcInst from './removeXMLProcInst.js'; @@ -45,7 +44,6 @@ const presetNext = createPreset({ inlineStyles, minifyStyles, cleanupIds, - removeUselessDefs, convertColors, removeUnknownsAndDefaults, removeNonInheritableGroupAttrs, diff --git a/plugins/removeUselessDefs.js b/plugins/removeUselessDefs.js index 5da285f..cc2351e 100644 --- a/plugins/removeUselessDefs.js +++ b/plugins/removeUselessDefs.js @@ -8,14 +8,23 @@ import { elemsGroups } from './_collections.js'; export const name = 'removeUselessDefs'; export const description = 'removes elements in without id'; +let deprecationWarning = true; + /** * Removes content of defs and properties that aren't rendered directly without ids. * * @author Lev Solntsev * * @type {import('./plugins-types.js').Plugin<'removeUselessDefs'>} + * @deprecated */ export const fn = () => { + if (deprecationWarning) { + console.warn( + 'The moveGroupAttrsToElems plugin is deprecated and will be removed in a future release.', + ); + deprecationWarning = false; + } return { element: { enter: (node, parentNode) => { From 2ebc122a041ddb03877d5ec9a4f93e7821d34b00 Mon Sep 17 00:00:00 2001 From: John Kenny Date: Sat, 5 Oct 2024 10:52:06 -0700 Subject: [PATCH 17/26] Fix issues with markers. --- plugins/removeHiddenElems.js | 24 +++-- test/fixtures/files/marker.1.svg | 48 ++++++++++ ...g.svg.txt => removeHiddenElems.08.svg.txt} | 2 +- test/plugins/removeHiddenElems.30.svg.txt | 91 +++++++++++++++++++ 4 files changed, 154 insertions(+), 11 deletions(-) create mode 100644 test/fixtures/files/marker.1.svg rename test/plugins/{removeHiddenElems.08.failing.svg.txt => removeHiddenElems.08.svg.txt} (95%) create mode 100644 test/plugins/removeHiddenElems.30.svg.txt diff --git a/plugins/removeHiddenElems.js b/plugins/removeHiddenElems.js index 869fb07..3146dcb 100644 --- a/plugins/removeHiddenElems.js +++ b/plugins/removeHiddenElems.js @@ -59,9 +59,9 @@ export const fn = (root, params, info) => { */ function processElement(element) { // If the element is an empty shape, remove it and don't process any children. - if (removeEmptyShapes(element)) { - return; - } + // if (removeEmptyShapes(element)) { + // return; + // } if (element.attributes.id) { ids.add(element.attributes.id); @@ -198,9 +198,10 @@ export const fn = (root, params, info) => { /** * @param {import('../lib/types.js').XastElement} element + * @param {Map} properties * @returns {boolean} */ - function removeEmptyShapes(element) { + function removeEmptyShapes(element, properties) { switch (element.name) { case 'ellipse': // Ellipse with zero radius @@ -220,7 +221,10 @@ export const fn = (root, params, info) => { } try { const commands = parsePathCommands(element.attributes.d, 2); - if (commands.length < 2) { + if (commands.length === 1) { + if (properties.get('marker-end') !== undefined) { + return false; + } removeElement(element); return true; } @@ -274,17 +278,17 @@ export const fn = (root, params, info) => { return visitSkip; } - if (removeEmptyShapes(element)) { + const properties = styleData.computeStyle(element, parentInfo); + if (!properties) { return; } - // Remove any rendering elements which are not visible. - - const properties = styleData.computeStyle(element, parentInfo); - if (!properties) { + if (removeEmptyShapes(element, properties)) { return; } + // Remove any rendering elements which are not visible. + const display = properties.get('display'); if ( display === 'none' && diff --git a/test/fixtures/files/marker.1.svg b/test/fixtures/files/marker.1.svg new file mode 100644 index 0000000..7b8a20c --- /dev/null +++ b/test/fixtures/files/marker.1.svg @@ -0,0 +1,48 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/test/plugins/removeHiddenElems.08.failing.svg.txt b/test/plugins/removeHiddenElems.08.svg.txt similarity index 95% rename from test/plugins/removeHiddenElems.08.failing.svg.txt rename to test/plugins/removeHiddenElems.08.svg.txt index 61b41a8..5e046c9 100644 --- a/test/plugins/removeHiddenElems.08.failing.svg.txt +++ b/test/plugins/removeHiddenElems.08.svg.txt @@ -26,13 +26,13 @@ Remove empty or single point paths without markers + - diff --git a/test/plugins/removeHiddenElems.30.svg.txt b/test/plugins/removeHiddenElems.30.svg.txt new file mode 100644 index 0000000..1dfe050 --- /dev/null +++ b/test/plugins/removeHiddenElems.30.svg.txt @@ -0,0 +1,91 @@ +Test with markers + +=== + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +@@@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + From 2f2cb544a93c0656d2dfc85143ab3d11f1ad9abe Mon Sep 17 00:00:00 2001 From: John Kenny Date: Sat, 5 Oct 2024 11:31:37 -0700 Subject: [PATCH 18/26] Preserve only comment and element children of . --- plugins/removeHiddenElems.js | 12 ++++++++++-- test/plugins/removeHiddenElems.29.svg.txt | 3 +++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/plugins/removeHiddenElems.js b/plugins/removeHiddenElems.js index 3146dcb..0b3da34 100644 --- a/plugins/removeHiddenElems.js +++ b/plugins/removeHiddenElems.js @@ -86,8 +86,16 @@ export const fn = (root, params, info) => { * @returns {import('../lib/types.js').XastChild[]} */ function getChildrenWithIds(child) { - if (child.type !== 'element' || child.attributes.id) { - return [child]; + switch (child.type) { + case 'comment': + return [child]; + case 'element': + if (child.attributes.id) { + return [child]; + } + break; + default: + return []; } // Preserve styles and scripts with no id. diff --git a/test/plugins/removeHiddenElems.29.svg.txt b/test/plugins/removeHiddenElems.29.svg.txt index c3883da..21967b1 100644 --- a/test/plugins/removeHiddenElems.29.svg.txt +++ b/test/plugins/removeHiddenElems.29.svg.txt @@ -12,6 +12,8 @@ Hoist children of so all children have id attributes. + + xxx @@ -30,6 +32,7 @@ Hoist children of so all children have id attributes. + From 891069219da9f292f2c52a3c6e733a964c5cdf3e Mon Sep 17 00:00:00 2001 From: John Kenny Date: Sat, 5 Oct 2024 13:28:32 -0700 Subject: [PATCH 19/26] Check for id rather than checking for references. --- plugins/removeHiddenElems.js | 133 ++++-------------- ... => removeHiddenElems.19.obsolete.svg.txt} | 0 ... => removeHiddenElems.20.obsolete.svg.txt} | 0 ...t => removeHiddenElems.24.failing.svg.txt} | 0 ... => removeHiddenElems.25.obsolete.svg.txt} | 0 ... => removeHiddenElems.26.obsolete.svg.txt} | 0 ... => removeHiddenElems.27.obsolete.svg.txt} | 0 test/plugins/removeHiddenElems.28.svg.txt | 5 + test/plugins/removeHiddenElems.31.svg.txt | 29 ++++ 9 files changed, 59 insertions(+), 108 deletions(-) rename test/plugins/{removeHiddenElems.19.svg.txt => removeHiddenElems.19.obsolete.svg.txt} (100%) rename test/plugins/{removeHiddenElems.20.svg.txt => removeHiddenElems.20.obsolete.svg.txt} (100%) rename test/plugins/{removeHiddenElems.24.svg.txt => removeHiddenElems.24.failing.svg.txt} (100%) rename test/plugins/{removeHiddenElems.25.svg.txt => removeHiddenElems.25.obsolete.svg.txt} (100%) rename test/plugins/{removeHiddenElems.26.svg.txt => removeHiddenElems.26.obsolete.svg.txt} (100%) rename test/plugins/{removeHiddenElems.27.svg.txt => removeHiddenElems.27.obsolete.svg.txt} (100%) create mode 100644 test/plugins/removeHiddenElems.31.svg.txt diff --git a/plugins/removeHiddenElems.js b/plugins/removeHiddenElems.js index 0b3da34..a5c1855 100644 --- a/plugins/removeHiddenElems.js +++ b/plugins/removeHiddenElems.js @@ -1,5 +1,4 @@ import { getReferencedIds } from '../lib/svgo/tools.js'; -import { visitSkip } from '../lib/xast.js'; import { elemsGroups } from './_collections.js'; import { parsePathCommands, PathParseError } from './minifyPathData.js'; @@ -33,9 +32,8 @@ export const fn = (root, params, info) => { /** @type {Map>} */ const nonRenderedElements = new Map(); - /** Associate children of non-rendering elements with the top-level element. */ - /** @type {Map} */ - const nonRenderingChildren = new Map(); + /** @type {import('../lib/types.js').XastElement[]} */ + const nonRenderingStack = []; /** * @param {string} id @@ -50,37 +48,6 @@ export const fn = (root, params, info) => { referencingElements.add(element); } - /** - * @param {import('../lib/types.js').XastElement} topElement - */ - function addNonRenderedElement(topElement) { - /** - * @param {import('../lib/types.js').XastElement} element - */ - function processElement(element) { - // If the element is an empty shape, remove it and don't process any children. - // if (removeEmptyShapes(element)) { - // return; - // } - - if (element.attributes.id) { - ids.add(element.attributes.id); - } - if (recordReferencedIds(element)) { - nonRenderingChildren.set(element, topElement); - } - for (const child of element.children) { - if (child.type === 'element') { - processElement(child); - } - } - } - - const ids = new Set(); - processElement(topElement); - nonRenderedElements.set(topElement, ids); - } - /** * @param {import('../lib/types.js').XastChild} child * @returns {import('../lib/types.js').XastChild[]} @@ -113,48 +80,6 @@ export const fn = (root, params, info) => { return children; } - /** - * @param {import('../lib/types.js').XastElement} topElement - * @param {Set} [checkedElements] - * @returns {boolean} - */ - function isReferenced(topElement, checkedElements) { - const idsInNonRenderedBranch = nonRenderedElements.get(topElement); - if (!idsInNonRenderedBranch) { - throw new Error(); - } - for (const id of idsInNonRenderedBranch) { - const referencingEls = referencedIds.get(id); - if (referencingEls) { - // See if any of the referencing nodes are referenced. - for (const el of referencingEls) { - if (el === undefined) { - // Referenced in a + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/test/plugins/removeHiddenElems.24.failing.svg.txt b/test/plugins/removeHiddenElems.24.failing.svg.txt deleted file mode 100644 index b6d06f8..0000000 --- a/test/plugins/removeHiddenElems.24.failing.svg.txt +++ /dev/null @@ -1,19 +0,0 @@ -Remove unused circular dependencies. - -=== - - - - - - - - - - -@@@ - - - - - diff --git a/test/plugins/removeHiddenElems.24.svg.txt b/test/plugins/removeHiddenElems.24.svg.txt new file mode 100644 index 0000000..c1cab40 --- /dev/null +++ b/test/plugins/removeHiddenElems.24.svg.txt @@ -0,0 +1,119 @@ +Test with ellipses. + +=== + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +@@@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/test/plugins/removeHiddenElems.25.obsolete.svg.txt b/test/plugins/removeHiddenElems.25.obsolete.svg.txt deleted file mode 100644 index b717a30..0000000 --- a/test/plugins/removeHiddenElems.25.obsolete.svg.txt +++ /dev/null @@ -1,20 +0,0 @@ -Remove unused rendering elements from . - -=== - - - - - - - - - -@@@ - - - - - - - diff --git a/test/plugins/removeHiddenElems.26.obsolete.svg.txt b/test/plugins/removeHiddenElems.26.obsolete.svg.txt deleted file mode 100644 index 76053c8..0000000 --- a/test/plugins/removeHiddenElems.26.obsolete.svg.txt +++ /dev/null @@ -1,26 +0,0 @@ -Don't remove - - - - - - -@@@ - - - - - - - - diff --git a/test/plugins/removeHiddenElems.27.obsolete.svg.txt b/test/plugins/removeHiddenElems.27.obsolete.svg.txt deleted file mode 100644 index 2620b6e..0000000 --- a/test/plugins/removeHiddenElems.27.obsolete.svg.txt +++ /dev/null @@ -1,24 +0,0 @@ -Preserve referenced path, even when path has opacity=0 and is not in . - -=== - - - - - - this is path 2 - - - - - -@@@ - - - - - - this is path 2 - - - diff --git a/test/plugins/removeHiddenElems.30.svg.txt b/test/plugins/removeHiddenElems.30.svg.txt index 1dfe050..ac72e7c 100644 --- a/test/plugins/removeHiddenElems.30.svg.txt +++ b/test/plugins/removeHiddenElems.30.svg.txt @@ -1,4 +1,4 @@ -Test with markers +Test with markers. === From 51459dba9fd3488df659a32d7fd8ac99ca4216c8 Mon Sep 17 00:00:00 2001 From: John Kenny Date: Sun, 6 Oct 2024 06:53:19 -0700 Subject: [PATCH 22/26] Don't change values if geometry properties are set in styles. --- lib/docdata.js | 8 +++++++- plugins/removeHiddenElems.js | 8 ++++++-- test/plugins/removeHiddenElems.24.svg.txt | 8 ++++++-- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/lib/docdata.js b/lib/docdata.js index 3b194d8..69a097d 100644 --- a/lib/docdata.js +++ b/lib/docdata.js @@ -119,6 +119,9 @@ export class StyleData { const hasVars = VAR_REGEXP.test(value.value); if (hasVars) { computedStyles.set(name, null); + } else if (geometryProperties[name]) { + // Support for geometry properties is inconsistent. Avoid changing these. + computedStyles.set(name, null); } else { computedStyles.set(name, value.value); if (value.important) { @@ -136,7 +139,10 @@ export class StyleData { } if (declarations) { declarations.forEach((value, name) => { - if (value.important || !importantProperties.has(name)) { + if (geometryProperties[name]) { + // Support for geometry properties is inconsistent. Avoid changing these. + computedStyles.set(name, null); + } else if (value.important || !importantProperties.has(name)) { computedStyles.set(name, value.value); } }); diff --git a/plugins/removeHiddenElems.js b/plugins/removeHiddenElems.js index 680552a..b77681a 100644 --- a/plugins/removeHiddenElems.js +++ b/plugins/removeHiddenElems.js @@ -110,12 +110,16 @@ export const fn = (root, params, info) => { } return false; case 'path': { - if (!element.attributes.d) { + const d = properties.get('d'); + if (d === null) { + return false; + } + if (!d) { removeElement(element); return true; } try { - const commands = parsePathCommands(element.attributes.d, 2); + const commands = parsePathCommands(d, 2); if (commands.length === 1) { if (properties.get('marker-end') !== undefined) { return false; diff --git a/test/plugins/removeHiddenElems.24.svg.txt b/test/plugins/removeHiddenElems.24.svg.txt index c1cab40..b2bd059 100644 --- a/test/plugins/removeHiddenElems.24.svg.txt +++ b/test/plugins/removeHiddenElems.24.svg.txt @@ -94,7 +94,9 @@ Test with ellipses. - + + + @@ -107,7 +109,9 @@ Test with ellipses. - + + + From 3930402e521b41514bb1312104f7402bf74ece0c Mon Sep 17 00:00:00 2001 From: John Kenny Date: Sun, 6 Oct 2024 07:19:52 -0700 Subject: [PATCH 23/26] Convert elements to rather than removing them directly, so any referenced elements they contain are retained. --- plugins/removeHiddenElems.js | 113 ++++++++++++---------- test/plugins/removeHiddenElems.01.svg.txt | 1 + test/plugins/removeHiddenElems.02.svg.txt | 1 + test/plugins/removeHiddenElems.25.svg.txt | 27 ++++++ 4 files changed, 90 insertions(+), 52 deletions(-) create mode 100644 test/plugins/removeHiddenElems.25.svg.txt diff --git a/plugins/removeHiddenElems.js b/plugins/removeHiddenElems.js index b77681a..08d656d 100644 --- a/plugins/removeHiddenElems.js +++ b/plugins/removeHiddenElems.js @@ -26,54 +26,6 @@ export const fn = (root, params, info) => { /** @type {import('../lib/types.js').XastElement[]} */ const nonRenderingStack = []; - /** - * @param {import('../lib/types.js').XastChild} child - * @returns {import('../lib/types.js').XastChild[]} - */ - function getChildrenWithIds(child) { - switch (child.type) { - case 'comment': - return [child]; - case 'element': - if (child.attributes.id) { - return [child]; - } - break; - default: - return []; - } - - // Preserve styles and scripts with no id. - switch (child.name) { - case 'script': - case 'style': - return [child]; - } - - // It's an element with no id; return its children which have ids. - const children = []; - for (const grandchild of child.children) { - children.push(...getChildrenWithIds(grandchild)); - } - return children; - } - - /** - * @param {import('../lib/types.js').XastElement} element - */ - function processDefsChildren(element) { - /** @type {import('../lib/types.js').XastChild[]} */ - const children = []; - - // Make sure all children of have an id; otherwise they can't be rendered. If a child doesn't have an id, delete it and move up - // its children so they are immediate children of the . - for (const child of element.children) { - children.push(...getChildrenWithIds(child)); - } - children.forEach((c) => (c.parentNode = element)); - element.children = children; - } - /** * @param {import('../lib/types.js').XastElement} element */ @@ -166,8 +118,7 @@ export const fn = (root, params, info) => { if (elemsGroups.nonRendering.has(element.name)) { if (!element.attributes.id) { // If the element doesn't have an id, it can't be referenced; but it may contain referenced elements. Change it to . - element.name = 'defs'; - processDefsChildren(element); + convertToDefs(element); } else { nonRenderingStack.push(element); } @@ -196,7 +147,7 @@ export const fn = (root, params, info) => { // markers with display: none still rendered element.name !== 'marker' ) { - removeElement(element); + convertToDefs(element); return; } @@ -204,7 +155,8 @@ export const fn = (root, params, info) => { // Don't delete elements with opacity 0 which are in a non-rendering element. const opacity = properties.get('opacity'); if (opacity === '0') { - removeElement(element); + convertToDefs(element); + return; } } }, @@ -226,3 +178,60 @@ export const fn = (root, params, info) => { }, }; }; + +/** + * @param {import('../lib/types.js').XastElement} element + */ +function convertToDefs(element) { + element.name = 'defs'; + element.attributes = {}; + processDefsChildren(element); +} + +/** + * @param {import('../lib/types.js').XastChild} child + * @returns {import('../lib/types.js').XastChild[]} + */ +function getChildrenWithIds(child) { + switch (child.type) { + case 'comment': + return [child]; + case 'element': + if (child.attributes.id) { + return [child]; + } + break; + default: + return []; + } + + // Preserve styles and scripts with no id. + switch (child.name) { + case 'script': + case 'style': + return [child]; + } + + // It's an element with no id; return its children which have ids. + const children = []; + for (const grandchild of child.children) { + children.push(...getChildrenWithIds(grandchild)); + } + return children; +} + +/** + * @param {import('../lib/types.js').XastElement} element + */ +function processDefsChildren(element) { + /** @type {import('../lib/types.js').XastChild[]} */ + const children = []; + + // Make sure all children of have an id; otherwise they can't be rendered. If a child doesn't have an id, delete it and move up + // its children so they are immediate children of the . + for (const child of element.children) { + children.push(...getChildrenWithIds(child)); + } + children.forEach((c) => (c.parentNode = element)); + element.children = children; +} diff --git a/test/plugins/removeHiddenElems.01.svg.txt b/test/plugins/removeHiddenElems.01.svg.txt index 93d8d93..4b00f00 100644 --- a/test/plugins/removeHiddenElems.01.svg.txt +++ b/test/plugins/removeHiddenElems.01.svg.txt @@ -19,6 +19,7 @@ Remove elements with display=none .a { display: block; } + diff --git a/test/plugins/removeHiddenElems.02.svg.txt b/test/plugins/removeHiddenElems.02.svg.txt index 20bc177..53e2dde 100644 --- a/test/plugins/removeHiddenElems.02.svg.txt +++ b/test/plugins/removeHiddenElems.02.svg.txt @@ -19,6 +19,7 @@ Remove elements with zero opacity .a { opacity: 0.5; } + diff --git a/test/plugins/removeHiddenElems.25.svg.txt b/test/plugins/removeHiddenElems.25.svg.txt new file mode 100644 index 0000000..c3d0d8d --- /dev/null +++ b/test/plugins/removeHiddenElems.25.svg.txt @@ -0,0 +1,27 @@ +Preserve referenced elements within display:none or opacity:0 elements. + +=== + + + + + + + + + + + + +@@@ + + + + + + + + + + + From f41125fe8673b9bdcff831c619b040f28f1241e5 Mon Sep 17 00:00:00 2001 From: John Kenny Date: Sun, 6 Oct 2024 08:22:45 -0700 Subject: [PATCH 24/26] Remove newly created empty defs in removeHiddenElems. --- plugins/removeHiddenElems.js | 22 +++++++++++++--------- test/plugins/removeHiddenElems.01.svg.txt | 1 - test/plugins/removeHiddenElems.02.svg.txt | 1 - 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/plugins/removeHiddenElems.js b/plugins/removeHiddenElems.js index 08d656d..dd704ba 100644 --- a/plugins/removeHiddenElems.js +++ b/plugins/removeHiddenElems.js @@ -26,6 +26,19 @@ export const fn = (root, params, info) => { /** @type {import('../lib/types.js').XastElement[]} */ const nonRenderingStack = []; + /** + * @param {import('../lib/types.js').XastElement} element + */ + function convertToDefs(element) { + element.name = 'defs'; + element.attributes = {}; + processDefsChildren(element); + if (element.children.length === 0) { + // If there are no children, delete the element; otherwise it may limit opportunities for compression of siblings. + removeElement(element); + } + } + /** * @param {import('../lib/types.js').XastElement} element */ @@ -179,15 +192,6 @@ export const fn = (root, params, info) => { }; }; -/** - * @param {import('../lib/types.js').XastElement} element - */ -function convertToDefs(element) { - element.name = 'defs'; - element.attributes = {}; - processDefsChildren(element); -} - /** * @param {import('../lib/types.js').XastChild} child * @returns {import('../lib/types.js').XastChild[]} diff --git a/test/plugins/removeHiddenElems.01.svg.txt b/test/plugins/removeHiddenElems.01.svg.txt index 4b00f00..93d8d93 100644 --- a/test/plugins/removeHiddenElems.01.svg.txt +++ b/test/plugins/removeHiddenElems.01.svg.txt @@ -19,7 +19,6 @@ Remove elements with display=none .a { display: block; } - diff --git a/test/plugins/removeHiddenElems.02.svg.txt b/test/plugins/removeHiddenElems.02.svg.txt index 53e2dde..20bc177 100644 --- a/test/plugins/removeHiddenElems.02.svg.txt +++ b/test/plugins/removeHiddenElems.02.svg.txt @@ -19,7 +19,6 @@ Remove elements with zero opacity .a { opacity: 0.5; } - From 4e6aa8d147823251f8e354a14b685b0d978491f5 Mon Sep 17 00:00:00 2001 From: John Kenny Date: Sun, 6 Oct 2024 13:32:33 -0700 Subject: [PATCH 25/26] Remove circles with r="0". --- plugins/removeHiddenElems.js | 31 +++++++++++++++++-- ...g.svg.txt => removeHiddenElems.03.svg.txt} | 0 2 files changed, 29 insertions(+), 2 deletions(-) rename test/plugins/{removeHiddenElems.03.failing.svg.txt => removeHiddenElems.03.svg.txt} (100%) diff --git a/plugins/removeHiddenElems.js b/plugins/removeHiddenElems.js index dd704ba..30475fe 100644 --- a/plugins/removeHiddenElems.js +++ b/plugins/removeHiddenElems.js @@ -51,6 +51,18 @@ export const fn = (root, params, info) => { childrenToDelete.add(element); } + /** + * @param {import('../lib/types.js').XastElement} element + */ + function removeUndisplayedElement(element) { + if (element.name === 'g') { + // It may contain referenced elements; treat it as . + convertToDefs(element); + } else { + removeElement(element); + } + } + /** * @param {import('../lib/types.js').XastElement} element * @param {Map} properties @@ -58,6 +70,12 @@ export const fn = (root, params, info) => { */ function removeEmptyShapes(element, properties) { switch (element.name) { + case 'circle': + if (properties.get('r') === '0' && !isAnimated(element)) { + removeElement(element); + return true; + } + return false; case 'ellipse': { // Ellipse with zero radius -- https://svgwg.org/svg2-draft/geometry.html#RxProperty @@ -160,7 +178,7 @@ export const fn = (root, params, info) => { // markers with display: none still rendered element.name !== 'marker' ) { - convertToDefs(element); + removeUndisplayedElement(element); return; } @@ -168,7 +186,7 @@ export const fn = (root, params, info) => { // Don't delete elements with opacity 0 which are in a non-rendering element. const opacity = properties.get('opacity'); if (opacity === '0') { - convertToDefs(element); + removeUndisplayedElement(element); return; } } @@ -224,6 +242,15 @@ function getChildrenWithIds(child) { return children; } +/** + * @param {import('../lib/types.js').XastElement} element + */ +function isAnimated(element) { + // TODO: fix this - it doesn't really tell whether the element is animated, and doesn't handle the case of the animation being somewhere + // besdides within the element. + return element.children.length !== 0; +} + /** * @param {import('../lib/types.js').XastElement} element */ diff --git a/test/plugins/removeHiddenElems.03.failing.svg.txt b/test/plugins/removeHiddenElems.03.svg.txt similarity index 100% rename from test/plugins/removeHiddenElems.03.failing.svg.txt rename to test/plugins/removeHiddenElems.03.svg.txt From 42f0beb507ec4145354612e18f9cbad6f6210343 Mon Sep 17 00:00:00 2001 From: John Kenny Date: Sun, 6 Oct 2024 14:40:52 -0700 Subject: [PATCH 26/26] Remove original version. --- plugins/removeHiddenElems-orig.js | 514 ------------------------------ 1 file changed, 514 deletions(-) delete mode 100644 plugins/removeHiddenElems-orig.js diff --git a/plugins/removeHiddenElems-orig.js b/plugins/removeHiddenElems-orig.js deleted file mode 100644 index 42891d9..0000000 --- a/plugins/removeHiddenElems-orig.js +++ /dev/null @@ -1,514 +0,0 @@ -/** - * @typedef {import('../lib/types.js').XastChild} XastChild - * @typedef {import('../lib/types.js').XastElement} XastElement - * @typedef {import('../lib/types.js').XastParent} XastParent - */ - -import { elemsGroups } from './_collections.js'; -import { querySelector, detachNodeFromParent } from '../lib/xast.js'; -import { parsePathData } from '../lib/path.js'; -import { findReferences } from '../lib/svgo/tools.js'; - -const nonRendering = elemsGroups.nonRendering; - -export const name = 'removeHiddenElems'; -export const description = - 'removes hidden elements (zero sized, with absent attributes)'; - -/** - * Remove hidden elements with disabled rendering: - * - display="none" - * - opacity="0" - * - circle with zero radius - * - ellipse with zero x-axis or y-axis radius - * - rectangle with zero width or height - * - pattern with zero width or height - * - image with zero width or height - * - path with empty data - * - polyline with empty points - * - polygon with empty points - * - * @author Kir Belevich - * - * @type {import('./plugins-types.js').Plugin<'removeHiddenElems'>} - */ -export const fn = (root, params, info) => { - const { - isHidden = true, - displayNone = true, - opacity0 = true, - circleR0 = true, - ellipseRX0 = true, - ellipseRY0 = true, - rectWidth0 = true, - rectHeight0 = true, - patternWidth0 = true, - patternHeight0 = true, - imageWidth0 = true, - imageHeight0 = true, - pathEmptyD = true, - polylineEmptyPoints = true, - polygonEmptyPoints = true, - } = params; - - const styleData = info.docData.getStyles(); - if ( - info.docData.hasScripts() || - styleData === null || - !styleData.hasOnlyFeatures(['simple-selectors', 'attribute-selectors']) - ) { - return; - } - - let inNonRenderingNode = 0; - - /** - * Skip non-rendered nodes initially, and only detach if they have no ID, or - * their ID is not referenced by another node. - * - * @type {Set} - */ - const nonRenderedNodes = new Set(); - - /** - * IDs for removed hidden definitions. - * - * @type {Set} - */ - const removedDefIds = new Set(); - - /** @type {Set} */ - const defNodesToRemove = new Set(); - - /** - * @type {Set} - */ - const allDefs = new Set(); - - /** @type {Map>} */ - const allReferences = new Map(); - - /** @type {Map} */ - const referencedIdsByNode = new Map(); - - /** - * @type {Map} - */ - const referencesById = new Map(); - - /** - * Nodes can't be removed if they or any of their children have an id attribute that is referenced. - * @param {XastElement} node - * @returns boolean - */ - function canRemoveNonRenderingNode(node) { - const refs = allReferences.get(node.attributes.id); - if (refs && refs.size) { - return false; - } - for (const child of node.children) { - if (child.type === 'element' && !canRemoveNonRenderingNode(child)) { - return false; - } - } - return true; - } - - /** - * Retrieve information about all IDs referenced by an element and its children. - * @param {XastElement} node - * @returns {XastElement[]} - */ - function getNodesReferencedByBranch(node) { - const allIds = []; - const thisNodeIds = referencedIdsByNode.get(node); - if (thisNodeIds) { - allIds.push(node); - } - for (const child of node.children) { - if (child.type === 'element') { - allIds.push(...getNodesReferencedByBranch(child)); - } - } - return allIds; - } - - /** - * @param {string} referencedId - * @param {XastElement} referencingElement - */ - function recordReference(referencedId, referencingElement) { - const refs = allReferences.get(referencedId); - if (refs) { - refs.add(referencingElement); - } else { - allReferences.set(referencedId, new Set([referencingElement])); - } - } - - /** - * @param {XastElement} node - * @param {XastParent} parentNode - */ - function removeElement(node, parentNode) { - if ( - node.type === 'element' && - node.attributes.id != null && - parentNode.type === 'element' && - parentNode.name === 'defs' - ) { - removedDefIds.add(node.attributes.id); - } - - defNodesToRemove.add(node); - } - - // Record all references in the style element. - const styleElement = styleData.getFirstStyleElement(); - if (styleElement) { - for (const id of styleData.getIdsReferencedByProperties()) { - recordReference(id, styleElement); - } - } - - return { - element: { - enter: (node, parentNode, parentInfo) => { - if (nonRendering.has(node.name)) { - nonRenderedNodes.add(node); - inNonRenderingNode++; - } - const computedStyle = styleData.computeStyle(node, parentInfo); - const opacity = computedStyle.get('opacity'); - // https://www.w3.org/TR/SVG11/masking.html#ObjectAndGroupOpacityProperties - if (opacity0 && opacity === '0') { - if (!inNonRenderingNode) { - if (node.name === 'path') { - // It's possible this will be referenced in a . - nonRenderedNodes.add(node); - } else { - removeElement(node, parentNode); - return; - } - } - } - - if (node.name === 'defs') { - allDefs.add(node); - } - - if (node.name === 'use') { - for (const attr of Object.keys(node.attributes)) { - if (attr !== 'href' && !attr.endsWith(':href')) continue; - const value = node.attributes[attr]; - const id = value.slice(1); - - let refs = referencesById.get(id); - if (!refs) { - refs = []; - referencesById.set(id, refs); - } - refs.push(node); - } - } - - // Removes hidden elements - // https://www.w3schools.com/cssref/pr_class_visibility.asp - const visibility = computedStyle.get('visibility'); - if ( - isHidden && - visibility === '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" - const display = computedStyle.get('display'); - if ( - displayNone && - display === '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 - // "A value of zero disables rendering of the element" - // - // - if ( - circleR0 && - node.name === 'circle' && - node.children.length === 0 && - node.attributes.r === '0' - ) { - removeElement(node, parentNode); - return; - } - - // Ellipse with zero x-axis radius - // - // https://www.w3.org/TR/SVG11/shapes.html#EllipseElementRXAttribute - // "A value of zero disables rendering of the element" - // - // - if ( - ellipseRX0 && - node.name === 'ellipse' && - node.children.length === 0 && - node.attributes.rx === '0' - ) { - removeElement(node, parentNode); - return; - } - - // Ellipse with zero y-axis radius - // - // https://www.w3.org/TR/SVG11/shapes.html#EllipseElementRYAttribute - // "A value of zero disables rendering of the element" - // - // - if ( - ellipseRY0 && - node.name === 'ellipse' && - node.children.length === 0 && - node.attributes.ry === '0' - ) { - removeElement(node, parentNode); - return; - } - - // Rectangle with zero width - // - // https://www.w3.org/TR/SVG11/shapes.html#RectElementWidthAttribute - // "A value of zero disables rendering of the element" - // - // - if ( - rectWidth0 && - node.name === 'rect' && - node.children.length === 0 && - node.attributes.width === '0' - ) { - removeElement(node, parentNode); - return; - } - - // Rectangle with zero height - // - // https://www.w3.org/TR/SVG11/shapes.html#RectElementHeightAttribute - // "A value of zero disables rendering of the element" - // - // - if ( - rectHeight0 && - rectWidth0 && - node.name === 'rect' && - node.children.length === 0 && - node.attributes.height === '0' - ) { - removeElement(node, parentNode); - return; - } - - // Pattern with zero width - // - // https://www.w3.org/TR/SVG11/pservers.html#PatternElementWidthAttribute - // "A value of zero disables rendering of the element (i.e., no paint is applied)" - // - // - if ( - patternWidth0 && - node.name === 'pattern' && - node.attributes.width === '0' - ) { - removeElement(node, parentNode); - return; - } - - // Pattern with zero height - // - // https://www.w3.org/TR/SVG11/pservers.html#PatternElementHeightAttribute - // "A value of zero disables rendering of the element (i.e., no paint is applied)" - // - // - if ( - patternHeight0 && - node.name === 'pattern' && - node.attributes.height === '0' - ) { - removeElement(node, parentNode); - return; - } - - // Image with zero width - // - // https://www.w3.org/TR/SVG11/struct.html#ImageElementWidthAttribute - // "A value of zero disables rendering of the element" - // - // - if ( - imageWidth0 && - node.name === 'image' && - node.attributes.width === '0' - ) { - removeElement(node, parentNode); - return; - } - - // Image with zero height - // - // https://www.w3.org/TR/SVG11/struct.html#ImageElementHeightAttribute - // "A value of zero disables rendering of the element" - // - // - if ( - imageHeight0 && - node.name === 'image' && - node.attributes.height === '0' - ) { - 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.has('marker-start') && - !computedStyle.has('marker-end') - ) { - removeElement(node, parentNode); - return; - } - } - - // Polyline with empty points - // - // https://www.w3.org/TR/SVG11/shapes.html#PolylineElementPointsAttribute - // - // - if ( - polylineEmptyPoints && - node.name === 'polyline' && - node.attributes.points == null - ) { - removeElement(node, parentNode); - return; - } - - // Polygon with empty points - // - // https://www.w3.org/TR/SVG11/shapes.html#PolygonElementPointsAttribute - // - // - if ( - polygonEmptyPoints && - node.name === 'polygon' && - node.attributes.points == null - ) { - removeElement(node, parentNode); - return; - } - - const allIds = []; - for (const [name, value] of Object.entries(node.attributes)) { - const ids = findReferences(name, value); - - // Record which other nodes are referenced by this node. - for (const id of ids) { - allIds.push(id); - recordReference(id, node); - } - } - - // Record which ids are referenced by this node. - if (allIds.length) { - referencedIdsByNode.set(node, allIds); - } - }, - exit: (node) => { - if (nonRendering.has(node.name)) { - inNonRenderingNode--; - } - }, - }, - root: { - exit: () => { - for (const child of defNodesToRemove) { - detachNodeFromParent(child); - nonRenderedNodes.delete(child); - } - - for (const id of removedDefIds) { - const refs = referencesById.get(id); - if (refs) { - for (const node of refs) { - detachNodeFromParent(node); - } - } - } - - let tryAgain; - do { - tryAgain = false; - for (const nonRenderedNode of nonRenderedNodes) { - if (canRemoveNonRenderingNode(nonRenderedNode)) { - detachNodeFromParent(nonRenderedNode); - nonRenderedNodes.delete(nonRenderedNode); - - // For any elements referenced by the just-deleted node and its children, remove the node from the list of referencing nodes. - const deletedReferenceNodes = - getNodesReferencedByBranch(nonRenderedNode); - for (const deletedNode of deletedReferenceNodes) { - const referencedIds = referencedIdsByNode.get(deletedNode); - if (referencedIds) { - for (const id of referencedIds) { - const referencingNodes = allReferences.get(id); - if (referencingNodes) { - referencingNodes.delete(deletedNode); - if (referencingNodes.size === 0) { - tryAgain = true; - } - } - } - } - } - } - } - } while (tryAgain); - - for (const node of allDefs) { - if (node.children.length === 0) { - detachNodeFromParent(node); - } - } - }, - }, - }; -};