Prevent mixing up ShaderEffect materials

In 6.5 we tried to eliminate the unlikely, but possible case of a
forever-growing QSGMaterialType cache for ShaderEffects: in the
unlikely event a ShaderEffect continuously assigns new,
not-yet-encountered shaders as its vertex and/or fragment shaders, the
internal per-window data structure that holds a unique QSGMaterialType
value for each QSGMaterial(Shader) grows continuously.

Before 6.5 this table was only cleared when shutting
down. (simplified) To remedy this, 6.5 adds a simple reference
counting system that allows dropping QSGMaterialShader keys that are
not needed anymore, i.e. when all ShaderEffect materials with a given
set of vertex+fragment shaders are destroyed. But this ignored the
fact that 'new' is not a monotonic unique value generator: it is
perfectly possible that a previously encountered pointer is handed out
again after the previous QSGMaterialType with that same address was
deleted, thus the active renderers get confused when looking up their
caches based on the material type due to getting a hit if a given type
value is reused.

So we have no choice now but to make the type cache hold on any unused
(zero ref) QSGMaterialType to prevent key clashes. This reintroduces
the potential memory usage growth in the uncommon cases, but that
cannot be solved as things stand now.

However, as a small remedy, make QQuickWindow::releaseResources()
release this unused data. (this was not the case before 6.5, that
could not do this due to not knowing which QSGMaterialType was really
in use)

This is safe to do only because all renderers on the same render
thread share the same internal ShaderManager object (which in turn
contains all material and shader related caches), so when the
QQuickWindow's main renderer drops its cached resources, the
ShaderManager is emptied, affecting all renderers in the same window
(render thread) (so e.g. all QSGLayers in the same QQuickWindow),
meaning if a QSGMaterialType value pops up again later on, it won't
accidentally generate a hit in the ShaderManager's tables for any
renderers in that window since they all use the same
ShaderManager. See ShaderManager in qsgbatchrenderer_p.h. and in
particular its ShaderKey-based hash tables for more insight.

So long story short, in the unlikely case of materials that
continuously vary their shaders and so their type, the continuous
memory growth can now be controlled as usual by calling
QQuickWindow::releaseResources(). It is not automatic, as the original
patch intended, but better than what was there before 6.5.

Amends c5fb2c10a18a54bb852defc1f1fbf598aecdc90b

Also restores the cleanupMaterialTypeCache (now called
resetMaterialTypeCache) approach from before the above patch.
Leaving things to the destructor is a bad idea when it comes
to the global static object.

Thinking long term, perhaps Qt 7 and beyond, QSGMaterial::type()
should be removed (relying on a pointer as a unique key is a mistake,
even though it makes creating simple materials convenient), and
instead the generation of unique keys and such for materials should be
performed internally in the scenegraph. But this then may also need a
revamp of the material APIs and system altogether, because while the
dynamic behavior a QSGMaterialShader can exhibit is massively reduced
in Qt 6 (it cannot randomly change GL state for instance), there is
still some, e.g. there is no telling what exactly the code in an
updateGraphicsPipelineState() reimplementation alters, or if two
material instances with the same set of shaders but a different
updateSampledImage() implementation are meant to be the same or not,
which in turn means the upfront unique key (type) generation cannot be
done by the framework but must be under the material writer's
control. Therefore it remains to be seen if this is
realistic/sensible. In the meantime we just have to continue juggling
QSGMaterialType pointers as needed.

Pick-to: 6.5
Fixes: QTBUG-109726
Change-Id: I0d3edf17d093773d858f6b796c60e128972142fe
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Kaj Grönholm <kaj.gronholm@qt.io>

1 files changed, 6 insertions, 0 deletions

diff --git a/src/quick/scenegraph/qsgthreadedrenderloop.cpp b/src/quick/scenegraph/qsgthreadedrenderloop.cpp
index 928ebcaffe..5731003197 100644
--- a/src/quick/scenegraph/qsgthreadedrenderloop.cpp
+++ b/src/quick/scenegraph/qsgthreadedrenderloop.cpp
@@ -387,6 +387,9 @@ bool QSGRenderThread::event(QEvent *e)
                     qCDebug(QSG_LOG_RENDERLOOP, QSG_RT_PAD, "- requesting renderer to release cached resources");
                     d->renderer->releaseCachedResources();
                 }
+#if QT_CONFIG(quick_shadereffect)
+                QSGRhiShaderEffectNode::garbageCollectMaterialTypeCache(window);
+#endif
             }
         }
         waitCondition.wakeOne();
@@ -481,6 +484,9 @@ void QSGRenderThread::invalidateGraphics(QQuickWindow *window, bool inDestructor
     // The canvas nodes must be cleaned up regardless if we are in the destructor..
     if (wipeSG) {
         dd->cleanupNodesOnShutdown();
+#if QT_CONFIG(quick_shadereffect)
+        QSGRhiShaderEffectNode::resetMaterialTypeCache(window);
+#endif
     } else {
         qCDebug(QSG_LOG_RENDERLOOP, QSG_RT_PAD, "- persistent SG, avoiding cleanup");
         return;