Fixed bug in the SVG module where display="none" was not respected.

Elements with display="none" should not be rendered. Modified and
autotest added by Kim.

Reviewed-by: Kim

3 files changed, 123 insertions, 53 deletions

diff --git a/src/svg/qsvgstructure.cpp b/src/svg/qsvgstructure.cpp
index 67a21bf..c1ad4bf 100644
--- a/src/svg/qsvgstructure.cpp
+++ b/src/svg/qsvgstructure.cpp
@@ -68,13 +68,11 @@ void QSvgG::draw(QPainter *p, QSvgExtraStates &states)
     QList<QSvgNode*>::iterator itr = m_renderers.begin();
     applyStyle(p, states);
 
-    if (displayMode() != QSvgNode::NoneMode) {
-        while (itr != m_renderers.end()) {
-            QSvgNode *node = *itr;
-            if (node->isVisible())
-                node->draw(p, states);
-            ++itr;
-        }
+    while (itr != m_renderers.end()) {
+        QSvgNode *node = *itr;
+        if ((node->isVisible()) && (node->displayMode() != QSvgNode::NoneMode))
+            node->draw(p, states);
+        ++itr;
     }
     revertStyle(p, states);
 }
@@ -321,63 +319,61 @@ void QSvgSwitch::draw(QPainter *p, QSvgExtraStates &states)
     QList<QSvgNode*>::iterator itr = m_renderers.begin();
     applyStyle(p, states);
 
