From 124876833a3c0aed40883045dce80046b69a466c Mon Sep 17 00:00:00 2001 From: John Kenny Date: Sun, 4 Feb 2024 18:39:13 -0800 Subject: [PATCH 1/3] Fix for removeHiddenElems is not idempotent. --- plugins/removeHiddenElems.js | 81 ++++++++++++++++++++--- test/plugins/removeHiddenElems.20.svg.txt | 24 +++++++ 2 files changed, 94 insertions(+), 11 deletions(-) create mode 100644 test/plugins/removeHiddenElems.20.svg.txt diff --git a/plugins/removeHiddenElems.js b/plugins/removeHiddenElems.js index 79d51504e..65eade7a8 100644 --- a/plugins/removeHiddenElems.js +++ b/plugins/removeHiddenElems.js @@ -78,8 +78,11 @@ export const fn = (root, params) => { */ const allDefs = new Map(); - /** @type {Set} */ - const allReferences = new Set(); + /** @type {Map>} */ + const allReferences = new Map(); + + /** @type {Map} */ + const referencedIdsByNode = new Map(); /** * @type {Map>} @@ -97,7 +100,8 @@ export const fn = (root, params) => { * @returns boolean */ function canRemoveNonRenderingNode(node) { - if (allReferences.has(node.attributes.id)) { + const refs = allReferences.get(node.attributes.id); + if (refs && refs.size) { return false; } for (const child of node.children) { @@ -108,6 +112,28 @@ export const fn = (root, params) => { return true; } + /** + * Retrieve information about all IDs referenced by an element and its children. + * @param {XastElement} node + * @returns {Array<{node:XastElement,refId:string}>} + */ + function getIdsReferencedByBranch(node) { + const allIds = []; + const thisNodeIds = referencedIdsByNode.get(node); + if (thisNodeIds) { + const refs = thisNodeIds.map((e) => { + return { node: node, refId: e }; + }); + allIds.push(...refs); + } + for (const child of node.children) { + if (child.type === 'element') { + allIds.push(...getIdsReferencedByBranch(child)); + } + } + return allIds; + } + /** * @param {XastChild} node * @param {XastParent} parentNode @@ -410,13 +436,26 @@ export const fn = (root, params) => { 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) { - allReferences.add(id); + allIds.push(id); + const refs = allReferences.get(id); + if (refs) { + refs.add(node); + } else { + allReferences.set(id, new Set([node])); + } } } + + // Record which ids are referenced by this node. + if (allIds.length) { + referencedIdsByNode.set(node, allIds); + } }, }, root: { @@ -431,14 +470,34 @@ export const fn = (root, params) => { } if (!deoptimized) { - for (const [ - nonRenderedNode, - nonRenderedParent, - ] of nonRenderedNodes.entries()) { - if (canRemoveNonRenderingNode(nonRenderedNode)) { - detachNodeFromParent(nonRenderedNode, nonRenderedParent); + let tryAgain; + do { + tryAgain = false; + for (const [ + nonRenderedNode, + nonRenderedParent, + ] of nonRenderedNodes.entries()) { + if (canRemoveNonRenderingNode(nonRenderedNode)) { + detachNodeFromParent(nonRenderedNode, nonRenderedParent); + + // For any elements referenced by the just-deleted node, remove the node from the list of referencing nodes. + const referencedIds = getIdsReferencedByBranch(nonRenderedNode); + if (referencedIds) { + for (const refInfo of referencedIds) { + const refs = allReferences.get(refInfo.refId); + if (refs) { + refs.delete(refInfo.node); + if (refs.size === 0) { + tryAgain = true; + } + } + } + } + + nonRenderedNodes.delete(nonRenderedNode); + } } - } + } while (tryAgain); } for (const [node, parentNode] of allDefs.entries()) { diff --git a/test/plugins/removeHiddenElems.20.svg.txt b/test/plugins/removeHiddenElems.20.svg.txt new file mode 100644 index 000000000..524807d30 --- /dev/null +++ b/test/plugins/removeHiddenElems.20.svg.txt @@ -0,0 +1,24 @@ +Remove all unreferenced elements which are only referenced by other unreferenced elements. + +=== + + + + + + + + + + + + + + + +@@@ + + + + From b01ede0e171995b0d0fd58e998f90414a5b4f93a Mon Sep 17 00:00:00 2001 From: John Kenny Date: Sun, 4 Feb 2024 18:57:08 -0800 Subject: [PATCH 2/3] Removed redundant bookkeeping. --- plugins/removeHiddenElems.js | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/plugins/removeHiddenElems.js b/plugins/removeHiddenElems.js index 65eade7a8..c2f7a4583 100644 --- a/plugins/removeHiddenElems.js +++ b/plugins/removeHiddenElems.js @@ -115,20 +115,17 @@ export const fn = (root, params) => { /** * Retrieve information about all IDs referenced by an element and its children. * @param {XastElement} node - * @returns {Array<{node:XastElement,refId:string}>} + * @returns {XastElement[]} */ - function getIdsReferencedByBranch(node) { + function getNodesReferencedByBranch(node) { const allIds = []; const thisNodeIds = referencedIdsByNode.get(node); if (thisNodeIds) { - const refs = thisNodeIds.map((e) => { - return { node: node, refId: e }; - }); - allIds.push(...refs); + allIds.push(node); } for (const child of node.children) { if (child.type === 'element') { - allIds.push(...getIdsReferencedByBranch(child)); + allIds.push(...getNodesReferencedByBranch(child)); } } return allIds; @@ -480,15 +477,19 @@ export const fn = (root, params) => { if (canRemoveNonRenderingNode(nonRenderedNode)) { detachNodeFromParent(nonRenderedNode, nonRenderedParent); - // For any elements referenced by the just-deleted node, remove the node from the list of referencing nodes. - const referencedIds = getIdsReferencedByBranch(nonRenderedNode); - if (referencedIds) { - for (const refInfo of referencedIds) { - const refs = allReferences.get(refInfo.refId); - if (refs) { - refs.delete(refInfo.node); - if (refs.size === 0) { - tryAgain = true; + // 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; + } } } } From d9912406066255989db731713bbe5a64df6e61d1 Mon Sep 17 00:00:00 2001 From: John Kenny Date: Mon, 5 Feb 2024 06:02:35 -0800 Subject: [PATCH 3/3] Delete node from nonRenderedNodes after detaching. --- plugins/removeHiddenElems.js | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/removeHiddenElems.js b/plugins/removeHiddenElems.js index c2f7a4583..f165cd54a 100644 --- a/plugins/removeHiddenElems.js +++ b/plugins/removeHiddenElems.js @@ -476,6 +476,7 @@ export const fn = (root, params) => { ] of nonRenderedNodes.entries()) { if (canRemoveNonRenderingNode(nonRenderedNode)) { detachNodeFromParent(nonRenderedNode, nonRenderedParent); + 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 =