Remove the "groupNode"

We originally had the groupnode to simplify adding and removing clip /
effect / opacity nodes to the item tree at a time when the renderer
was a bit more trivial and only did a single pass over the tree during
rendering. The runtime cost at the time was negligible. The
QSGBatchRenderer has a bit more logic, so the extra node now costs a
bit more. In addition to extra memory, we need to allocate shadow
nodes for it and put those into the renderer's internal shadownode
hash.

This removal increases the performance of adding / removal of simple
items by ~10% in addition to reducing the number of nodes in the
scene graph by up to 1/3.

Change-Id: I8cd64984f868d75820e25d33dfdbebd4d20651fe
Reviewed-by: Robin Burchell <robin.burchell@viroteck.net>

6 files changed, 105 insertions, 107 deletions

diff --git a/src/quick/items/qquickitem.cpp b/src/quick/items/qquickitem.cpp
index 459cde9bdc..b68a9b5455 100644
--- a/src/quick/items/qquickitem.cpp
+++ b/src/quick/items/qquickitem.cpp
@@ -2843,7 +2843,6 @@ void QQuickItemPrivate::derefWindow()
         extra->rootNode = 0;
     }
 
-    groupNode = 0;
     paintNode = 0;
 
     for (int ii = 0; ii < childItems.count(); ++ii) {
@@ -2968,7 +2967,6 @@ QQuickItemPrivate::QQuickItemPrivate()
     , implicitHeight(0)
     , baselineOffset(0)
     , itemNodeInstance(0)
-    , groupNode(0)
     , paintNode(0)
 {
 }
diff --git a/src/quick/items/qquickitem_p.h b/src/quick/items/qquickitem_p.h
index 01fc09c619..2ae67c4c23 100644
--- a/src/quick/items/qquickitem_p.h
+++ b/src/quick/items/qquickitem_p.h
@@ -555,8 +555,7 @@ public:
          - itemNode
          - (opacityNode)
          - (clipNode)
-         - (effectNode)
-         - groupNode
+         - (rootNode) (shader effect source's root node)
      */
 
     QSGOpacityNode *opacityNode() const { return extra.isAllocated()?extra->opacityNode:0; }
@@ -564,7 +563,6 @@ public:
     QSGRootNode *rootNode() const { return extra.isAllocated()?extra->rootNode:0; }
 
     QSGTransformNode *itemNodeInstance;
-    QSGNode *groupNode;
     QSGNode *paintNode;
 
     virtual QSGTransformNode *createTransformNode();
@@ -867,7 +865,7 @@ QSGTransformNode *QQuickItemPrivate::itemNode()
         itemNodeInstance->setFlag(QSGNode::OwnedByParent, false);
 #ifdef QSG_RUNTIME_DESCRIPTION
         Q_Q(QQuickItem);
-        qsgnode_set_description(itemNodeInstance, QString::fromLatin1("QQuickItem(%1)").arg(QString::fromLatin1(q->metaObject()->className())));
+        qsgnode_set_description(itemNodeInstance, QString::fromLatin1("QQuickItem(%1:%2)").arg(QString::fromLatin1(q->metaObject()->className())).arg(q->objectName()));
 #endif
     }
     return itemNodeInstance;
@@ -875,21 +873,14 @@ QSGTransformNode *QQuickItemPrivate::itemNode()
 
 QSGNode *QQuickItemPrivate::childContainerNode()
 {
-    if (!groupNode) {
-        groupNode = new QSGNode();
-        if (rootNode())
-            rootNode()->appendChildNode(groupNode);
-        else if (clipNode())
-            clipNode()->appendChildNode(groupNode);
-        else if (opacityNode())
-            opacityNode()->appendChildNode(groupNode);
-        else
-            itemNode()->appendChildNode(groupNode);
-#ifdef QSG_RUNTIME_DESCRIPTION
-        qsgnode_set_description(groupNode, QLatin1String("group"));
-#endif
-    }
-    return groupNode;
+    if (rootNode())
+        return rootNode();
+    else if (clipNode())
+        return clipNode();
+    else if (opacityNode())
+        return opacityNode();
+    else
+        return itemNode();
 }
 
 Q_DECLARE_OPERATORS_FOR_FLAGS(QQuickItemPrivate::ChangeTypes)
diff --git a/src/quick/items/qquickwindow.cpp b/src/quick/items/qquickwindow.cpp
index 59aae82e28..1fc0584a34 100644
--- a/src/quick/items/qquickwindow.cpp
+++ b/src/quick/items/qquickwindow.cpp
@@ -2591,7 +2591,6 @@ void QQuickWindowPrivate::cleanupNodesOnShutdown(QQuickItem *item)
             p->extra->rootNode = 0;
         }
 
