From 90b3577d8da4b5f4d37605035f8bc8d85a7c8048 Mon Sep 17 00:00:00 2001 From: Mira Grudzinska Date: Mon, 14 Jul 2025 23:18:22 +0200 Subject: [PATCH] svg: fix nested use nodes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, only single-level references were supported. If a node pointed to another element that itself contained a node, the reference wasn’t resolved. This has been fixed by replacing the array of postponed elements with a list. The list is traversed, and nodes aren’t cloned while they or any of their children remain unresolved. In such cases, the target element also gets added to the list, enabling recursive resolution of nested href references. issue: https://github.com/thorvg/thorvg/issues/3615 --- src/loaders/svg/tvgSvgLoader.cpp | 89 ++++++++++++++++++++-------- src/loaders/svg/tvgSvgLoaderCommon.h | 5 +- 2 files changed, 69 insertions(+), 25 deletions(-) diff --git a/src/loaders/svg/tvgSvgLoader.cpp b/src/loaders/svg/tvgSvgLoader.cpp index 952c9efd..bb922f59 100644 --- a/src/loaders/svg/tvgSvgLoader.cpp +++ b/src/loaders/svg/tvgSvgLoader.cpp @@ -894,12 +894,6 @@ error: } -static void _postpone(Array& nodes, SvgNode *node, char* id) -{ - nodes.push({node, id}); -} - - static bool _attrParseSvgNode(void* data, const char* key, const char* value) { SvgLoaderData* loader = (SvgLoaderData*)data; @@ -1157,7 +1151,7 @@ static void _handleCssClassAttr(SvgLoaderData* loader, SvgNode* node, const char cssCopyStyleAttr(node, cssNode); } - if (!cssClassFound) _postpone(loader->nodesToStyle, node, *cssClass); + if (!cssClassFound) loader->nodesToStyle.push({node, *cssClass}); } @@ -2155,6 +2149,23 @@ static SvgNode* _findParentById(SvgNode* node, char* id, SvgNode* doc) } +static bool _checkPostponed(SvgNode* node, SvgNode* cloneNode, int depth) +{ + if (node == cloneNode) return true; + + if (depth == 512) { + TVGERR("SVG", "Infinite recursive call - stopped after %d calls! Svg file may be incorrectly formatted.", depth); + return false; + } + + ARRAY_FOREACH(p, node->child) { + if (_checkPostponed(*p, cloneNode, depth + 1)) return true; + } + + return false; +} + + static constexpr struct { const char* tag; @@ -2196,17 +2207,30 @@ static bool _attrParseUseNode(void* data, const char* key, const char* value) nodeFrom = _findNodeById(defs, id); if (nodeFrom) { if (!_findParentById(node, id, loader->doc)) { - _cloneNode(nodeFrom, node, 0); - if (nodeFrom->type == SvgNodeType::Symbol) use->symbol = nodeFrom; + //Check if none of nodeFrom's children are in the cloneNodes list + auto postpone = false; + INLIST_FOREACH(loader->cloneNodes, pair) { + if (_checkPostponed(nodeFrom, pair->node, 1)) { + postpone = true; + loader->cloneNodes.back(new(tvg::malloc(sizeof(SvgNodeIdPair))) SvgNodeIdPair(node, id)); + break; + } + } + //None of the children of nodeFrom are on the cloneNodes list, so it can be cloned immediately + if (!postpone) { + _cloneNode(nodeFrom, node, 0); + if (nodeFrom->type == SvgNodeType::Symbol) use->symbol = nodeFrom; + tvg::free(id); + } } else { TVGLOG("SVG", "%s is ancestor element. This reference is invalid.", id); + tvg::free(id); } - tvg::free(id); } else { //some svg export software include element at the end of the file //if so the 'from' element won't be found now and we have to repeat finding //after the whole file is parsed - _postpone(loader->cloneNodes, node, id); + loader->cloneNodes.back(new(tvg::malloc(sizeof(SvgNodeIdPair))) SvgNodeIdPair(node, id)); } } else { return _attrParseGNode(data, key, value); @@ -3356,22 +3380,39 @@ static void _cloneNode(SvgNode* from, SvgNode* parent, int depth) } -static void _clonePostponedNodes(Array* cloneNodes, SvgNode* doc) +static void _clonePostponedNodes(Inlist* cloneNodes, SvgNode* doc) { - ARRAY_FOREACH(p, *cloneNodes) { - auto nodeIdPair = *p; - auto defs = _getDefsNode(nodeIdPair.node); - auto nodeFrom = _findNodeById(defs, nodeIdPair.id); - if (!nodeFrom) nodeFrom = _findNodeById(doc, nodeIdPair.id); - if (!_findParentById(nodeIdPair.node, nodeIdPair.id, doc)) { - _cloneNode(nodeFrom, nodeIdPair.node, 0); - if (nodeFrom && nodeFrom->type == SvgNodeType::Symbol && nodeIdPair.node->type == SvgNodeType::Use) { - nodeIdPair.node->node.use.symbol = nodeFrom; + auto nodeIdPair = cloneNodes->front(); + while (nodeIdPair) { + if (!_findParentById(nodeIdPair->node, nodeIdPair->id, doc)) { + //Check if none of nodeFrom's children are in the cloneNodes list + auto postpone = false; + auto nodeFrom = _findNodeById(_getDefsNode(nodeIdPair->node), nodeIdPair->id); + if (!nodeFrom) nodeFrom = _findNodeById(doc, nodeIdPair->id); + if (nodeFrom) { + INLIST_FOREACH((*cloneNodes), pair) { + if (_checkPostponed(nodeFrom, pair->node, 1)) { + postpone = true; + cloneNodes->back(nodeIdPair); + break; + } + } + } + //Since none of the child nodes of nodeFrom are present in the cloneNodes list, it can be cloned immediately + if (!postpone) { + _cloneNode(nodeFrom, nodeIdPair->node, 0); + if (nodeFrom && nodeFrom->type == SvgNodeType::Symbol && nodeIdPair->node->type == SvgNodeType::Use) { + nodeIdPair->node->node.use.symbol = nodeFrom; + } + tvg::free(nodeIdPair->id); + tvg::free(nodeIdPair); } } else { - TVGLOG("SVG", "%s is ancestor element. This reference is invalid.", nodeIdPair.id); + TVGLOG("SVG", "%s is ancestor element. This reference is invalid.", nodeIdPair->id); + tvg::free(nodeIdPair->id); + tvg::free(nodeIdPair); } - tvg::free(nodeIdPair.id); + nodeIdPair = cloneNodes->front(); } } @@ -3901,7 +3942,7 @@ void SvgLoader::run(unsigned tid) if (loaderData.nodesToStyle.count > 0) cssApplyStyleToPostponeds(loaderData.nodesToStyle, loaderData.cssStyle); if (loaderData.cssStyle) cssUpdateStyle(loaderData.doc, loaderData.cssStyle); - if (loaderData.cloneNodes.count > 0) _clonePostponedNodes(&loaderData.cloneNodes, loaderData.doc); + if (!loaderData.cloneNodes.empty()) _clonePostponedNodes(&loaderData.cloneNodes, loaderData.doc); _updateComposite(loaderData.doc, loaderData.doc); if (defs) _updateComposite(loaderData.doc, defs); diff --git a/src/loaders/svg/tvgSvgLoaderCommon.h b/src/loaders/svg/tvgSvgLoaderCommon.h index e3b917a8..e439a16e 100644 --- a/src/loaders/svg/tvgSvgLoaderCommon.h +++ b/src/loaders/svg/tvgSvgLoaderCommon.h @@ -25,6 +25,7 @@ #include "tvgCommon.h" #include "tvgArray.h" +#include "tvgInlist.h" struct Box { @@ -564,6 +565,8 @@ struct SvgParser struct SvgNodeIdPair { + INLIST_ITEM(SvgNodeIdPair); + SvgNodeIdPair(SvgNode* n, char* i) : node{n}, id{i} {} SvgNode* node; char *id; }; @@ -592,7 +595,7 @@ struct SvgLoaderData Array gradients; Array gradientStack; //For stops SvgParser* svgParse = nullptr; - Array cloneNodes; + Inlist cloneNodes; Array nodesToStyle; Array images; //embedded images Array fonts;