From 907c2db46915eedac02f2a5a26291d2ce19282f9 Mon Sep 17 00:00:00 2001 From: Eirik Aavitsland Date: Fri, 10 Aug 2018 13:25:11 +0200 Subject: Fix parsing of forward referrals in use elements SVG elements may refer to a node that is defined later in the file. The parser would then fail to resolve the link. Implement delayed link resolution in the parser to handle such cases. Task-number: QTBUG-69694 Change-Id: I94ffb3511dbd0fac822bb56991415b2d99ccf90e Reviewed-by: Friedemann Kleint --- src/svg/qsvggraphics_p.h | 7 ++++ src/svg/qsvghandler.cpp | 62 +++++++++++++++++++++------- src/svg/qsvghandler_p.h | 1 + tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp | 45 +++++++++++++++++--- 4 files changed, 95 insertions(+), 20 deletions(-) diff --git a/src/svg/qsvggraphics_p.h b/src/svg/qsvggraphics_p.h index 33b5154..6e5b9d6 100644 --- a/src/svg/qsvggraphics_p.h +++ b/src/svg/qsvggraphics_p.h @@ -237,13 +237,20 @@ class QSvgUse : public QSvgNode { public: QSvgUse(const QPointF &start, QSvgNode *parent, QSvgNode *link); + QSvgUse(const QPointF &start, QSvgNode *parent, const QString &linkId) + : QSvgUse(start, parent, nullptr) + { m_linkId = linkId; } void draw(QPainter *p, QSvgExtraStates &states) override; Type type() const override; QRectF bounds(QPainter *p, QSvgExtraStates &states) const override; + bool isResolved() const { return m_link != nullptr; } + QString linkId() const { return m_linkId; } + void setLink(QSvgNode *link) { m_link = link; } private: QSvgNode *m_link; QPointF m_start; + QString m_linkId; }; class QSvgVideo : public QSvgNode diff --git a/src/svg/qsvghandler.cpp b/src/svg/qsvghandler.cpp index b625d3f..5242ffe 100644 --- a/src/svg/qsvghandler.cpp +++ b/src/svg/qsvghandler.cpp @@ -3359,29 +3359,30 @@ static QSvgNode *createUseNode(QSvgNode *parent, } if (group) { + QPointF pt; + if (!xStr.isNull() || !yStr.isNull()) { + QSvgHandler::LengthType type; + qreal nx = parseLength(xStr, type, handler); + nx = convertToPixels(nx, true, type); + + qreal ny = parseLength(yStr, type, handler); + ny = convertToPixels(ny, true, type); + pt = QPointF(nx, ny); + } + QSvgNode *link = group->scopeNode(linkId); if (link) { if (parent->isDescendantOf(link)) qCWarning(lcSvgHandler, "link #%s is recursive!", qPrintable(linkId)); - QPointF pt; - if (!xStr.isNull() || !yStr.isNull()) { - QSvgHandler::LengthType type; - qreal nx = parseLength(xStr, type, handler); - nx = convertToPixels(nx, true, type); - - qreal ny = parseLength(yStr, type, handler); - ny = convertToPixels(ny, true, type); - pt = QPointF(nx, ny); - } - //delay link resolving till the first draw call on - //use nodes, link 2might have not been created yet - QSvgUse *node = new QSvgUse(pt, parent, link); - return node; + return new QSvgUse(pt, parent, link); } + + //delay link resolving, link might have not been created yet + return new QSvgUse(pt, parent, linkId); } - qCWarning(lcSvgHandler, "link %s hasn't been detected!", qPrintable(linkId)); + qCWarning(lcSvgHandler, " element %s in wrong context!", qPrintable(linkId)); return 0; } @@ -3647,6 +3648,7 @@ void QSvgHandler::parse() } } resolveGradients(m_doc); + resolveNodes(); } bool QSvgHandler::startElement(const QString &localName, @@ -3751,6 +3753,9 @@ bool QSvgHandler::startElement(const QString &localName, static_cast(node)->setWhitespaceMode(m_whitespaceMode.top()); } else if (node->type() == QSvgNode::TSPAN) { static_cast(node)->setWhitespaceMode(m_whitespaceMode.top()); + } else if (node->type() == QSvgNode::USE) { + if (!static_cast(node)->isResolved()) + m_resolveNodes.append(node); } } } @@ -3853,6 +3858,33 @@ void QSvgHandler::resolveGradients(QSvgNode *node) } } +void QSvgHandler::resolveNodes() +{ + for (QSvgNode *node : qAsConst(m_resolveNodes)) { + if (!node || !node->parent() || node->type() != QSvgNode::USE) + continue; + QSvgUse *useNode = static_cast(node); + if (useNode->isResolved()) + continue; + QSvgNode::Type t = useNode->parent()->type(); + if (!(t == QSvgNode::DOC || t == QSvgNode::DEFS || t == QSvgNode::G || t == QSvgNode::SWITCH)) + continue; + + QSvgStructureNode *group = static_cast(useNode->parent()); + QSvgNode *link = group->scopeNode(useNode->linkId()); + if (!link) { + qCWarning(lcSvgHandler, "link #%s is undefined!", qPrintable(useNode->linkId())); + continue; + } + + if (useNode->parent()->isDescendantOf(link)) + qCWarning(lcSvgHandler, "link #%s is recursive!", qPrintable(useNode->linkId())); + + useNode->setLink(link); + } + m_resolveNodes.clear(); +} + bool QSvgHandler::characters(const QStringRef &str) { #ifndef QT_NO_CSSPARSER diff --git a/src/svg/qsvghandler_p.h b/src/svg/qsvghandler_p.h index 2c06cb4..5c13003 100644 --- a/src/svg/qsvghandler_p.h +++ b/src/svg/qsvghandler_p.h @@ -178,6 +178,7 @@ private: #endif void parse(); void resolveGradients(QSvgNode *node); + void resolveNodes(); QPen m_defaultPen; /** diff --git a/tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp b/tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp index a8fc9de..5e13bee 100644 --- a/tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp +++ b/tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp @@ -1261,32 +1261,36 @@ void tst_QSvgRenderer::testStopOffsetOpacity() void tst_QSvgRenderer::testUseElement() { static const char *svgs[] = { - //Use referring to non group node (1) + // 0 - Use referring to non group node (1) "" " " " " "", + // 1 "" " " " " "", + // 2 "" " " " " "" "" "", + // 3 "" " " " " " " " " "", - //Use referring to non group node (2) + // 4 - Use referring to non group node (2) "" " " " " "", + // 5 "" " " " " @@ -1295,6 +1299,7 @@ void tst_QSvgRenderer::testUseElement() " " " " "", + // 6 "" " " " " @@ -1303,7 +1308,7 @@ void tst_QSvgRenderer::testUseElement() " " " " "", - //Use referring to group node + // 7 - Use referring to group node "" " " " " @@ -1311,6 +1316,7 @@ void tst_QSvgRenderer::testUseElement() " " " " "", + // 8 "" " " " " @@ -1321,6 +1327,7 @@ void tst_QSvgRenderer::testUseElement() " " " " "", + // 9 "" " " " " @@ -1335,16 +1342,40 @@ void tst_QSvgRenderer::testUseElement() " " " " "", - // Self referral, should be ignored + // 10 - Self referral, should be ignored "", + // 11 "" " " "", + // 12 "" " " "", + // 13 "" " " + "", + // 14 (undefined) + "" + " " + " " + "", + // 15 - Forward references + "" + " " + " " + "", + // 16 + "" + " " + " " + " " + " " + " " + " " + " " + " " "" }; @@ -1373,8 +1404,12 @@ void tst_QSvgRenderer::testUseElement() } } else if (i > 7 && i < 10) { QCOMPARE(images[8], images[i]); - } else if (i > 11) { + } else if (i > 11 && i < 15) { QCOMPARE(images[11], images[i]); + } else if (i == 15) { + QCOMPARE(images[0], images[i]); + } else if (i == 16) { + QCOMPARE(images[8], images[i]); } } } -- cgit v1.2.1