-        p->groupNode = 0;
         p->paintNode = 0;
 
         p->dirty(QQuickItemPrivate::Window);
@@ -2698,42 +2697,43 @@ void QQuickWindowPrivate::updateDirtyNode(QQuickItem *item)
 
     if (clipEffectivelyChanged) {
         QSGNode *parent = itemPriv->opacityNode() ? (QSGNode *) itemPriv->opacityNode() :
-                                                    (QSGNode *)itemPriv->itemNode();
-        QSGNode *child = itemPriv->rootNode() ? (QSGNode *)itemPriv->rootNode() :
-                                                (QSGNode *)itemPriv->groupNode;
+                                                    (QSGNode *) itemPriv->itemNode();
+        QSGNode *child = itemPriv->rootNode();
 
         if (item->clip()) {
             Q_ASSERT(itemPriv->clipNode() == 0);
-            itemPriv->extra.value().clipNode = new QQuickDefaultClipNode(item->clipRect());
-            itemPriv->clipNode()->update();
+            QQuickDefaultClipNode *clip = new QQuickDefaultClipNode(item->clipRect());
+            itemPriv->extra.value().clipNode = clip;
+            clip->update();
 
-            if (child)
+            if (!child) {
+                parent->reparentChildNodesTo(clip);
+                parent->appendChildNode(clip);
+            } else {
                 parent->removeChildNode(child);
-            parent->appendChildNode(itemPriv->clipNode());
-            if (child)
-                itemPriv->clipNode()->appendChildNode(child);
+                clip->appendChildNode(child);
+                parent->appendChildNode(clip);
+            }
 
         } else {
-            Q_ASSERT(itemPriv->clipNode() != 0);
-            parent->removeChildNode(itemPriv->clipNode());
-            if (child)
-                itemPriv->clipNode()->removeChildNode(child);
+            QQuickDefaultClipNode *clip = itemPriv->clipNode();
+            Q_ASSERT(clip);
+            parent->removeChildNode(clip);
+            if (child) {
+                clip->removeChildNode(child);
+                parent->appendChildNode(child);
+            } else {
+                clip->reparentChildNodesTo(parent);
+            }
+
             delete itemPriv->clipNode();
             itemPriv->extra->clipNode = 0;
-            if (child)
-                parent->appendChildNode(child);
         }
     }
 
     if (effectRefEffectivelyChanged) {
-        QSGNode *child = 0;
-
-        if (dirty & QQuickItemPrivate::ChildrenUpdateMask) {
-            child = itemPriv->childContainerNode();
-            child->removeAllChildNodes();
-        } else {
-            child = itemPriv->groupNode;
-        }
+        if (dirty & QQuickItemPrivate::ChildrenUpdateMask)
+            itemPriv->childContainerNode()->removeAllChildNodes();
 
         QSGNode *parent = itemPriv->clipNode();
         if (!parent)
@@ -2743,29 +2743,23 @@ void QQuickWindowPrivate::updateDirtyNode(QQuickItem *item)
 
         if (itemPriv->extra.isAllocated() && itemPriv->extra->effectRefCount) {
             Q_ASSERT(itemPriv->rootNode() == 0);
-            itemPriv->extra->rootNode = new QSGRootNode;
-
-            if (child)
-                parent->removeChildNode(child);
-            parent->appendChildNode(itemPriv->rootNode());
-            if (child)
-                itemPriv->rootNode()->appendChildNode(child);
+            QSGRootNode *root = new QSGRootNode();
+            itemPriv->extra->rootNode = root;
+            parent->reparentChildNodesTo(root);
+            parent->appendChildNode(root);
         } else {
             Q_ASSERT(itemPriv->rootNode() != 0);
-            parent->removeChildNode(itemPriv->rootNode());
-            if (child)
-                itemPriv->rootNode()->removeChildNode(child);
+            QSGRootNode *root = itemPriv->rootNode();
+            parent->removeChildNode(root);
+            root->reparentChildNodesTo(parent);
             delete itemPriv->rootNode();
             itemPriv->extra->rootNode = 0;
-            if (child)
-                parent->appendChildNode(child);
         }
     }
 
     if (dirty & QQuickItemPrivate::ChildrenUpdateMask) {
-        QSGNode *groupNode = itemPriv->groupNode;
-        if (groupNode)
-            groupNode->removeAllChildNodes();
+        QSGNode *parent = itemPriv->childContainerNode();
+        parent->removeAllChildNodes();
 
         QList<QQuickItem *> orderedChildren = itemPriv->paintOrderChildItems();
         int ii = 0;
@@ -2808,20 +2802,22 @@ void QQuickWindowPrivate::updateDirtyNode(QQuickItem *item)
                       ? itemPriv->opacity() : qreal(0);
 
         if (opacity != 1 && !itemPriv->opacityNode()) {
-            itemPriv->extra.value().opacityNode = new QSGOpacityNode;
+            QSGOpacityNode *node = new QSGOpacityNode;
+            itemPriv->extra.value().opacityNode = node;
 
             QSGNode *parent = itemPriv->itemNode();
             QSGNode *child = itemPriv->clipNode();
             if (!child)
                 child = itemPriv->rootNode();
-            if (!child)
-                child = itemPriv->groupNode;
 
-            if (child)
+            if (child) {
                 parent->removeChildNode(child);
-            parent->appendChildNode(itemPriv->opacityNode());
-            if (child)
-                itemPriv->opacityNode()->appendChildNode(child);
+                node->appendChildNode(child);
+                parent->appendChildNode(node);
+            } else {
+                parent->reparentChildNodesTo(node);
+                parent->appendChildNode(node);
+            }
         }
         if (itemPriv->opacityNode())
             itemPriv->opacityNode()->setOpacity(opacity);
@@ -2852,37 +2848,22 @@ void QQuickWindowPrivate::updateDirtyNode(QQuickItem *item)
 
 #ifndef QT_NO_DEBUG
     // Check consistency.
-    const QSGNode *nodeChain[] = {
-        itemPriv->itemNodeInstance,
-        itemPriv->opacityNode(),
-        itemPriv->clipNode(),
-        itemPriv->rootNode(),
-        itemPriv->groupNode,
-        itemPriv->paintNode,
-    };
-
-    int ip = 0;
-    for (;;) {
-        while (ip < 5 && nodeChain[ip] == 0)
-            ++ip;
-        if (ip == 5)
-            break;
-        int ic = ip + 1;
-        while (ic < 5 && nodeChain[ic] == 0)
-            ++ic;
-        const QSGNode *parent = nodeChain[ip];
-        const QSGNode *child = nodeChain[ic];
-        if (child == 0) {
-            Q_ASSERT(parent == itemPriv->groupNode || parent->childCount() == 0);
-        } else {
-            Q_ASSERT(parent == itemPriv->groupNode || parent->childCount() == 1);
-            Q_ASSERT(child->parent() == parent);
-            bool containsChild = false;
-            for (QSGNode *n = parent->firstChild(); n; n = n->nextSibling())
-                containsChild |= (n == child);
-            Q_ASSERT(containsChild);
-        }
-        ip = ic;
+
+    QList<QSGNode *> nodes;
+    nodes << itemPriv->itemNodeInstance
+          << itemPriv->opacityNode()
+          << itemPriv->clipNode()
+          << itemPriv->rootNode()
+          << itemPriv->paintNode;
+    nodes.removeAll(0);
+
+    Q_ASSERT(nodes.first() == itemPriv->itemNodeInstance);
+    for (int i=1; i<nodes.size(); ++i) {
+        QSGNode *n = nodes.at(i);
+        // Failing this means we messed up reparenting
+        Q_ASSERT(n->parent() == nodes.at(i-1));
+        // Only the paintNode and the one who is childContainer may have more than one child.
+        Q_ASSERT(n == itemPriv->paintNode || n == itemPriv->childContainerNode() || n->childCount() == 1);
     }
 #endif
 
diff --git a/src/quick/scenegraph/coreapi/qsgnode.cpp b/src/quick/scenegraph/coreapi/qsgnode.cpp
index 58b6462d23..6271d12998 100644
--- a/src/quick/scenegraph/coreapi/qsgnode.cpp
+++ b/src/quick/scenegraph/coreapi/qsgnode.cpp
@@ -575,6 +575,21 @@ void QSGNode::removeAllChildNodes()
     }
 }
 
