Fix performance regression caused by SG signals in QQuickItem.

For a testcase with thosands of items, I measured an increase
in shutdown time from 800ms to 7500ms, all spent in disconnect().
This is not acceptible, so we're choosing a different approach.

If items implement a invalidateSceneGraph slot, this function
will be called during shutdown. It should be made a proper virtual
in Qt 6. This approach costs very little.

Change-Id: I5970143cc0a0744955687e17586f0bb00c9afb26
Reviewed-by: Lars Knoll <lars.knoll@digia.com>

diff --git a/src/quick/items/context2d/qquickcanvasitem.cpp b/src/quick/items/context2d/qquickcanvasitem.cpp
index 878cc70..fffd469 100644 (file)
--- a/src/quick/items/context2d/qquickcanvasitem.cpp
+++ b/src/quick/items/context2d/qquickcanvasitem.cpp
@@ -302,7 +302,6 @@ QQuickCanvasItem::QQuickCanvasItem(QQuickItem *parent)
     : QQuickItem(*(new QQuickCanvasItemPrivate), parent)
 {
     setFlag(ItemHasContents);
-    connect(this, SIGNAL(sceneGraphInvalidated()), this, SLOT(invalidateSG()));
 }
 
 QQuickCanvasItem::~QQuickCanvasItem()
@@ -632,7 +631,7 @@ void QQuickCanvasItem::releaseResources()
     }
 }
 
