[frameworks/ksvg] /: SvgItem: hold one texture for one picture, like FrameSvgItem does

Git commit c35038c1f73d1e0cbeae72128acfe0efffa435fb by Méven Car.
Committed on 17/08/2026 at 17:15.
Pushed by meven into branch 'master'.

SvgItem: hold one texture for one picture, like FrameSvgItem does

SvgItem uploaded its own texture straight from the window, so every item drawing the same element at the
same size uploaded the same picture again and took an atlas entry of its own. Counted with texture
accounting over one plasmashell session, widgets/arrows down-arrow was uploaded 69 times for 4 distinct
sizes, 60 of them the very same 25x25, and not one was served from a cache. A FrameSvgItem in the same
session was served 88 of 90 requests.

FrameSvgItem has gone through ImageTexturesCache since it was written, which keys on the image and the
window. KSvg hands back the same image for the same request, so the same key comes out and one texture
serves every item drawing that picture.

The cache is now shared between the two rather than living in framesvgitem.cpp, and it takes a lock: an
item asks from the render thread of its own window, and a window has its own, so with a second caller
the hash is reached from several threads at once.

M  +30   -0    autotests/svgtest.cpp
M  +2    -3    src/declarativeimports/framesvgitem.cpp
M  +13   -0    src/declarativeimports/imagetexturescache.cpp
M  +8    -0    src/declarativeimports/imagetexturescache.h
M  +5    -2    src/declarativeimports/svgitem.cpp

https://invent.kde.org/frameworks/ksvg/-/commit/c35038c1f73d1e0cbeae72128acfe0efffa435fb

diff --git a/autotests/svgtest.cpp b/autotests/svgtest.cpp
index 304c1f39..5526db5d 100644
--- a/autotests/svgtest.cpp
+++ b/autotests/svgtest.cpp
@@ -32,6 +32,7 @@ private Q_SLOTS:
     void testElements();
     void testColors();
     void testStylesheetOverrideColorChange();
+    void theSameImageIsHandedBackForTheSameRequest();
 
 private:
     KSvg::Svg *m_svg;
@@ -99,6 +100,35 @@ void SvgTest::testSize()
     QCOMPARE(m_svg->size(), QSizeF(148, 148));
 }
 