+/*!
+ * \internal
+ *
+ * Reparents all nodes of this node to \a newParent.
+ */
+void QSGNode::reparentChildNodesTo(QSGNode *newParent)
+{
+    Q_ASSERT_X(!newParent->parent(), "QSGNode::reparentChildNodesTo", "newParent is already part of a hierarchy");
+
+    for (QSGNode *c = firstChild(); c; c = c->nextSibling()) {
+        removeChildNode(c);
+        newParent->appendChildNode(c);
+    }
+}
+
 
 int QSGNode::childCount() const
 {
diff --git a/src/quick/scenegraph/coreapi/qsgnode.h b/src/quick/scenegraph/coreapi/qsgnode.h
index dfacd84288..059cbeebdf 100644
--- a/src/quick/scenegraph/coreapi/qsgnode.h
+++ b/src/quick/scenegraph/coreapi/qsgnode.h
@@ -132,6 +132,7 @@ public:
     void appendChildNode(QSGNode *node);
     void insertChildNodeBefore(QSGNode *node, QSGNode *before);
     void insertChildNodeAfter(QSGNode *node, QSGNode *after);
+    void reparentChildNodesTo(QSGNode *newParent);
 
     int childCount() const;
     QSGNode *childAtIndex(int i) const;
diff --git a/src/quick/util/qquickanimatorjob.cpp b/src/quick/util/qquickanimatorjob.cpp
index fdbffd4709..8c72c09738 100644
--- a/src/quick/util/qquickanimatorjob.cpp
+++ b/src/quick/util/qquickanimatorjob.cpp
@@ -412,21 +412,33 @@ void QQuickOpacityAnimatorJob::initialize(QQuickAnimatorController *controller)
     m_opacityNode = d->opacityNode();
     if (!m_opacityNode) {
         m_opacityNode = new QSGOpacityNode();
-        d->extra.value().opacityNode = m_opacityNode;
-
-        QSGNode *child = d->clipNode();
-        if (!child)
-            child = d->rootNode();
-        if (!child)
-            child = d->groupNode;
 
-        if (child) {
+        /* The item node subtree is like this
+         *
+         * itemNode
+         * (opacityNode)            optional
+         * (clipNode)               optional
+         * (rootNode)               optional
+         * children / paintNode
+         *
+         * If the opacity node doesn't exist, we need to insert it into
+         * the hierarchy between itemNode and clipNode or rootNode. If
+         * neither clip or root exists, we need to reparent all children
+         * from itemNode to opacityNode.
+         */
+        QSGNode *iNode = d->itemNode();
+        QSGNode *child = d->childContainerNode();
+        if (child != iNode) {
             if (child->parent())
                 child->parent()->removeChildNode(child);
             m_opacityNode->appendChildNode(child);
+            iNode->appendChildNode(m_opacityNode);
+        } else {
+            iNode->reparentChildNodesTo(m_opacityNode);
+            iNode->appendChildNode(m_opacityNode);
         }
 
-        d->itemNode()->appendChildNode(m_opacityNode);
+        d->extra.value().opacityNode = m_opacityNode;
     }
 }