rhi: Fix crashing Controls autotests due to shadereffect lifetime issues

Make the QSGShaderEffectNode base class a QObject, the subclass
(QSGRhiShaderEffectNode) is already a QObject so we lose nothing.

This way we can get rid of the textureChanged() signal from the
shadereffectmanager (the object living on the gui thread), and so the
node (living on the render thread) does not need to do emit
m_mgr->textureChanged() on the render thread. The signal emission in
itself was perfectly fine, except when m_mgr happened to be destroyed
already in case the node outlived the gui thread's item (and m_mgr).

After the change we have something that is closer to what the direct
OpenGL implementation did - and some of the interfaces in QSGContext get
a lot cleaner in fact because the node can be instantiated on its own,
without the involving the shadereffectmanager object that does not even
live on the render thread.

Change-Id: Ibb918cc3eaa08faddb8b8e5dfa3e34b212161703
Reviewed-by: Eirik Aavitsland <eirik.aavitsland@qt.io>

8 files changed, 20 insertions, 25 deletions

diff --git a/src/quick/items/qquickgenericshadereffect.cpp b/src/quick/items/qquickgenericshadereffect.cpp
index a3d04f5dd3..b7f52a8146 100644
--- a/src/quick/items/qquickgenericshadereffect.cpp
+++ b/src/quick/items/qquickgenericshadereffect.cpp
@@ -269,12 +269,14 @@ QSGNode *QQuickGenericShaderEffect::handleUpdatePaintNode(QSGNode *oldNode, QQui
 
     if (!node) {
         QSGRenderContext *rc = QQuickWindowPrivate::get(m_item->window())->context;
-        node = rc->sceneGraphContext()->createShaderEffectNode(rc, mgr);
+        node = rc->sceneGraphContext()->createShaderEffectNode(rc);
         if (!node) {
             qWarning("No shader effect node");
             return nullptr;
         }
         m_dirty = QSGShaderEffectNode::DirtyShaderAll;
+        connect(node, &QSGShaderEffectNode::textureChanged,
+                this, &QQuickGenericShaderEffect::markGeometryDirtyAndUpdateIfSupportsAtlas);
     }
 
     QSGShaderEffectNode::SyncData sd;
@@ -369,7 +371,6 @@ QSGGuiThreadShaderEffectManager *QQuickGenericShaderEffect::shaderEffectManager(
             if (m_mgr) {
                 connect(m_mgr, SIGNAL(logAndStatusChanged()), m_item, SIGNAL(logChanged()));
                 connect(m_mgr, SIGNAL(logAndStatusChanged()), m_item, SIGNAL(statusChanged()));
-                connect(m_mgr, SIGNAL(textureChanged()), this, SLOT(markGeometryDirtyAndUpdateIfSupportsAtlas()));
                 connect(m_mgr, &QSGGuiThreadShaderEffectManager::shaderCodePrepared, this, &QQuickGenericShaderEffect::shaderCodePrepared);
             }
         }
diff --git a/src/quick/scenegraph/qsgadaptationlayer_p.h b/src/quick/scenegraph/qsgadaptationlayer_p.h
index 1db49feb5d..6376317427 100644
--- a/src/quick/scenegraph/qsgadaptationlayer_p.h
+++ b/src/quick/scenegraph/qsgadaptationlayer_p.h
@@ -299,7 +299,6 @@ public:
 
 Q_SIGNALS:
     void shaderCodePrepared(bool ok, ShaderInfo::Type typeHint, const QByteArray &src, ShaderInfo *result);
-    void textureChanged();
     void logAndStatusChanged();
 };
 
@@ -307,8 +306,10 @@ Q_SIGNALS:
 Q_QUICK_PRIVATE_EXPORT QDebug operator<<(QDebug debug, const QSGGuiThreadShaderEffectManager::ShaderInfo::Variable &v);
 #endif
 
