Skip to content
Permalink
Browse files

QgsSvgCache fetches remote SVG files in a background task

Previously QgsSvgCache would often try to fetch remote images
using a network request on the main thread, by calling
processEvents repeatedly until the request was complete.

This caused lots of bugs, since the main thread processEvents
would proceed with all kinds of stuff assuming that the
svg fetch operation was complete, leading to frequent crashes
and deadlocks and making remote svg use impossible (it's
likely that the SVG cache remote fetching code was written
in the pre-multi-threaded rendering era).

There's no way to fix this with async svg fetching - we
HAVE to remove the processEvents call, and a QEventLoop
won't help either (since the method may be called on the
main thread). Accordingly the only solution is to
fetch the requested svg in the background, and return
a temporary "downloading" svg for use in the meantime.
We use a QgsNetworkContentFetcherTask to do this, so it's
nicely integrated with task manager.

A request task is fired up when a remote svg is requested
for the first time, with the temporary downloading svg
returned for use by the caller asynchronously. QgsSvgCache
then emits the remoteSvgFetched signal when a previously
requested remote SVG has been successfully fetched,
triggering a map canvas redraw with the correct SVG
graphic.

Fixes #18504

(cherry-picked from 45c400c)
  • Loading branch information
nyalldawson committed Apr 9, 2018
1 parent 5de0bdc commit da0de64998e807ea282ddc8da78e4246c6ccff21
@@ -138,6 +138,13 @@ Get SVG content
void statusChanged( const QString &statusQString );
%Docstring
Emit a signal to be caught by qgisapp and display a msg on status bar
%End

void remoteSvgFetched( const QString &url );
%Docstring
Emitted when the cache has finished retrieving an SVG file from a remote ``url``.

.. versionadded:: 3.2
%End

};
@@ -112,13 +112,6 @@ Check whether images of rendered layers are curerently being cached
Make sure to remove any rendered images from cache (does nothing if cache is not enabled)

.. versionadded:: 2.4
%End

void refreshAllLayers();
%Docstring
Reload all layers, clear the cache and refresh the canvas

.. versionadded:: 2.9
%End

void waitWhileRendering();
@@ -721,6 +714,13 @@ for the canvas.
void refresh();
%Docstring
Repaints the canvas map
%End

void refreshAllLayers();
%Docstring
Reload all layers, clear the cache and refresh the canvas

.. versionadded:: 2.9
%End

void selectionChangedSlot();
@@ -21,6 +21,7 @@
#include "qgsnetworkaccessmanager.h"
#include "qgsmessagelog.h"
#include "qgssymbollayerutils.h"
#include "qgsnetworkcontentfetchertask.h"

#include <QApplication>
#include <QCoreApplication>
@@ -80,8 +81,10 @@ int QgsSvgCacheEntry::dataSize() const

QgsSvgCache::QgsSvgCache( QObject *parent )
: QObject( parent )
, mMutex( QMutex::Recursive )
{
mMissingSvg = QStringLiteral( "<svg width='10' height='10'><text x='5' y='10' font-size='10' text-anchor='middle'>?</text></svg>" ).toLatin1();
mFetchingSvg = QStringLiteral( "<svg width='10' height='10'><text x='5' y='10' font-size='10' text-anchor='middle'>x</text></svg>" ).toLatin1();
}

QgsSvgCache::~QgsSvgCache()
@@ -406,77 +409,70 @@ QByteArray QgsSvgCache::getImageData( const QString &path ) const
return mMissingSvg;
}

// the url points to a remote resource, download it!
QNetworkReply *reply = nullptr;
QMutexLocker locker( &mMutex );

// already a request in progress for this url
if ( mPendingRemoteUrls.contains( path ) )
return mFetchingSvg;

// The following code blocks until the file is downloaded...
// TODO: use signals to get reply finished notification, in this moment
// it's executed while rendering.
while ( true )
if ( mRemoteContentCache.contains( path ) )
{
QgsDebugMsg( QString( "get svg: %1" ).arg( svgUrl.toString() ) );
QNetworkRequest request( svgUrl );
request.setAttribute( QNetworkRequest::CacheLoadControlAttribute, QNetworkRequest::PreferCache );
request.setAttribute( QNetworkRequest::CacheSaveControlAttribute, true );
// already fetched this content - phew. Just return what we already got.
return *mRemoteContentCache[ path ];
}