+void SvgTest::theSameImageIsHandedBackForTheSameRequest()
+{
+    // The texture cache the QML items share keys on QImage::cacheKey(), so two items drawing one element
+    // at one size hold a single texture between them only as long as the same image comes back for it.
+    // Sixty list rows showing one arrow are sixty uploads and sixty atlas entries if this stops holding.
+    const QSize size(25, 25);
+
+    // Two items are two Svg objects, so the question is asked of two of them rather than of m_svg, which
+    // the cases before this one leave carrying color overrides.
+    KSvg::Svg one;
+    one.setImagePath(QFINDTESTDATA("data/background.svgz"));
+    QVERIFY(one.isValid());
+    const QImage once = one.image(size, QString());
+    QVERIFY(!once.isNull());
+    QCOMPARE(one.image(size, QString()).cacheKey(), once.cacheKey());
+
+    KSvg::Svg other;
+    other.setImagePath(QFINDTESTDATA("data/background.svgz"));
+    QVERIFY(other.isValid());
+    QCOMPARE(other.image(size, QString()).cacheKey(), once.cacheKey());
+
+    // A different size is a different picture, and has to be a different image.
+    QVERIFY(one.image(QSize(26, 26), QString()).cacheKey() != once.cacheKey());
+
+    // A color override is a different picture too, so it must not be served the same image.
+    other.setColor(KSvg::Svg::StyleSheetColor::Text, Qt::magenta);
+    QVERIFY(other.image(size, QString()).cacheKey() != once.cacheKey());
+}
+
 void SvgTest::testElements()
 {
     QVERIFY(m_svg->hasElement("center"));
diff --git a/src/declarativeimports/framesvgitem.cpp b/src/declarativeimports/framesvgitem.cpp
index dddf7a39..9ead7310 100644
--- a/src/declarativeimports/framesvgitem.cpp
+++ b/src/declarativeimports/framesvgitem.cpp
@@ -28,7 +28,6 @@
 
 namespace KSvg
 {
-Q_GLOBAL_STATIC(ImageTexturesCache, s_cache)
 
 class FrameNode : public QSGNode
 {
@@ -118,7 +117,7 @@ public:
         if (m_fitMode != Tile) {
             options = QQuickWindow::TextureCanUseAtlas;
         }
-        setTexture(s_cache->loadTexture(m_frameSvg->window(), m_frameSvg->frameSvg()->image(size, elementId), options));
+        setTexture(ImageTexturesCache::instance()->loadTexture(m_frameSvg->window(), m_frameSvg->frameSvg()->image(size, elementId), options));
     }
 
     void reposition(const QRect &frameGeometry, QSize &fullSize)
@@ -664,7 +663,7 @@ QSGNode *FrameSvgItem::updatePaintNode(QSGNode *oldNode, QQuickItem::UpdatePaint
 
         if ((m_textureChanged || m_sizeChanged) || textureNode->texture()->textureSize() != m_frameSvg->size()) {
             QImage image = m_frameSvg->framePixmap().toImage();
-            textureNode->setTexture(s_cache->loadTexture(window(), image));
+            textureNode->setTexture(ImageTexturesCache::instance()->loadTexture(window(), image));
             textureNode->setRect(0, 0, width(), height());
 
             m_textureChanged = false;
diff --git a/src/declarativeimports/imagetexturescache.cpp b/src/declarativeimports/imagetexturescache.cpp
index 34f410f7..bd46e428 100644
--- a/src/declarativeimports/imagetexturescache.cpp
+++ b/src/declarativeimports/imagetexturescache.cpp
@@ -5,6 +5,8 @@
 */
 
 #include "imagetexturescache.h"
+#include <QMutex>
+#include <QMutexLocker>
 #include <QSGTexture>
 
 typedef QHash<qint64, QHash<QWindow *, QWeakPointer<QSGTexture>>> TexturesCache;
@@ -13,6 +15,9 @@ class ImageTexturesCachePrivate
 {
 public:
     TexturesCache cache;
+    // Items ask for their textures from the render thread of the window they are in, and a window has its
+    // own, so the cache is reached from several threads at once.
+    QMutex lock;
 };
 
 ImageTexturesCache::ImageTexturesCache()
@@ -24,13 +29,21 @@ ImageTexturesCache::~ImageTexturesCache()
 {
 }
 
+ImageTexturesCache *ImageTexturesCache::instance()
+{
+    static ImageTexturesCache cache;
+    return &cache;
+}
+
 QSharedPointer<QSGTexture> ImageTexturesCache::loadTexture(QQuickWindow *window, const QImage &image, QQuickWindow::CreateTextureOptions options)
 {
     qint64 id = image.cacheKey();
+    QMutexLocker locked(&d->lock);
     QSharedPointer<QSGTexture> texture = d->cache.value(id).value(window).toStrongRef();
 
     if (!texture) {
         auto cleanAndDelete = [this, window, id](QSGTexture *texture) {
+            QMutexLocker locked(&d->lock);
             QHash<QWindow *, QWeakPointer<QSGTexture>> &textures = (d->cache)[id];
             textures.remove(window);
             if (textures.isEmpty()) {
diff --git a/src/declarativeimports/imagetexturescache.h b/src/declarativeimports/imagetexturescache.h
index 910c5b50..614b385d 100644
--- a/src/declarativeimports/imagetexturescache.h
+++ b/src/declarativeimports/imagetexturescache.h
@@ -29,6 +29,14 @@ public:
     ImageTexturesCache();
     ~ImageTexturesCache();
 
+    /*!
+     * Returns the cache every item shares.
+     *
+     * Two items drawing the same picture should hold one texture between them, which only works if they
+     * ask the same cache for it.
+     */
+    static ImageTexturesCache *instance();
+
     /*!
      * Returns the texture for a given window and image.
      *
diff --git a/src/declarativeimports/svgitem.cpp b/src/declarativeimports/svgitem.cpp
index fd37ebd9..fb19b39e 100644
--- a/src/declarativeimports/svgitem.cpp
+++ b/src/declarativeimports/svgitem.cpp
@@ -14,6 +14,7 @@
 
 #include "ksvg/svg.h"
 
+#include "imagetexturescache.h"
 #include "managedtexturenode.h"
 
 #include <KColorScheme>
@@ -222,8 +223,10 @@ QSGNode *SvgItem::updatePaintNode(QSGNode *oldNode, UpdatePaintNodeData *updateP
             return nullptr;
         }
 
-        QSharedPointer<QSGTexture> texture(window()->createTextureFromImage(m_image, QQuickWindow::TextureCanUseAtlas));
-        textureNode->setTexture(texture);
+        // The same element at the same size is one picture however many items draw it, and KSvg hands back
+        // the same image for it, so one texture is enough: an arrow in sixty list rows was sixty uploads
+        // and sixty atlas entries before this.
+        textureNode->setTexture(ImageTexturesCache::instance()->loadTexture(window(), m_image, QQuickWindow::TextureCanUseAtlas));
         m_textureChanged = false;
 
         textureNode->setRect(0, 0, width(), height());