Skip to content
Permalink
Browse files

Play it safe and work with clones of symbol renderers

This fixes some definite crashes, e.g. saving a vector layer
with symbology, which is caused by accessing the same
layer renderer across different threads.

But I've also swapped a lot of non-threaded code to use
clones of the renderers for extra safety.
  • Loading branch information
nyalldawson committed Nov 19, 2017
1 parent d13eaa6 commit fe535c5f775f099a22c9aae0ddbb152e85f59752
@@ -70,9 +70,10 @@ void QgsMapToolPointSymbol::canvasPressEvent( QgsMapMouseEvent *e )
}

//check whether selected feature has a modifiable symbol
QgsFeatureRenderer *renderer = mActiveLayer->renderer();
if ( !renderer )
if ( !mActiveLayer->renderer() )
return;

std::unique_ptr< QgsFeatureRenderer > renderer( mActiveLayer->renderer()->clone() );
QgsRenderContext context = QgsRenderContext::fromMapSettings( mCanvas->mapSettings() );
context.expressionContext() << QgsExpressionContextUtils::layerScope( mActiveLayer );
context.expressionContext().setFeature( feature );
@@ -229,9 +229,12 @@ QgsFeatureIds QgsMapToolSelectUtils::getMatchingFeatures( QgsMapCanvas *canvas,

QgsRenderContext context = QgsRenderContext::fromMapSettings( canvas->mapSettings() );
context.expressionContext() << QgsExpressionContextUtils::layerScope( vlayer );
QgsFeatureRenderer *r = vlayer->renderer();
if ( r )
std::unique_ptr< QgsFeatureRenderer > r;
if ( vlayer->renderer() )
{
r.reset( vlayer->renderer()->clone() );
r->startRender( context, vlayer->fields() );
}

QgsFeatureRequest request;
request.setFilterRect( selectGeomTrans.boundingBox() );
@@ -982,13 +982,14 @@ void QgsDxfExport::writeEntities()
}

QgsSymbolRenderContext sctx( ctx, QgsUnitTypes::RenderMillimeters, 1.0, false, 0, nullptr );
QgsFeatureRenderer *renderer = vl->renderer();
if ( !renderer )
if ( !vl->renderer() )
{
if ( hasStyleOverride )
vl->styleManager()->restoreOverrideStyle();
continue;
}

std::unique_ptr< QgsFeatureRenderer > renderer( vl->renderer()->clone() );
renderer->startRender( ctx, vl->fields() );

QSet<QString> attributes = renderer->usedAttributes( ctx );
@@ -1118,12 +1119,12 @@ void QgsDxfExport::writeEntitiesSymbolLevels( QgsVectorLayer *layer )
return;
}

QgsFeatureRenderer *renderer = layer->renderer();
if ( !renderer )
if ( !layer->renderer() )
{
// TODO return error
return;
}
std::unique_ptr< QgsFeatureRenderer > renderer( layer->renderer()->clone() );
QHash< QgsSymbol *, QList<QgsFeature> > features;

QgsRenderContext ctx = renderContext();
@@ -106,7 +106,7 @@ void QgsMapHitTest::runHitTestLayer( QgsVectorLayer *vl, SymbolSet &usedSymbols,
if ( hasStyleOverride )
vl->styleManager()->setOverrideStyle( mSettings.layerStyleOverrides().value( vl->id() ) );

QgsFeatureRenderer *r = vl->renderer();
std::unique_ptr< QgsFeatureRenderer > r( vl->renderer()->clone() );
bool moreSymbolsPerFeature = r->capabilities() & QgsFeatureRenderer::MoreSymbolsPerFeature;
r->startRender( context, vl->fields() );

@@ -2655,7 +2655,7 @@ QgsVectorFileWriter::writeAsVectorFormat( QgsVectorLayer *layer,
n++;
}

writer->stopRender( layer );
writer->stopRender();
delete writer;

if ( errors > 0 && errorMessage && n > 0 )
@@ -3135,7 +3135,7 @@ QgsVectorFileWriter::WriterError QgsVectorFileWriter::exportFeaturesSymbolLevels
}
}

stopRender( layer );
stopRender();