-    if (displayMode() != QSvgNode::NoneMode) {
-        while (itr != m_renderers.end()) {
-            QSvgNode *node = *itr;
-            if (node->isVisible()) {
-                const QStringList &features  = node->requiredFeatures();
-                const QStringList &extensions = node->requiredExtensions();
-                const QStringList &languages = node->requiredLanguages();
-                const QStringList &formats = node->requiredFormats();
-                const QStringList &fonts = node->requiredFonts();
-
-                bool okToRender = true;
-                if (!features.isEmpty()) {
-                    QStringList::const_iterator sitr = features.constBegin();
-                    for (; sitr != features.constEnd(); ++sitr) {
-                        if (!isSupportedSvgFeature(*sitr)) {
-                            okToRender = false;
-                            break;
-                        }
+    while (itr != m_renderers.end()) {
+        QSvgNode *node = *itr;
+        if (node->isVisible() && (node->displayMode() != QSvgNode::NoneMode)) {
+            const QStringList &features  = node->requiredFeatures();
+            const QStringList &extensions = node->requiredExtensions();
+            const QStringList &languages = node->requiredLanguages();
+            const QStringList &formats = node->requiredFormats();
+            const QStringList &fonts = node->requiredFonts();
+
+            bool okToRender = true;
+            if (!features.isEmpty()) {
+                QStringList::const_iterator sitr = features.constBegin();
+                for (; sitr != features.constEnd(); ++sitr) {
+                    if (!isSupportedSvgFeature(*sitr)) {
+                        okToRender = false;
+                        break;
                     }
                 }
+            }
 
-                if (okToRender && !extensions.isEmpty()) {
-                    QStringList::const_iterator sitr = extensions.constBegin();
-                    for (; sitr != extensions.constEnd(); ++sitr) {
-                        if (!isSupportedSvgExtension(*sitr)) {
-                            okToRender = false;
-                            break;
-                        }
+            if (okToRender && !extensions.isEmpty()) {
+                QStringList::const_iterator sitr = extensions.constBegin();
+                for (; sitr != extensions.constEnd(); ++sitr) {
+                    if (!isSupportedSvgExtension(*sitr)) {
+                        okToRender = false;
+                        break;
                     }
                 }
+            }
 
-                if (okToRender && !languages.isEmpty()) {
-                    QStringList::const_iterator sitr = languages.constBegin();
-                    okToRender = false;
-                    for (; sitr != languages.constEnd(); ++sitr) {
-                        if ((*sitr).startsWith(m_systemLanguagePrefix)) {
-                            okToRender = true;
-                            break;
-                        }
+            if (okToRender && !languages.isEmpty()) {
+                QStringList::const_iterator sitr = languages.constBegin();
+                okToRender = false;
+                for (; sitr != languages.constEnd(); ++sitr) {
+                    if ((*sitr).startsWith(m_systemLanguagePrefix)) {
+                        okToRender = true;
+                        break;
                     }
                 }
+            }
 
-                if (okToRender && !formats.isEmpty()) {
-                    okToRender = false;
-                }
+            if (okToRender && !formats.isEmpty()) {
+                okToRender = false;
+            }
 
-                if (okToRender && !fonts.isEmpty()) {
-                    okToRender = false;
-                }
+            if (okToRender && !fonts.isEmpty()) {
+                okToRender = false;
+            }
 
-                if (okToRender) {
-                    node->draw(p, states);
-                    break;
-                }
+            if (okToRender) {
+                node->draw(p, states);
+                break;
             }
-            ++itr;
         }
+        ++itr;
     }
     revertStyle(p, states);
 }
diff --git a/src/svg/qsvgtinydocument.cpp b/src/svg/qsvgtinydocument.cpp
index de214bb..3058569 100644
--- a/src/svg/qsvgtinydocument.cpp
+++ b/src/svg/qsvgtinydocument.cpp
@@ -231,8 +231,10 @@ void QSvgTinyDocument::draw(QPainter *p, const QRectF &bounds)
         m_time.start();
     }
 
-    p->save();
+    if (displayMode() == QSvgNode::NoneMode)
+        return;
 
+    p->save();
     //sets default style on the painter
     //### not the most optimal way
     mapSourceToTarget(p, bounds);
@@ -244,7 +246,7 @@ void QSvgTinyDocument::draw(QPainter *p, const QRectF &bounds)
     applyStyle(p, m_states);
     while (itr != m_renderers.end()) {
         QSvgNode *node = *itr;
-        if (node->isVisible())
+        if ((node->isVisible()) && (node->displayMode() != QSvgNode::NoneMode))
             node->draw(p, m_states);
         ++itr;
     }
@@ -262,6 +264,12 @@ void QSvgTinyDocument::draw(QPainter *p, const QString &id,
         qDebug("Couldn't find node %s. Skipping rendering.", qPrintable(id));
         return;
     }
+    if (m_time.isNull()) {
+        m_time.start();
+    }
+
+    if (node->displayMode() == QSvgNode::NoneMode)
+        return;
 
     p->save();
 
diff --git a/tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp b/tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp
index 820bcba..f11aff9 100644
--- a/tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp
+++ b/tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp
@@ -79,6 +79,7 @@ private slots:
     void fillRule();
     void opacity();
     void paths();
+    void displayMode();
 
 #ifndef QT_NO_COMPRESS
     void testGzLoading();
@@ -859,5 +860,70 @@ void tst_QSvgRenderer::paths()
     }
 }
 
+void tst_QSvgRenderer::displayMode()
+{
+    static const char *svgs[] = {
+        // All visible.
+        "<svg>"
+        "   <g>"
+        "       <rect x=\"0\" y=\"0\" height=\"10\" width=\"10\" fill=\"red\" />"
+        "       <rect x=\"0\" y=\"0\" height=\"10\" width=\"10\" fill=\"blue\" />"
+        "   </g>"
+        "</svg>",
+        // Don't display svg element.
+        "<svg display=\"none\">"
+        "   <g>"
+        "       <rect x=\"0\" y=\"0\" height=\"10\" width=\"10\" fill=\"red\" />"
+        "       <rect x=\"0\" y=\"0\" height=\"10\" width=\"10\" fill=\"blue\" />"
+        "   </g>"
+        "</svg>",
+        // Don't display g element.
+        "<svg>"
+        "   <g display=\"none\">"
+        "       <rect x=\"0\" y=\"0\" height=\"10\" width=\"10\" fill=\"red\" />"
+        "       <rect x=\"0\" y=\"0\" height=\"10\" width=\"10\" fill=\"blue\" />"
+        "   </g>"
+        "</svg>",
+        // Don't display first rect element.
+        "<svg>"
+        "   <g>"
+        "       <rect x=\"0\" y=\"0\" height=\"10\" width=\"10\" fill=\"red\" display=\"none\" />"
+        "       <rect x=\"0\" y=\"0\" height=\"10\" width=\"10\" fill=\"blue\" />"
+        "   </g>"
+        "</svg>",
+        // Don't display second rect element.
+        "<svg>"
+        "   <g>"
+        "       <rect x=\"0\" y=\"0\" height=\"10\" width=\"10\" fill=\"red\" />"
+        "       <rect x=\"0\" y=\"0\" height=\"10\" width=\"10\" fill=\"blue\" display=\"none\" />"
+        "   </g>"
+        "</svg>",
+        // Don't display svg element, but set display mode to "inline" for other elements.
+        "<svg display=\"none\">"
+        "   <g display=\"inline\">"
+        "       <rect x=\"0\" y=\"0\" height=\"10\" width=\"10\" fill=\"red\" display=\"inline\" />"
+        "       <rect x=\"0\" y=\"0\" height=\"10\" width=\"10\" fill=\"blue\" display=\"inline\" />"
+        "   </g>"
+        "</svg>"
+    };
+
+    QRgb expectedColors[] = {0xff0000ff, 0xff00ff00, 0xff00ff00, 0xff0000ff, 0xffff0000, 0xff00ff00};
+
+    const int COUNT = sizeof(svgs) / sizeof(svgs[0]);
+    QPainter p;
+
+    for (int i = 0; i < COUNT; ++i) {
+        QByteArray data(svgs[i]);
+        QSvgRenderer renderer(data);
+        QVERIFY(renderer.isValid());
+        QImage image(10, 10, QImage::Format_ARGB32_Premultiplied);
+        image.fill(0xff00ff00);
+        p.begin(&image);
+        renderer.render(&p);
+        p.end();
+        QCOMPARE(image.pixel(5, 5), expectedColors[i]);
+    }
+}
+
 QTEST_MAIN(tst_QSvgRenderer)
 #include "tst_qsvgrenderer.moc"