reply = QgsNetworkAccessManager::instance()->get( request );
connect( reply, &QNetworkReply::downloadProgress, this, &QgsSvgCache::downloadProgress );
mPendingRemoteUrls.insert( path );
//fire up task to fetch image in background
QNetworkRequest request( svgUrl );
request.setAttribute( QNetworkRequest::CacheLoadControlAttribute, QNetworkRequest::PreferCache );
request.setAttribute( QNetworkRequest::CacheSaveControlAttribute, true );

//emit statusChanged( tr( "Downloading svg." ) );
QgsNetworkContentFetcherTask *task = new QgsNetworkContentFetcherTask( request );
connect( task, &QgsNetworkContentFetcherTask::fetched, this, [this, task, path]
{
QMutexLocker locker( &mMutex );

// wait until the image download finished
// TODO: connect to the reply->finished() signal
while ( !reply->isFinished() )
QNetworkReply *reply = task->reply();
if ( !reply )
{
QCoreApplication::processEvents( QEventLoop::ExcludeUserInputEvents, 500 );
// cancelled
QMetaObject::invokeMethod( const_cast< QgsSvgCache * >( this ), "onRemoteSvgFetched", Qt::QueuedConnection, Q_ARG( QString, path ), Q_ARG( bool, false ) );
return;
}

if ( reply->error() != QNetworkReply::NoError )
{
QgsMessageLog::logMessage( tr( "SVG request failed [error: %1 - url: %2]" ).arg( reply->errorString(), path ), tr( "SVG" ) );
reply->deleteLater();
return mMissingSvg;
return;
}

QVariant redirect = reply->attribute( QNetworkRequest::RedirectionTargetAttribute );
if ( redirect.isNull() )
QVariant status = reply->attribute( QNetworkRequest::HttpStatusCodeAttribute );
if ( !status.isNull() && status.toInt() >= 400 )
{
// neither network error nor redirection
// TODO: cache the image
break;
QVariant phrase = reply->attribute( QNetworkRequest::HttpReasonPhraseAttribute );
QgsMessageLog::logMessage( tr( "SVG request error [status: %1 - reason phrase: %2] for %3" ).arg( status.toInt() ).arg( phrase.toString(), path ), tr( "SVG" ) );
mRemoteContentCache.insert( path, new QByteArray( mMissingSvg ) );
return;
}

// do a new request to the redirect url
svgUrl = redirect.toUrl();
reply->deleteLater();
}

QVariant status = reply->attribute( QNetworkRequest::HttpStatusCodeAttribute );
if ( !status.isNull() && status.toInt() >= 400 )
{
QVariant phrase = reply->attribute( QNetworkRequest::HttpReasonPhraseAttribute );
QgsMessageLog::logMessage( tr( "SVG request error [status: %1 - reason phrase: %2] for %3" ).arg( status.toInt() ).arg( phrase.toString(), path ), tr( "SVG" ) );

reply->deleteLater();
return mMissingSvg;
}

// we accept both real SVG mime types AND plain text types - because some sites
// (notably github) serve up svgs as raw text
QString contentType = reply->header( QNetworkRequest::ContentTypeHeader ).toString();
if ( !contentType.startsWith( QLatin1String( "image/svg+xml" ), Qt::CaseInsensitive )
&& !contentType.startsWith( QLatin1String( "text/plain" ), Qt::CaseInsensitive ) )
{
QgsMessageLog::logMessage( tr( "Unexpected MIME type %1 received for %2" ).arg( contentType, path ), tr( "SVG" ) );
reply->deleteLater();
return mMissingSvg;
}
// we accept both real SVG mime types AND plain text types - because some sites
// (notably github) serve up svgs as raw text
QString contentType = reply->header( QNetworkRequest::ContentTypeHeader ).toString();
if ( !contentType.startsWith( QLatin1String( "image/svg+xml" ), Qt::CaseInsensitive )
&& !contentType.startsWith( QLatin1String( "text/plain" ), Qt::CaseInsensitive ) )
{
QgsMessageLog::logMessage( tr( "Unexpected MIME type %1 received for %2" ).arg( contentType, path ), tr( "SVG" ) );
mRemoteContentCache.insert( path, new QByteArray( mMissingSvg ) );
return;
}

// read the image data
QByteArray ba = reply->readAll();
reply->deleteLater();
// read the image data
mRemoteContentCache.insert( path, new QByteArray( reply->readAll() ) );
QMetaObject::invokeMethod( const_cast< QgsSvgCache * >( this ), "onRemoteSvgFetched", Qt::QueuedConnection, Q_ARG( QString, path ), Q_ARG( bool, true ) );
} );