if ( nErrors > 0 && errorMessage )
{
@@ -3181,46 +3181,44 @@ double QgsVectorFileWriter::mapUnitScaleFactor( double scale, QgsUnitTypes::Rend

void QgsVectorFileWriter::startRender( QgsVectorLayer *vl )
{
QgsFeatureRenderer *renderer = symbologyRenderer( vl );
if ( !renderer )
mRenderer = createSymbologyRenderer( vl );
if ( !mRenderer )
{
return;
}

renderer->startRender( mRenderContext, vl->fields() );
mRenderer->startRender( mRenderContext, vl->fields() );
}

void QgsVectorFileWriter::stopRender( QgsVectorLayer *vl )
void QgsVectorFileWriter::stopRender()
{
QgsFeatureRenderer *renderer = symbologyRenderer( vl );
if ( !renderer )
if ( !mRenderer )
{
return;
}

renderer->stopRender( mRenderContext );
mRenderer->stopRender( mRenderContext );
}

QgsFeatureRenderer *QgsVectorFileWriter::symbologyRenderer( QgsVectorLayer *vl ) const
std::unique_ptr<QgsFeatureRenderer> QgsVectorFileWriter::createSymbologyRenderer( QgsVectorLayer *vl ) const
{
if ( mSymbologyExport == NoSymbology )
{
return nullptr;
}
if ( !vl )
if ( !vl || !vl->renderer() )
{
return nullptr;
}

return vl->renderer();
return std::unique_ptr< QgsFeatureRenderer >( vl->renderer()->clone() );
}

void QgsVectorFileWriter::addRendererAttributes( QgsVectorLayer *vl, QgsAttributeList &attList )
{
QgsFeatureRenderer *renderer = symbologyRenderer( vl );
if ( renderer )
if ( mRenderer )
{
const QSet<QString> rendererAttributes = renderer->usedAttributes( mRenderContext );
const QSet<QString> rendererAttributes = mRenderer->usedAttributes( mRenderContext );
for ( const QString &attr : rendererAttributes )
{
int index = vl->fields().lookupField( attr );
@@ -729,6 +729,7 @@ class CORE_EXPORT QgsVectorFileWriter : public QgsFeatureSink
QgsVectorFileWriter::ActionOnExistingFile action );
void resetMap( const QgsAttributeList &attributes );

std::unique_ptr< QgsFeatureRenderer > mRenderer;
QgsRenderContext mRenderContext;

bool mUsingTransaction = false;
@@ -744,8 +745,8 @@ class CORE_EXPORT QgsVectorFileWriter : public QgsFeatureSink
double mapUnitScaleFactor( double scale, QgsUnitTypes::RenderUnit symbolUnits, QgsUnitTypes::DistanceUnit mapUnits );

void startRender( QgsVectorLayer *vl );
void stopRender( QgsVectorLayer *vl );
QgsFeatureRenderer *symbologyRenderer( QgsVectorLayer *vl ) const;
void stopRender();
std::unique_ptr< QgsFeatureRenderer > createSymbologyRenderer( QgsVectorLayer *vl ) const;
//! Adds attributes needed for classification
void addRendererAttributes( QgsVectorLayer *vl, QgsAttributeList &attList );
static QMap<QString, MetaData> sDriverMetadata;
@@ -103,13 +103,13 @@ void QgsHighlight::setFillColor( const QColor &fillColor )
mBrush.setStyle( Qt::SolidPattern );
}

QgsFeatureRenderer *QgsHighlight::getRenderer( QgsRenderContext &context, const QColor &color, const QColor &fillColor )
std::unique_ptr<QgsFeatureRenderer> QgsHighlight::createRenderer( QgsRenderContext &context, const QColor &color, const QColor &fillColor )
{
QgsFeatureRenderer *renderer = nullptr;
std::unique_ptr<QgsFeatureRenderer> renderer;
QgsVectorLayer *layer = qobject_cast<QgsVectorLayer *>( mLayer );
if ( layer && layer->renderer() )
{
renderer = layer->renderer()->clone();
renderer.reset( layer->renderer()->clone() );
}
if ( renderer )
{
@@ -328,23 +328,23 @@ void QgsHighlight::paint( QPainter *p )
QColor tmpColor( 255, 0, 0, 255 );
QColor tmpFillColor( 0, 255, 0, 255 );

QgsFeatureRenderer *renderer = getRenderer( context, tmpColor, tmpFillColor );
std::unique_ptr< QgsFeatureRenderer > renderer = createRenderer( context, tmpColor, tmpFillColor );
if ( layer && renderer )
{

QSize imageSize( mMapCanvas->mapSettings().outputSize() );
QImage image = QImage( imageSize.width(), imageSize.height(), QImage::Format_ARGB32 );
image.fill( 0 );
QPainter *imagePainter = new QPainter( &image );
imagePainter->setRenderHint( QPainter::Antialiasing, true );
QPainter imagePainter( &image );
imagePainter.setRenderHint( QPainter::Antialiasing, true );

context.setPainter( imagePainter );
context.setPainter( &imagePainter );

renderer->startRender( context, layer->fields() );
renderer->renderFeature( mFeature, context );
renderer->stopRender( context );

imagePainter->end();
imagePainter.end();

QColor color( mPen.color() ); // true output color
// coefficient to subtract alpha using green (temporary fill)
@@ -366,9 +366,6 @@ void QgsHighlight::paint( QPainter *p )
}

p->drawImage( 0, 0, image );

delete imagePainter;
delete renderer;
}
}
}
@@ -120,7 +120,7 @@ class GUI_EXPORT QgsHighlight: public QgsMapCanvasItem
void setSymbol( QgsSymbol *symbol, const QgsRenderContext &context, const QColor &color, const QColor &fillColor );
double getSymbolWidth( const QgsRenderContext &context, double width, QgsUnitTypes::RenderUnit unit );
//! Get renderer for current color mode and colors. The renderer should be freed by caller.
QgsFeatureRenderer *getRenderer( QgsRenderContext &context, const QColor &color, const QColor &fillColor );
std::unique_ptr< QgsFeatureRenderer > createRenderer( QgsRenderContext &context, const QColor &color, const QColor &fillColor );
void paintPoint( QPainter *p, const QgsPointXY &point );
void paintLine( QPainter *p, QgsPolylineXY line );
void paintPolygon( QPainter *p, const QgsPolygonXY &polygon );
@@ -253,8 +253,8 @@ bool QgsMapToolIdentify::identifyVectorLayer( QList<IdentifyResult> *results, Qg

QgsRenderContext context( QgsRenderContext::fromMapSettings( mCanvas->mapSettings() ) );
context.expressionContext() << QgsExpressionContextUtils::layerScope( layer );
QgsFeatureRenderer *renderer = layer->renderer();
if ( renderer && renderer->capabilities() & QgsFeatureRenderer::ScaleDependent )
std::unique_ptr< QgsFeatureRenderer > renderer( layer->renderer() ? layer->renderer()->clone() : nullptr );
if ( renderer )
{
// setup scale for scale dependent visibility (rule based)
renderer->startRender( context, layer->fields() );
@@ -280,7 +280,7 @@ bool QgsMapToolIdentify::identifyVectorLayer( QList<IdentifyResult> *results, Qg
results->append( IdentifyResult( qobject_cast<QgsMapLayer *>( layer ), *f_it, derivedAttributes ) );
}

if ( renderer && renderer->capabilities() & QgsFeatureRenderer::ScaleDependent )
if ( renderer )
{
renderer->stopRender( context );
}
@@ -255,7 +255,7 @@ namespace QgsWms

void QgsRenderer::runHitTestLayer( QgsVectorLayer *vl, SymbolSet &usedSymbols, QgsRenderContext &context ) const
{
QgsFeatureRenderer *r = vl->renderer();
std::unique_ptr< QgsFeatureRenderer > r( vl->renderer()->clone() );
bool moreSymbolsPerFeature = r->capabilities() & QgsFeatureRenderer::MoreSymbolsPerFeature;
r->startRender( context, vl->pendingFields() );
QgsFeature f;
@@ -1476,7 +1476,7 @@ namespace QgsWms
#endif

QgsFeatureIterator fit = layer->getFeatures( fReq );
QgsFeatureRenderer *r2 = layer->renderer();
std::unique_ptr< QgsFeatureRenderer > r2( layer->renderer() ? layer->renderer()->clone() : nullptr );
if ( r2 )
{
r2->startRender( renderContext, layer->pendingFields() );

0 comments on commit fe535c5

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