svg: fix nested use nodes
Some checks are pending
Android / build_x86_64 (push) Waiting to run
Android / build_aarch64 (push) Waiting to run
iOS / build_x86_64 (push) Waiting to run
iOS / build_arm64 (push) Waiting to run
macOS / build (push) Waiting to run
macOS / compact_test (push) Waiting to run
macOS / unit_test (push) Waiting to run
Ubuntu / build (push) Waiting to run
Ubuntu / compact_test (push) Waiting to run
Ubuntu / unit_test (push) Waiting to run
Windows / build (push) Waiting to run
Windows / compact_test (push) Waiting to run
Windows / unit_test (push) Waiting to run

Previously, only single-level <use> references were supported.
If a <use> node pointed to another element that itself contained
a <use> 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
This commit is contained in:
Mira Grudzinska 2025-07-14 23:18:22 +02:00 committed by Hermet Park
parent d44098180c
commit d8bbb0df31
2 changed files with 69 additions and 25 deletions

View file

@ -894,12 +894,6 @@ error:
} }
static void _postpone(Array<SvgNodeIdPair>& nodes, SvgNode *node, char* id)
{
nodes.push({node, id});
}
static bool _attrParseSvgNode(void* data, const char* key, const char* value) static bool _attrParseSvgNode(void* data, const char* key, const char* value)
{ {
SvgLoaderData* loader = (SvgLoaderData*)data; SvgLoaderData* loader = (SvgLoaderData*)data;
@ -1157,7 +1151,7 @@ static void _handleCssClassAttr(SvgLoaderData* loader, SvgNode* node, const char
cssCopyStyleAttr(node, cssNode); 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 static constexpr struct
{ {
const char* tag; const char* tag;
@ -2196,17 +2207,30 @@ static bool _attrParseUseNode(void* data, const char* key, const char* value)
nodeFrom = _findNodeById(defs, id); nodeFrom = _findNodeById(defs, id);
if (nodeFrom) { if (nodeFrom) {
if (!_findParentById(node, id, loader->doc)) { if (!_findParentById(node, id, loader->doc)) {
_cloneNode(nodeFrom, node, 0); //Check if none of nodeFrom's children are in the cloneNodes list
if (nodeFrom->type == SvgNodeType::Symbol) use->symbol = nodeFrom; auto postpone = false;
INLIST_FOREACH(loader->cloneNodes, pair) {
if (_checkPostponed(nodeFrom, pair->node, 1)) {
postpone = true;
loader->cloneNodes.back(new(tvg::malloc<SvgNodeIdPair*>(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 { } else {
TVGLOG("SVG", "%s is ancestor element. This reference is invalid.", id); TVGLOG("SVG", "%s is ancestor element. This reference is invalid.", id);
tvg::free(id);
} }
tvg::free(id);
} else { } else {
//some svg export software include <defs> element at the end of the file //some svg export software include <defs> element at the end of the file
//if so the 'from' element won't be found now and we have to repeat finding //if so the 'from' element won't be found now and we have to repeat finding
//after the whole file is parsed //after the whole file is parsed
_postpone(loader->cloneNodes, node, id); loader->cloneNodes.back(new(tvg::malloc<SvgNodeIdPair*>(sizeof(SvgNodeIdPair))) SvgNodeIdPair(node, id));
} }
} else { } else {
return _attrParseGNode(data, key, value); return _attrParseGNode(data, key, value);
@ -3356,22 +3380,39 @@ static void _cloneNode(SvgNode* from, SvgNode* parent, int depth)
} }
static void _clonePostponedNodes(Array<SvgNodeIdPair>* cloneNodes, SvgNode* doc) static void _clonePostponedNodes(Inlist<SvgNodeIdPair>* cloneNodes, SvgNode* doc)
{ {
ARRAY_FOREACH(p, *cloneNodes) { auto nodeIdPair = cloneNodes->front();
auto nodeIdPair = *p; while (nodeIdPair) {
auto defs = _getDefsNode(nodeIdPair.node); if (!_findParentById(nodeIdPair->node, nodeIdPair->id, doc)) {
auto nodeFrom = _findNodeById(defs, nodeIdPair.id); //Check if none of nodeFrom's children are in the cloneNodes list
if (!nodeFrom) nodeFrom = _findNodeById(doc, nodeIdPair.id); auto postpone = false;
if (!_findParentById(nodeIdPair.node, nodeIdPair.id, doc)) { auto nodeFrom = _findNodeById(_getDefsNode(nodeIdPair->node), nodeIdPair->id);
_cloneNode(nodeFrom, nodeIdPair.node, 0); if (!nodeFrom) nodeFrom = _findNodeById(doc, nodeIdPair->id);
if (nodeFrom && nodeFrom->type == SvgNodeType::Symbol && nodeIdPair.node->type == SvgNodeType::Use) { if (nodeFrom) {
nodeIdPair.node->node.use.symbol = 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 { } 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.nodesToStyle.count > 0) cssApplyStyleToPostponeds(loaderData.nodesToStyle, loaderData.cssStyle);
if (loaderData.cssStyle) cssUpdateStyle(loaderData.doc, 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); _updateComposite(loaderData.doc, loaderData.doc);
if (defs) _updateComposite(loaderData.doc, defs); if (defs) _updateComposite(loaderData.doc, defs);

View file

@ -25,6 +25,7 @@
#include "tvgCommon.h" #include "tvgCommon.h"
#include "tvgArray.h" #include "tvgArray.h"
#include "tvgInlist.h"
struct Box struct Box
{ {
@ -564,6 +565,8 @@ struct SvgParser
struct SvgNodeIdPair struct SvgNodeIdPair
{ {
INLIST_ITEM(SvgNodeIdPair);
SvgNodeIdPair(SvgNode* n, char* i) : node{n}, id{i} {}
SvgNode* node; SvgNode* node;
char *id; char *id;
}; };
@ -592,7 +595,7 @@ struct SvgLoaderData
Array<SvgStyleGradient*> gradients; Array<SvgStyleGradient*> gradients;
Array<SvgStyleGradient*> gradientStack; //For stops Array<SvgStyleGradient*> gradientStack; //For stops
SvgParser* svgParse = nullptr; SvgParser* svgParse = nullptr;
Array<SvgNodeIdPair> cloneNodes; Inlist<SvgNodeIdPair> cloneNodes;
Array<SvgNodeIdPair> nodesToStyle; Array<SvgNodeIdPair> nodesToStyle;
Array<char*> images; //embedded images Array<char*> images; //embedded images
Array<FontFace> fonts; Array<FontFace> fonts;