-class Q_QUICK_PRIVATE_EXPORT QSGShaderEffectNode : public QSGVisitableNode
+class Q_QUICK_PRIVATE_EXPORT QSGShaderEffectNode : public QObject, public QSGVisitableNode
 {
+    Q_OBJECT
+
 public:
     enum DirtyShaderFlag {
         DirtyShaders = 0x01,
@@ -355,12 +356,14 @@ public:
     };
 
     // Each ShaderEffect item has one node (render thread) and one manager (gui thread).
-    QSGShaderEffectNode(QSGGuiThreadShaderEffectManager *) { }
 
     virtual QRectF updateNormalizedTextureSubRect(bool supportsAtlasTextures) = 0;
     virtual void syncMaterial(SyncData *syncData) = 0;
 
     void accept(QSGNodeVisitorEx *visitor) override { if (visitor->visit(this)) visitor->visitChildren(this); visitor->endVisit(this); }
+
+Q_SIGNALS:
+    void textureChanged();
 };
 
 Q_DECLARE_OPERATORS_FOR_FLAGS(QSGShaderEffectNode::DirtyShaderFlags)
diff --git a/src/quick/scenegraph/qsgcontext.cpp b/src/quick/scenegraph/qsgcontext.cpp
index e3c951e5ed..9baf530889 100644
--- a/src/quick/scenegraph/qsgcontext.cpp
+++ b/src/quick/scenegraph/qsgcontext.cpp
@@ -284,7 +284,7 @@ QSGGuiThreadShaderEffectManager *QSGContext::createGuiThreadShaderEffectManager(
     valid as long as the backend does not claim SupportsShaderEffectNode or
     ignoring ShaderEffect elements is acceptable.
  */
-QSGShaderEffectNode *QSGContext::createShaderEffectNode(QSGRenderContext *, QSGGuiThreadShaderEffectManager *)
+QSGShaderEffectNode *QSGContext::createShaderEffectNode(QSGRenderContext *)
 {
     return nullptr;
 }
diff --git a/src/quick/scenegraph/qsgcontext_p.h b/src/quick/scenegraph/qsgcontext_p.h
index d389420907..a2cb21a60c 100644
--- a/src/quick/scenegraph/qsgcontext_p.h
+++ b/src/quick/scenegraph/qsgcontext_p.h
@@ -128,8 +128,7 @@ public:
     virtual QSGGlyphNode *createGlyphNode(QSGRenderContext *rc, bool preferNativeGlyphNode) = 0;
     virtual QSGLayer *createLayer(QSGRenderContext *renderContext) = 0;
     virtual QSGGuiThreadShaderEffectManager *createGuiThreadShaderEffectManager();
-    virtual QSGShaderEffectNode *createShaderEffectNode(QSGRenderContext *renderContext,
-                                                        QSGGuiThreadShaderEffectManager *mgr);
+    virtual QSGShaderEffectNode *createShaderEffectNode(QSGRenderContext *renderContext);
 #if QT_CONFIG(quick_sprite)
     virtual QSGSpriteNode *createSpriteNode() = 0;
 #endif
diff --git a/src/quick/scenegraph/qsgdefaultcontext.cpp b/src/quick/scenegraph/qsgdefaultcontext.cpp
index 1d70c826de..f7701d8f7f 100644
--- a/src/quick/scenegraph/qsgdefaultcontext.cpp
+++ b/src/quick/scenegraph/qsgdefaultcontext.cpp
@@ -297,13 +297,10 @@ QSGGuiThreadShaderEffectManager *QSGDefaultContext::createGuiThreadShaderEffectM
     return nullptr;
 }
 
-QSGShaderEffectNode *QSGDefaultContext::createShaderEffectNode(QSGRenderContext *renderContext,
-                                                               QSGGuiThreadShaderEffectManager *mgr)
+QSGShaderEffectNode *QSGDefaultContext::createShaderEffectNode(QSGRenderContext *renderContext)
 {
-    if (QSGRhiSupport::instance()->isRhiEnabled()) {
-        return new QSGRhiShaderEffectNode(static_cast<QSGDefaultRenderContext *>(renderContext),
-                                          static_cast<QSGRhiGuiThreadShaderEffectManager *>(mgr));
-    }
+    if (QSGRhiSupport::instance()->isRhiEnabled())
+        return new QSGRhiShaderEffectNode(static_cast<QSGDefaultRenderContext *>(renderContext));
 
     return nullptr;
 }