-void QQuickCanvasItem::invalidateSG()
+void QQuickCanvasItem::invalidateSceneGraph()
 {
     Q_D(QQuickCanvasItem);
     d->context->deleteLater();

diff --git a/src/quick/items/context2d/qquickcanvasitem_p.h b/src/quick/items/context2d/qquickcanvasitem_p.h
index 95bc6f3..b3509a5 100644 (file)
--- a/src/quick/items/context2d/qquickcanvasitem_p.h
+++ b/src/quick/items/context2d/qquickcanvasitem_p.h
@@ -159,7 +159,7 @@ public Q_SLOTS:
 private Q_SLOTS:
     void sceneGraphInitialized();
     void checkAnimationCallbacks();
-    void invalidateSG();
+    void invalidateSceneGraph();
 
 protected:
     void componentComplete();

diff --git a/src/quick/items/qquickframebufferobject.cpp b/src/quick/items/qquickframebufferobject.cpp
index 7c70f27..da1709f 100644 (file)
--- a/src/quick/items/qquickframebufferobject.cpp
+++ b/src/quick/items/qquickframebufferobject.cpp
@@ -108,7 +108,6 @@ QQuickFramebufferObject::QQuickFramebufferObject(QQuickItem *parent) :
     QQuickItem(*new QQuickFramebufferObjectPrivate, parent)
 {
     setFlag(ItemHasContents);
-    connect(this, SIGNAL(sceneGraphInvalidated()), this, SLOT(invalidateSG()));
 }
 
 /*!
@@ -308,7 +307,7 @@ void QQuickFramebufferObject::releaseResources()
     d->node = 0;
 }
 
-void QQuickFramebufferObject::invalidateSG()
+void QQuickFramebufferObject::invalidateSceneGraph()
 {
     Q_D(QQuickFramebufferObject);
     d->node = 0;

diff --git a/src/quick/items/qquickframebufferobject.h b/src/quick/items/qquickframebufferobject.h
index 99e68b9..f6431dc 100644 (file)
--- a/src/quick/items/qquickframebufferobject.h
+++ b/src/quick/items/qquickframebufferobject.h
@@ -89,7 +89,7 @@ Q_SIGNALS:
     void textureFollowsItemSizeChanged(bool);
 
 private Q_SLOTS:
-    void invalidateSG();
+    void invalidateSceneGraph();
 };
 
 QT_END_NAMESPACE

diff --git a/src/quick/items/qquickimage.cpp b/src/quick/items/qquickimage.cpp
index 073b107..1e389cf 100644 (file)
--- a/src/quick/items/qquickimage.cpp
+++ b/src/quick/items/qquickimage.cpp
@@ -152,13 +152,11 @@ QQuickImagePrivate::QQuickImagePrivate()
 QQuickImage::QQuickImage(QQuickItem *parent)
     : QQuickImageBase(*(new QQuickImagePrivate), parent)
 {
-    connect(this, SIGNAL(sceneGraphInvalidated()), this, SLOT(invalidateSG()));
 }
 
 QQuickImage::QQuickImage(QQuickImagePrivate &dd, QQuickItem *parent)
     : QQuickImageBase(dd, parent)
 {
-    connect(this, SIGNAL(sceneGraphInvalidated()), this, SLOT(invalidateSG()));
 }
 
 QQuickImage::~QQuickImage()
@@ -569,7 +567,7 @@ QSGTextureProvider *QQuickImage::textureProvider() const
     return d->provider;
 }
 
-void QQuickImage::invalidateSG()
+void QQuickImage::invalidateSceneGraph()
 {
     Q_D(QQuickImage);
     delete d->provider;

diff --git a/src/quick/items/qquickimage_p.h b/src/quick/items/qquickimage_p.h
index 22302f8..b359ff7 100644 (file)
--- a/src/quick/items/qquickimage_p.h
+++ b/src/quick/items/qquickimage_p.h
@@ -95,7 +95,7 @@ Q_SIGNALS:
     Q_REVISION(1) void mipmapChanged(bool);
 
 private Q_SLOTS:
-    void invalidateSG();
+    void invalidateSceneGraph();
 
 protected:
     QQuickImage(QQuickImagePrivate &dd, QQuickItem *parent);

diff --git a/src/quick/items/qquickitem.cpp b/src/quick/items/qquickitem.cpp
index b6939dd..d45fb31 100644 (file)
--- a/src/quick/items/qquickitem.cpp
+++ b/src/quick/items/qquickitem.cpp
@@ -1699,11 +1699,14 @@ void QQuickItemPrivate::updateSubFocusItem(QQuickItem *scope, bool focus)
     \list
 
     \li The scene graph is invalidated; This can happen, for instance,
-    if the window is hidden using QQuickWindow::hide(). The signal
-    QQuickItem::sceneGraphInvalidated() is emitted on the rendering
-    thread and the GUI thread is blocked for the duration of this
-    call. Graphics resources can be deleted directly when this signal
-    is connected to using a Qt::DirectConnection.
+    if the window is hidden using QQuickWindow::hide(). If the item
+    class implements a \c slot named \c invalidateSceneGraph(), this
+    slot will be called on the rendering thread while the GUI thread
+    is blocked. This is equivalent to connecting to
+    QQuickWindow::sceneGraphInvalidated(). The OpenGL context of this
+    item's window will be bound when this slot is called. The only
+    exception is if the native OpenGL has been destroyed outside Qt's
+    control, for instance through \c EGL_CONTEXT_LOST.
 
     \li The item is removed from the scene; If an item is taken out of
     the scene, for instance because it's parent was set to \c null or
@@ -2677,11 +2680,6 @@ void QQuickItemPrivate::refWindow(QQuickWindow *c)
     Q_ASSERT(window == 0);
     window = c;
 
-    if (q->flags() & QQuickItem::ItemHasContents) {
-        QObject::connect(window, SIGNAL(sceneGraphInvalidated()), q, SIGNAL(sceneGraphInvalidated()), Qt::DirectConnection);
-        QObject::connect(window, SIGNAL(sceneGraphInitialized()), q, SIGNAL(sceneGraphInitialized()), Qt::DirectConnection);
-    }
-
     if (polishScheduled)
         QQuickWindowPrivate::get(window)->itemsToPolish.insert(q);
 
@@ -2711,11 +2709,6 @@ void QQuickItemPrivate::derefWindow()
     if (--windowRefCount > 0)
         return; // There are still other references, so don't set window to null yet.
 
-    if (q->flags() & QQuickItem::ItemHasContents) {
-        QObject::disconnect(window, SIGNAL(sceneGraphInvalidated()), q, SIGNAL(sceneGraphInvalidated()));
-        QObject::disconnect(window, SIGNAL(sceneGraphInitialized()), q, SIGNAL(sceneGraphInitialized()));
-    }
-
     q->releaseResources();
     removeFromDirtyList();
     QQuickWindowPrivate *c = QQuickWindowPrivate::get(window);
@@ -5866,67 +5859,10 @@ void QQuickItem::setFlags(Flags flags)
     if (int(flags & ItemClipsChildrenToShape) != int(d->flags & ItemClipsChildrenToShape))
         d->dirty(QQuickItemPrivate::Clip);
 
-    if (window() && (flags & ItemHasContents) ^ (d->flags & ItemHasContents)) {
-        if (flags & ItemHasContents)  {
-            connect(window(), SIGNAL(sceneGraphInvalidated()), this, SIGNAL(sceneGraphInvalidated()), Qt::DirectConnection);
-            connect(window(), SIGNAL(sceneGraphInitialized()), this, SIGNAL(sceneGraphInitialized()), Qt::DirectConnection);
-        } else {
-            disconnect(window(), SIGNAL(sceneGraphInvalidated()), this, SIGNAL(sceneGraphInvalidated()));
-            disconnect(window(), SIGNAL(sceneGraphInitialized()), this, SIGNAL(sceneGraphInitialized()));
-        }
-    }
-
     d->flags = flags;
 }
 
 /*!
-    \fn void QQuickItem::sceneGraphInvalidated()
-
-    This signal is emitted when the scene graph is invalidated for
-    items that have the ItemHasContents flag set.
-
-    QSGNode instances will be cleaned up by the scene graph
-    automatically. An application will only need to react to this signal
-    to clean up resources that are stored and managed outside the
-    QSGNode structure returned from updatePaintNode().
-
-    When the scene graph is using a dedicated render thread, this
-    signal will be emitted on the scene graph's render thread. The
-    GUI thread is blocked for the duration of this call. Connections
-    should for this reason be made using Qt::DirectConnection.
-
-    The OpenGL context of this item's window will be bound when this
-    signal is emitted. The only exception is if the native OpenGL has
-    been destroyed outside Qt's control, for instance through
-    EGL_CONTEXT_LOST.
-
-    \since 5.4
-    \since QtQuick 2.4
-
-    \sa QQuickWindow::sceneGraphInvalidated(), {Graphics Resource Handling}
- */
-
-/*!
-    \fn void QQuickItem::sceneGraphInitialized()
-
-    This signal is emitted when the scene graph is initialized for
-    items that have the ItemHasContents flag set.
-
-    When the scene graph is using a dedicated render thread, this
-    function will be called on the scene graph's render thread. The
-    GUI thread is blocked for the duration of this call. Connections
-    should for this reason be made using Qt::DirectConnection.
-
-    The OpenGL context of this item's window will be bound when
-    this signal is emitted.
-
-    \since 5.4
-    \since QtQuick 2.4
-
-    \sa QQuickWindow::sceneGraphInitialized()
- */
-
-/*!
   \qmlproperty real QtQuick::Item::x
   \qmlproperty real QtQuick::Item::y
   \qmlproperty real QtQuick::Item::width

diff --git a/src/quick/items/qquickitem.h b/src/quick/items/qquickitem.h
index 0fad96b..c17f6a6 100644 (file)
--- a/src/quick/items/qquickitem.h
+++ b/src/quick/items/qquickitem.h
@@ -375,9 +375,6 @@ Q_SIGNALS:
     void implicitWidthChanged();
     void implicitHeightChanged();
 
-    Q_REVISION(2) void sceneGraphInvalidated();
-    Q_REVISION(2) void sceneGraphInitialized();
-
 protected:
     virtual bool event(QEvent *);
 

diff --git a/src/quick/items/qquickpainteditem.cpp b/src/quick/items/qquickpainteditem.cpp
index bc148c5..a79f0b5 100644 (file)
--- a/src/quick/items/qquickpainteditem.cpp
+++ b/src/quick/items/qquickpainteditem.cpp
@@ -140,7 +140,6 @@ QQuickPaintedItem::QQuickPaintedItem(QQuickItem *parent)
     : QQuickItem(*(new QQuickPaintedItemPrivate), parent)
 {
     setFlag(ItemHasContents);
-    connect(this, SIGNAL(sceneGraphInvalidated()), this, SLOT(invalidateSG()));
 }
 
 /*!
@@ -150,7 +149,6 @@ QQuickPaintedItem::QQuickPaintedItem(QQuickPaintedItemPrivate &dd, QQuickItem *p
     : QQuickItem(dd, parent)
 {
     setFlag(ItemHasContents);
-    connect(this, SIGNAL(sceneGraphInvalidated()), this, SLOT(invalidateSG()));
 }
 
 /*!
@@ -561,7 +559,7 @@ void QQuickPaintedItem::releaseResources()
     d->node = 0; // Managed by the scene graph, just clear the pointer.
 }
 
-void QQuickPaintedItem::invalidateSG()
+void QQuickPaintedItem::invalidateSceneGraph()
 {
     Q_D(QQuickPaintedItem);
     delete d->textureProvider;

diff --git a/src/quick/items/qquickpainteditem.h b/src/quick/items/qquickpainteditem.h
index 0a387e6..b5e0469 100644 (file)
--- a/src/quick/items/qquickpainteditem.h
+++ b/src/quick/items/qquickpainteditem.h
@@ -111,7 +111,7 @@ protected:
     void releaseResources() Q_DECL_OVERRIDE;
 
 private Q_SLOTS:
-    void invalidateSG();
+    void invalidateSceneGraph();
 
 private:
     Q_DISABLE_COPY(QQuickPaintedItem)

diff --git a/src/quick/items/qquickshadereffectsource.cpp b/src/quick/items/qquickshadereffectsource.cpp
index 4861c9e..80be283 100644 (file)
--- a/src/quick/items/qquickshadereffectsource.cpp
+++ b/src/quick/items/qquickshadereffectsource.cpp
@@ -192,7 +192,6 @@ QQuickShaderEffectSource::QQuickShaderEffectSource(QQuickItem *parent)
     , m_grab(true)
 {
     setFlag(ItemHasContents);
-    connect(this, SIGNAL(sceneGraphInvalidated()), this, SLOT(invalidateSG()));
 }
 
 QQuickShaderEffectSource::~QQuickShaderEffectSource()
@@ -677,7 +676,7 @@ QSGNode *QQuickShaderEffectSource::updatePaintNode(QSGNode *oldNode, UpdatePaint
     return node;
 }
 
-void QQuickShaderEffectSource::invalidateSG()
+void QQuickShaderEffectSource::invalidateSceneGraph()
 {
     if (m_texture)
         delete m_texture;

diff --git a/src/quick/items/qquickshadereffectsource_p.h b/src/quick/items/qquickshadereffectsource_p.h
index 6da66d4..69e65f9 100644 (file)
--- a/src/quick/items/qquickshadereffectsource_p.h
+++ b/src/quick/items/qquickshadereffectsource_p.h
@@ -132,7 +132,7 @@ Q_SIGNALS:
 
 private Q_SLOTS:
     void sourceItemDestroyed(QObject *item);
-    void invalidateSG();
+    void invalidateSceneGraph();
 
 protected:
     virtual void releaseResources();

diff --git a/src/quick/items/qquickwindow.cpp b/src/quick/items/qquickwindow.cpp
index 0dd66e9..1cb88fd 100644 (file)
--- a/src/quick/items/qquickwindow.cpp
+++ b/src/quick/items/qquickwindow.cpp
@@ -2587,6 +2587,18 @@ void QQuickWindowPrivate::cleanupNodesOnShutdown(QQuickItem *item)
         p->dirty(QQuickItemPrivate::Window);
     }
 
+    // Qt 6: Make invalidateSceneGraph a virtual member of QQuickItem
+    if (p->flags & QQuickItem::ItemHasContents) {
+        const QMetaObject *mo = item->metaObject();
+        int index = mo->indexOfSlot("invalidateSceneGraph()");
+        if (index >= 0) {
+            const QMetaMethod &method = mo->method(index);
+            // Skip functions named invalidateSceneGraph() in QML items.
+            if (strstr(method.enclosingMetaObject()->className(), "_QML_") == 0)
+                method.invoke(item, Qt::DirectConnection);
+        }
+    }
+
     for (int ii = 0; ii < p->childItems.count(); ++ii)
         cleanupNodesOnShutdown(p->childItems.at(ii));
 }

diff --git a/tests/auto/quick/qquickitem/tst_qquickitem.cpp b/tests/auto/quick/qquickitem/tst_qquickitem.cpp
index 38dc8e0..0ce0ded 100644 (file)
--- a/tests/auto/quick/qquickitem/tst_qquickitem.cpp
+++ b/tests/auto/quick/qquickitem/tst_qquickitem.cpp
@@ -165,7 +165,7 @@ private slots:
     void visualParentOwnership();
     void visualParentOwnershipWindow();
 
-    void testSGInitializeAndInvalidate();
+    void testSGInvalidate();
 
     void objectChildTransform();
 
@@ -1880,30 +1880,35 @@ void tst_qquickitem::visualParentOwnershipWindow()
     }
 }
 
-void tst_qquickitem::testSGInitializeAndInvalidate()
+class InvalidatedItem : public QQuickItem {
+    Q_OBJECT
+signals:
+    void invalidated();
+public slots:
+    void invalidateSceneGraph() { emit invalidated(); }
+};
+
+void tst_qquickitem::testSGInvalidate()
 {
     for (int i=0; i<2; ++i) {
         QScopedPointer<QQuickView> view(new QQuickView());
 
-        QQuickItem *item = new QQuickItem();
+        InvalidatedItem *item = new InvalidatedItem();
 
-        int expected;
+        int expected = 0;
         if (i == 0) {
             // First iteration, item has contents and should get signals
             expected = 1;
             item->setFlag(QQuickItem::ItemHasContents, true);
         } else {
             // Second iteration, item does not have content and will not get signals
-            expected = 0;
         }
 
-        QSignalSpy initializeSpy(item, SIGNAL(sceneGraphInitialized()));
-        QSignalSpy invalidateSpy(item, SIGNAL(sceneGraphInvalidated()));
+        QSignalSpy invalidateSpy(item, SIGNAL(invalidated()));
         item->setParentItem(view->contentItem());
         view->show();
 
         QVERIFY(QTest::qWaitForWindowExposed(view.data()));
-        QCOMPARE(initializeSpy.size(), expected);
 
         delete view.take();
         QCOMPARE(invalidateSpy.size(), expected);