return ba;
QgsApplication::taskManager()->addTask( task );
return mFetchingSvg;
}

void QgsSvgCache::cacheImage( QgsSvgCacheEntry *entry )
@@ -998,3 +994,25 @@ void QgsSvgCache::downloadProgress( qint64 bytesReceived, qint64 bytesTotal )
QgsDebugMsg( msg );
emit statusChanged( msg );
}

void QgsSvgCache::onRemoteSvgFetched( const QString &url, bool success )
{
QMutexLocker locker( &mMutex );
mPendingRemoteUrls.remove( url );

QgsSvgCacheEntry *nextEntry = mLeastRecentEntry;
while ( QgsSvgCacheEntry *entry = nextEntry )
{
nextEntry = entry->nextEntry;
if ( entry->path == url )
{
takeEntryFromList( entry );
mEntryLookup.remove( entry->path, entry );
mTotalSize -= entry->dataSize();
delete entry;
}
}

if ( success )
emit remoteSvgFetched( url );
}
@@ -31,6 +31,8 @@
#include <QElapsedTimer>
#include <QPicture>
#include <QImage>
#include <QCache>
#include <QSet>

#include "qgis_core.h"

@@ -226,9 +228,17 @@ class CORE_EXPORT QgsSvgCache : public QObject
//! Emit a signal to be caught by qgisapp and display a msg on status bar
void statusChanged( const QString &statusQString );

/**
* Emitted when the cache has finished retrieving an SVG file from a remote \a url.
* \since QGIS 3.2
*/
void remoteSvgFetched( const QString &url );

private slots:
void downloadProgress( qint64, qint64 );

void onRemoteSvgFetched( const QString &url, bool success );

private:

/**
@@ -303,11 +313,18 @@ class CORE_EXPORT QgsSvgCache : public QObject
*/
QImage imageFromCachedPicture( const QgsSvgCacheEntry &entry ) const;

QByteArray fetchImageData( const QString &path, bool &ok ) const;

//! SVG content to be rendered if SVG file was not found.
QByteArray mMissingSvg;

QByteArray mFetchingSvg;

//! Mutex to prevent concurrent access to the class from multiple threads at once (may corrupt the entries otherwise).
QMutex mMutex;
mutable QMutex mMutex;

mutable QCache< QString, QByteArray > mRemoteContentCache;
mutable QSet< QString > mPendingRemoteUrls;

friend class TestQgsSvgCache;
};
@@ -65,6 +65,7 @@ email : sherman at mrcc.com
#include "qgsvectorlayer.h"
#include "qgsmapthemecollection.h"
#include "qgscoordinatetransformcontext.h"
#include "qgssvgcache.h"
#include <cmath>

/**
@@ -148,6 +149,9 @@ QgsMapCanvas::QgsMapCanvas( QWidget *parent )
refresh();
} );

// refresh canvas when a remote svg has finished downloading
connect( QgsApplication::svgCache(), &QgsSvgCache::remoteSvgFetched, this, &QgsMapCanvas::refreshAllLayers );

//segmentation parameters
QgsSettings settings;
double segmentationTolerance = settings.value( QStringLiteral( "qgis/segmentationTolerance" ), "0.01745" ).toDouble();
@@ -157,12 +157,6 @@ class GUI_EXPORT QgsMapCanvas : public QGraphicsView
*/
void clearCache();

/**
* Reload all layers, clear the cache and refresh the canvas
* \since QGIS 2.9
*/
void refreshAllLayers();

/**
* Blocks until the rendering job has finished.
*
@@ -643,6 +637,12 @@ class GUI_EXPORT QgsMapCanvas : public QGraphicsView
//! Repaints the canvas map
void refresh();

/**
* Reload all layers, clear the cache and refresh the canvas
* \since QGIS 2.9
*/
void refreshAllLayers();

//! Receives signal about selection change, and pass it on with layer info
void selectionChangedSlot();

0 comments on commit da0de64

Please sign in to comment.
You can’t perform that action at this time.