diff --git a/src/quick/scenegraph/qsgdefaultcontext_p.h b/src/quick/scenegraph/qsgdefaultcontext_p.h
index 8fdd29caee..414a4151f1 100644
--- a/src/quick/scenegraph/qsgdefaultcontext_p.h
+++ b/src/quick/scenegraph/qsgdefaultcontext_p.h
@@ -80,8 +80,7 @@ public:
     QSGSpriteNode *createSpriteNode() override;
 #endif
     QSGGuiThreadShaderEffectManager *createGuiThreadShaderEffectManager() override;
-    QSGShaderEffectNode *createShaderEffectNode(QSGRenderContext *renderContext,
-                                                QSGGuiThreadShaderEffectManager *mgr) override;
+    QSGShaderEffectNode *createShaderEffectNode(QSGRenderContext *renderContext) override;
 
     void setDistanceFieldEnabled(bool enabled);
     bool isDistanceFieldEnabled() const;
diff --git a/src/quick/scenegraph/qsgrhishadereffectnode.cpp b/src/quick/scenegraph/qsgrhishadereffectnode.cpp
index 9ec9e040fb..5d3b158de8 100644
--- a/src/quick/scenegraph/qsgrhishadereffectnode.cpp
+++ b/src/quick/scenegraph/qsgrhishadereffectnode.cpp
@@ -568,10 +568,8 @@ void QSGRhiShaderEffectMaterial::updateTextureProviders(bool layoutChange)
     }
 }
 
-QSGRhiShaderEffectNode::QSGRhiShaderEffectNode(QSGDefaultRenderContext *rc, QSGRhiGuiThreadShaderEffectManager *mgr)
-    : QSGShaderEffectNode(mgr),
-      m_rc(rc),
-      m_mgr(mgr),
+QSGRhiShaderEffectNode::QSGRhiShaderEffectNode(QSGDefaultRenderContext *rc)
+    : m_rc(rc),
       m_material(this)
 {
     setFlag(UsePreprocess, true);
@@ -749,7 +747,7 @@ void QSGRhiShaderEffectNode::syncMaterial(SyncData *syncData)
 void QSGRhiShaderEffectNode::handleTextureChange()
 {
     markDirty(QSGNode::DirtyMaterial);
-    emit m_mgr->textureChanged();
+    emit textureChanged();
 }
 
 void QSGRhiShaderEffectNode::handleTextureProviderDestroyed(QObject *object)
diff --git a/src/quick/scenegraph/qsgrhishadereffectnode_p.h b/src/quick/scenegraph/qsgrhishadereffectnode_p.h
index 26460d24b2..fb98bbf10e 100644
--- a/src/quick/scenegraph/qsgrhishadereffectnode_p.h
+++ b/src/quick/scenegraph/qsgrhishadereffectnode_p.h
@@ -59,7 +59,6 @@ QT_BEGIN_NAMESPACE
 class QSGDefaultRenderContext;
 class QSGPlainTexture;
 class QSGRhiShaderEffectNode;
-class QSGRhiGuiThreadShaderEffectManager;
 class QFileSelector;
 
 class QSGRhiShaderLinker
@@ -121,12 +120,12 @@ public:
     QSGPlainTexture *m_dummyTexture = nullptr;
 };
 
-class QSGRhiShaderEffectNode : public QObject, public QSGShaderEffectNode
+class QSGRhiShaderEffectNode : public QSGShaderEffectNode
 {
     Q_OBJECT
 
 public:
-    QSGRhiShaderEffectNode(QSGDefaultRenderContext *rc, QSGRhiGuiThreadShaderEffectManager *mgr);
+    QSGRhiShaderEffectNode(QSGDefaultRenderContext *rc);
 
     QRectF updateNormalizedTextureSubRect(bool supportsAtlasTextures) override;
     void syncMaterial(SyncData *syncData) override;
@@ -140,7 +139,6 @@ private Q_SLOTS:
 
 private:
     QSGDefaultRenderContext *m_rc;
-    QSGRhiGuiThreadShaderEffectManager *m_mgr;
     QSGRhiShaderEffectMaterial m_material;
 };