From 8176a94b8268cf4da851f19c6e8eba44814451ef Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Fri, 14 Feb 2025 10:11:28 +1000 Subject: [PATCH 1/2] More efficient cloning of QgsRasterMarkerSymbolLayer Avoid having to open image file to clone marker Refs #51273 --- src/core/symbology/qgsmarkersymbollayer.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/core/symbology/qgsmarkersymbollayer.cpp b/src/core/symbology/qgsmarkersymbollayer.cpp index a93ab3a254fa..1e7ef1925839 100644 --- a/src/core/symbology/qgsmarkersymbollayer.cpp +++ b/src/core/symbology/qgsmarkersymbollayer.cpp @@ -3116,7 +3116,7 @@ bool QgsRasterMarkerSymbolLayer::setPreservedAspectRatio( bool par ) double QgsRasterMarkerSymbolLayer::updateDefaultAspectRatio() { - if ( mDefaultAspectRatio == 0.0 ) + if ( mDefaultAspectRatio == 0.0 && !mPath.isEmpty() ) { const QSize size = QgsApplication::imageCache()->originalSize( mPath ); mDefaultAspectRatio = ( !size.isNull() && size.isValid() && size.width() > 0 ) ? static_cast< double >( size.height() ) / static_cast< double >( size.width() ) : 0.0; @@ -3365,7 +3365,12 @@ QVariantMap QgsRasterMarkerSymbolLayer::properties() const QgsRasterMarkerSymbolLayer *QgsRasterMarkerSymbolLayer::clone() const { - auto m = std::make_unique< QgsRasterMarkerSymbolLayer >( mPath, mSize, mAngle ); + auto m = std::make_unique< QgsRasterMarkerSymbolLayer >(); + m->mPath = mPath; + m->mDefaultAspectRatio = mDefaultAspectRatio; + m->mSize = mSize; + m->mAngle = mAngle; + // other members are copied by: copyCommonProperties( m.get() ); return m.release(); } From 12cffa3e59474088f109f7c97576cd7ae6925c15 Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Fri, 14 Feb 2025 10:59:35 +1000 Subject: [PATCH 2/2] Cache original image sizes QgsImageCache::originalSize required a re-open and parse of the image with every call, which could result in many file opens to the same image file. Use a cache to store these instead, so that we only have to determine the size once. Note that this isn't as simple as storing the size in a hash, as we need to handle file modifications like QgsImageCache does. Accordingly, we subclass QgsAbstractContentCache and specialise it for handling just the image size here. Fixes #51273 --- src/core/qgsimagecache.cpp | 84 ++++++++++++++++++++++++++++++++++++++ src/core/qgsimagecache.h | 53 ++++++++++++++++++++++++ 2 files changed, 137 insertions(+) diff --git a/src/core/qgsimagecache.cpp b/src/core/qgsimagecache.cpp index 1cef481339d0..d7d75bc0e8dc 100644 --- a/src/core/qgsimagecache.cpp +++ b/src/core/qgsimagecache.cpp @@ -215,6 +215,11 @@ QImage QgsImageCache::pathAsImagePrivate( const QString &f, const QSize size, co } QSize QgsImageCache::originalSize( const QString &path, bool blocking ) const +{ + return mImageSizeCache.originalSize( path, blocking ); +} + +QSize QgsImageCache::originalSizePrivate( const QString &path, bool blocking ) const { if ( path.isEmpty() ) return QSize(); @@ -527,4 +532,83 @@ QImage QgsImageCache::getFrameFromReader( QImageReader &reader, int frameNumber return reader.read(); } +///@cond PRIVATE template class QgsAbstractContentCache; // clazy:exclude=missing-qobject-macro + +QgsImageSizeCacheEntry::QgsImageSizeCacheEntry( const QString &path ) + : QgsAbstractContentCacheEntry( path ) +{ + +} + +int QgsImageSizeCacheEntry::dataSize() const +{ + return sizeof( QSize ); +} + +void QgsImageSizeCacheEntry::dump() const +{ + QgsDebugMsgLevel( QStringLiteral( "path: %1" ).arg( path ), 3 ); +} + +bool QgsImageSizeCacheEntry::isEqual( const QgsAbstractContentCacheEntry *other ) const +{ + const QgsImageSizeCacheEntry *otherImage = dynamic_cast< const QgsImageSizeCacheEntry * >( other ); + if ( !otherImage + || otherImage->path != path ) + return false; + + return true; +} + +template class QgsAbstractContentCache; // clazy:exclude=missing-qobject-macro + + +// +// QgsImageSizeCache +// + +QgsImageSizeCache::QgsImageSizeCache( QObject *parent ) + : QgsAbstractContentCache< QgsImageSizeCacheEntry >( parent, QObject::tr( "Image" ) ) +{ + mMaxCacheSize = 524288; // 500kb max cache size, we are only storing QSize objects here, so that should be heaps +} + +QgsImageSizeCache::~QgsImageSizeCache() = default; + +QSize QgsImageSizeCache::originalSize( const QString &f, bool blocking ) +{ + QString file = f.trimmed(); + + if ( file.isEmpty() ) + return QSize(); + + const QMutexLocker locker( &mMutex ); + + QString base64String; + QString mimeType; + if ( parseBase64DataUrl( file, &mimeType, &base64String ) && mimeType.startsWith( QLatin1String( "image/" ) ) ) + { + file = QStringLiteral( "base64:%1" ).arg( base64String ); + } + + QgsImageSizeCacheEntry *currentEntry = findExistingEntry( new QgsImageSizeCacheEntry( file ) ); + + QSize result; + + if ( !currentEntry->size.isValid() ) + { + result = QgsApplication::imageCache()->originalSizePrivate( file, blocking ); + mTotalSize += currentEntry->dataSize(); + currentEntry->size = result; + trimToMaximumSize(); + } + else + { + result = currentEntry->size; + } + + return result; +} + +///@endcond diff --git a/src/core/qgsimagecache.h b/src/core/qgsimagecache.h index b4376da87fb7..129c99257dfa 100644 --- a/src/core/qgsimagecache.h +++ b/src/core/qgsimagecache.h @@ -32,6 +32,54 @@ class QTemporaryDir; ///@cond PRIVATE +/** + * \ingroup core + * \class QgsImageSizeCacheEntry + * \brief An entry for a QgsImageSizeCache, representing the original size of a single raster image + * \since QGIS 3.42 + */ +class CORE_EXPORT QgsImageSizeCacheEntry : public QgsAbstractContentCacheEntry +{ + public: + + /** + * Constructor for QgsImageSizeCacheEntry, corresponding to the specified image \a path. + */ + QgsImageSizeCacheEntry( const QString &path ) ; + + //! Original image size + QSize size; + + int dataSize() const override; + void dump() const override; + bool isEqual( const QgsAbstractContentCacheEntry *other ) const override; + +}; + +/** + * \class QgsImageSizeCache + * \ingroup core + * \brief A cache for original image sizes. + * + * QgsImageSizeCache stores the original sizes of raster image files, allowing efficient + * reuse without incurring the cost of re-opening and parsing the image on every render. + * + * This is a private class, used internally in QgsImageCache. + * + * \since QGIS 3.42 +*/ +class CORE_EXPORT QgsImageSizeCache : public QgsAbstractContentCache< QgsImageSizeCacheEntry > +{ + Q_OBJECT + + public: + + QgsImageSizeCache( QObject *parent SIP_TRANSFERTHIS = nullptr ); + ~QgsImageSizeCache() override; + long maximumSize() const { return mMaxCacheSize; } + QSize originalSize( const QString &path, bool blocking = false ); +}; + /** * \ingroup core * \class QgsImageCacheEntry @@ -256,6 +304,8 @@ class CORE_EXPORT QgsImageCache : public QgsAbstractContentCache< QgsImageCacheE static QImage getFrameFromReader( QImageReader &reader, int frameNumber ); + QSize originalSizePrivate( const QString &path, bool blocking = false ) const; + //! SVG content to be rendered if SVG file was not found. QByteArray mMissingSvg; @@ -266,6 +316,9 @@ class CORE_EXPORT QgsImageCache : public QgsAbstractContentCache< QgsImageCacheE QMap< QString, int > mTotalFrameCounts; QMap< QString, QVector< int > > mImageDelays; + mutable QgsImageSizeCache mImageSizeCache; + + friend class QgsImageSizeCache; friend class TestQgsImageCache; };