Fix detection of broken images in layouts (master only)

qgis · Apr 21, 2020 · 5c587c0 · 5c587c0
1 parent 2ee717e
commit 5c587c0
Show file tree

Hide file tree

Showing 7 changed files with 93 additions and 12 deletions.
diff --git a/src/core/layout/qgslayoutitempicture.cpp b/src/core/layout/qgslayoutitempicture.cpp
@@ -446,6 +446,12 @@ void QgsLayoutItemPicture::loadLocalPicture( const QString &path )
 
 void QgsLayoutItemPicture::loadPictureUsingCache( const QString &path )
 {
+  if ( path.isEmpty() )
+  {
+    mImage = QImage();
+    return;
+  }
+
   switch ( mMode )
   {
     case FormatUnknown:
@@ -454,7 +460,10 @@ void QgsLayoutItemPicture::loadPictureUsingCache( const QString &path )
     case FormatRaster:
     {
       bool fitsInCache = false;
-      mImage = QgsApplication::imageCache()->pathAsImage( path, QSize(), true, 1, fitsInCache, true );
+      bool isMissing = false;
+      mImage = QgsApplication::imageCache()->pathAsImage( path, QSize(), true, 1, fitsInCache, true, &isMissing );
+      if ( mImage.isNull() || isMissing )
+        mMode = FormatUnknown;
       break;
     }
 
@@ -464,10 +473,11 @@ void QgsLayoutItemPicture::loadPictureUsingCache( const QString &path )
       QColor fillColor = mDataDefinedProperties.valueAsColor( QgsLayoutObject::PictureSvgBackgroundColor, context, mSvgFillColor );
       QColor strokeColor = mDataDefinedProperties.valueAsColor( QgsLayoutObject::PictureSvgStrokeColor, context, mSvgStrokeColor );
       double strokeWidth = mDataDefinedProperties.valueAsDouble( QgsLayoutObject::PictureSvgStrokeWidth, context, mSvgStrokeWidth );
+      bool isMissingImage = false;
       const QByteArray &svgContent = QgsApplication::svgCache()->svgContent( path, rect().width(), fillColor, strokeColor, strokeWidth,
-                                     1.0 );
+                                     1.0, 0, false, &isMissingImage );
       mSVG.load( svgContent );
-      if ( mSVG.isValid() )
+      if ( mSVG.isValid() && !isMissingImage )
       {
         mMode = FormatSVG;
         QRect viewBox = mSVG.viewBox(); //take width/height ratio from view box instead of default size

diff --git a/src/core/qgsimagecache.cpp b/src/core/qgsimagecache.cpp
@@ -101,9 +101,11 @@ QgsImageCache::QgsImageCache( QObject *parent )
   connect( this, &QgsAbstractContentCacheBase::remoteContentFetched, this, &QgsImageCache::remoteImageFetched );
 }
 
-QImage QgsImageCache::pathAsImage( const QString &f, const QSize size, const bool keepAspectRatio, const double opacity, bool &fitsInCache, bool blocking )
+QImage QgsImageCache::pathAsImage( const QString &f, const QSize size, const bool keepAspectRatio, const double opacity, bool &fitsInCache, bool blocking, bool *isMissing )
 {
   const QString file = f.trimmed();
+  if ( isMissing )
+    *isMissing = true;
 
   if ( file.isEmpty() )
     return QImage();
@@ -122,7 +124,8 @@ QImage QgsImageCache::pathAsImage( const QString &f, const QSize size, const boo
   if ( currentEntry->image.isNull() )
   {
     long cachedDataSize = 0;
-    result = renderImage( file, size, keepAspectRatio, opacity, blocking );
+    bool isBroken = false;
+    result = renderImage( file, size, keepAspectRatio, opacity, isBroken, blocking );
     cachedDataSize += result.width() * result.height() * 32;
     if ( cachedDataSize > mMaxCacheSize / 2 )
     {
@@ -134,11 +137,18 @@ QImage QgsImageCache::pathAsImage( const QString &f, const QSize size, const boo
       mTotalSize += ( result.width() * result.height() * 32 );
       currentEntry->image = result;
     }
+
+    if ( isMissing )
+      *isMissing = isBroken;
+    currentEntry->isMissingImage = isBroken;
+
     trimToMaximumSize();
   }
   else
   {
     result = currentEntry->image;
+    if ( isMissing )
+      *isMissing = currentEntry->isMissingImage;
   }
 
   return result;
@@ -180,9 +190,11 @@ QSize QgsImageCache::originalSize( const QString &path, bool blocking ) const
   return QSize();
 }
 
-QImage QgsImageCache::renderImage( const QString &path, QSize size, const bool keepAspectRatio, const double opacity, bool blocking ) const
+QImage QgsImageCache::renderImage( const QString &path, QSize size, const bool keepAspectRatio, const double opacity, bool &isBroken, bool blocking ) const
 {
   QImage im;
+  isBroken = false;
+
   // direct read if path is a file -- maybe more efficient than going the bytearray route? (untested!)
   if ( !path.startsWith( QStringLiteral( "base64:" ) ) && QFile::exists( path ) )
   {
@@ -194,6 +206,8 @@ QImage QgsImageCache::renderImage( const QString &path, QSize size, const bool k
 
     if ( ba == "broken" )
     {
+      isBroken = true;
+
       // if image size is set to respect aspect ratio, correct for broken image aspect ratio
       if ( size.width() == 0 )
         size.setWidth( size.height() );

diff --git a/src/core/qgsimagecache.h b/src/core/qgsimagecache.h
@@ -60,6 +60,13 @@ class CORE_EXPORT QgsImageCacheEntry : public QgsAbstractContentCacheEntry
     //! Rendered, resampled image.
     QImage image;
 
+    /**
+     * TRUE if the image represents a broken/missing path.
+     *
+     * \since QGIS 3.14
+     */
+    bool isMissingImage = false;
+
     int dataSize() const override;
     void dump() const override;
     bool isEqual( const QgsAbstractContentCacheEntry *other ) const override;
@@ -122,7 +129,11 @@ class CORE_EXPORT QgsImageCache : public QgsAbstractContentCache< QgsImageCacheE
      * be TRUE from GUI based applications (like the main QGIS application) or crashes will result. Only for
      * use in external scripts or QGIS server.
      */
+#ifndef SIP_RUN
+    QImage pathAsImage( const QString &path, const QSize size, const bool keepAspectRatio, const double opacity, bool &fitsInCache SIP_OUT, bool blocking = false, bool *isMissing = nullptr );
+#else
     QImage pathAsImage( const QString &path, const QSize size, const bool keepAspectRatio, const double opacity, bool &fitsInCache SIP_OUT, bool blocking = false );
+#endif
 
     /**
      * Returns the original size (in pixels) of the image at the specified \a path.
@@ -150,7 +161,7 @@ class CORE_EXPORT QgsImageCache : public QgsAbstractContentCache< QgsImageCacheE
 
   private:
 
-    QImage renderImage( const QString &path, QSize size, const bool keepAspectRatio, const double opacity, bool blocking = false ) const;
+    QImage renderImage( const QString &path, QSize size, const bool keepAspectRatio, const double opacity, bool &isBroken, bool blocking = false ) const;
 
     //! SVG content to be rendered if SVG file was not found.
     QByteArray mMissingSvg;

diff --git a/src/core/symbology/qgssvgcache.cpp b/src/core/symbology/qgssvgcache.cpp
@@ -204,11 +204,11 @@ QPicture QgsSvgCache::svgAsPicture( const QString &path, double size, const QCol
 }
 
 QByteArray QgsSvgCache::svgContent( const QString &path, double size, const QColor &fill, const QColor &stroke, double strokeWidth,
-                                    double widthScaleFactor, double fixedAspectRatio, bool blocking )
+                                    double widthScaleFactor, double fixedAspectRatio, bool blocking, bool *isMissingImage )
 {
   QMutexLocker locker( &mMutex );
 
-  QgsSvgCacheEntry *currentEntry = cacheEntry( path, size, fill, stroke, strokeWidth, widthScaleFactor, fixedAspectRatio, blocking );
+  QgsSvgCacheEntry *currentEntry = cacheEntry( path, size, fill, stroke, strokeWidth, widthScaleFactor, fixedAspectRatio, blocking, isMissingImage );
 
   return currentEntry->svgContent;
 }
@@ -289,8 +289,10 @@ void QgsSvgCache::replaceParamsAndCacheSvg( QgsSvgCacheEntry *entry, bool blocki
     return;
   }
 
+  const QByteArray content = getContent( entry->path, mMissingSvg, mFetchingSvg, blocking ) ;
+  entry->isMissingImage = content == mMissingSvg;
   QDomDocument svgDoc;
-  if ( !svgDoc.setContent( getContent( entry->path, mMissingSvg, mFetchingSvg, blocking ) ) )
+  if ( !svgDoc.setContent( content ) )
   {
     return;
   }
@@ -305,6 +307,7 @@ void QgsSvgCache::replaceParamsAndCacheSvg( QgsSvgCacheEntry *entry, bool blocki
 
   entry->svgContent = svgDoc.toByteArray( 0 );
 
+
   // toByteArray screws up tspans inside text by adding new lines before and after each span... this should help, at the
   // risk of potentially breaking some svgs where the newline is desired
   entry->svgContent.replace( "\n<tspan", "<tspan" );
@@ -465,7 +468,7 @@ void QgsSvgCache::cachePicture( QgsSvgCacheEntry *entry, bool forceVectorOutput
 }
 
 QgsSvgCacheEntry *QgsSvgCache::cacheEntry( const QString &path, double size, const QColor &fill, const QColor &stroke, double strokeWidth,
-    double widthScaleFactor, double fixedAspectRatio, bool blocking )
+    double widthScaleFactor, double fixedAspectRatio, bool blocking, bool *isMissingImage )
 {
   QgsSvgCacheEntry *currentEntry = findExistingEntry( new QgsSvgCacheEntry( path, size, strokeWidth, widthScaleFactor, fill, stroke, fixedAspectRatio ) );
 
@@ -474,6 +477,9 @@ QgsSvgCacheEntry *QgsSvgCache::cacheEntry( const QString &path, double size, con
     replaceParamsAndCacheSvg( currentEntry, blocking );
   }
 
+  if ( isMissingImage )
+    *isMissingImage = currentEntry->isMissingImage;
+
   return currentEntry;
 }
 

diff --git a/src/core/symbology/qgssvgcache.h b/src/core/symbology/qgssvgcache.h
@@ -75,6 +75,13 @@ class CORE_EXPORT QgsSvgCacheEntry : public QgsAbstractContentCacheEntry
     //content (with params replaced)
     QByteArray svgContent;
 
+    /**
+     * TRUE if the image represents a broken/missing path.
+     *
+     * \since QGIS 3.14
+     */
+    bool isMissingImage = false;
+
     bool isEqual( const QgsAbstractContentCacheEntry *other ) const override;
     int dataSize() const override;
     void dump() const override;
@@ -232,8 +239,13 @@ class CORE_EXPORT QgsSvgCache : public QgsAbstractContentCache< QgsSvgCacheEntry
      * be TRUE from GUI based applications (like the main QGIS application) or crashes will result. Only for
      * use in external scripts or QGIS server.
      */
+#ifndef SIP_RUN
+    QByteArray svgContent( const QString &path, double size, const QColor &fill, const QColor &stroke, double strokeWidth,
+                           double widthScaleFactor, double fixedAspectRatio = 0, bool blocking = false, bool *isMissingImage = nullptr );
+#else
     QByteArray svgContent( const QString &path, double size, const QColor &fill, const QColor &stroke, double strokeWidth,
                            double widthScaleFactor, double fixedAspectRatio = 0, bool blocking = false );
+#endif
 
   signals:
 
@@ -260,7 +272,7 @@ class CORE_EXPORT QgsSvgCache : public QgsAbstractContentCache< QgsSvgCacheEntry
     void cachePicture( QgsSvgCacheEntry *entry, bool forceVectorOutput = false );
     //! Returns entry from cache or creates a new entry if it does not exist already
     QgsSvgCacheEntry *cacheEntry( const QString &path, double size, const QColor &fill, const QColor &stroke, double strokeWidth,
-                                  double widthScaleFactor, double fixedAspectRatio = 0, bool blocking = false );
+                                  double widthScaleFactor, double fixedAspectRatio = 0, bool blocking = false, bool *isMissingImage = nullptr );
 
     //! Replaces parameters in elements of a dom node and calls method for all child nodes
     void replaceElemParams( QDomElement &elem, const QColor &fill, const QColor &stroke, double strokeWidth );

diff --git a/tests/src/core/testqgsimagecache.cpp b/tests/src/core/testqgsimagecache.cpp
@@ -49,6 +49,7 @@ class TestQgsImageCache : public QObject
     void cleanup() {} // will be called after every testfunction.
     void fillCache();
     void threadSafeImage();
+    void broken();
     void changeImage(); // check that cache is updated if image source file changes
     void size(); // check various size-specific handling
     void opacity(); // check non-opaque image rendering
@@ -138,6 +139,20 @@ void TestQgsImageCache::threadSafeImage()
   QtConcurrent::blockingMap( list, RenderImageWrapper( cache, imagePath ) );
 }
 
+void TestQgsImageCache::broken()
+{
+  QgsImageCache cache;
+  bool inCache = false;
+  bool missingImage = false;
+  QImage img = cache.pathAsImage( QStringLiteral( "bbbbbbb" ), QSize( 200, 200 ), true, 1.0, inCache, false, &missingImage );
+  QVERIFY( missingImage );
+  cache.pathAsImage( QStringLiteral( "bbbbbbb" ), QSize( 200, 200 ), true, 1.0, inCache, false, &missingImage );
+  QVERIFY( missingImage );
+  QString originalImage = TEST_DATA_DIR + QStringLiteral( "/sample_image.png" );
+  cache.pathAsImage( originalImage, QSize( 200, 200 ), true, 1.0, inCache, false, &missingImage );
+  QVERIFY( !missingImage );
+}
+
 void TestQgsImageCache::changeImage()
 {
   bool inCache;

diff --git a/tests/src/core/testqgssvgcache.cpp b/tests/src/core/testqgssvgcache.cpp
@@ -48,6 +48,7 @@ class TestQgsSvgCache : public QObject
     void init() {} // will be called before each testfunction is executed.
     void cleanup() {} // will be called after every testfunction.
     void fillCache();
+    void broken();
     void threadSafePicture();
     void threadSafeImage();
     void changeImage(); //check that cache is updated if svg source file changes
@@ -102,6 +103,18 @@ void TestQgsSvgCache::fillCache()
   }
 }
 
+void TestQgsSvgCache::broken()
+{
+  QgsSvgCache cache;
+  bool missingImage = false;
+  QByteArray content = cache.svgContent( QStringLiteral( "bbbbbbb" ), 14, QColor( 0, 0, 0 ), QColor( 255, 255, 255 ), 1, 1, 0, false, &missingImage );
+  QVERIFY( missingImage );
+  content = cache.svgContent( QStringLiteral( "bbbbbbb" ), 14, QColor( 0, 0, 0 ), QColor( 255, 255, 255 ), 1, 1, 0, false, &missingImage );
+  QVERIFY( missingImage );
+  content = cache.svgContent( TEST_DATA_DIR + QStringLiteral( "/sample_svg.svg" ), 14, QColor( 0, 0, 0 ), QColor( 255, 255, 255 ), 1, 1, 0, false, &missingImage );
+  QVERIFY( !missingImage );
+}
+
 struct RenderPictureWrapper
 {
   QgsSvgCache &cache;