Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Make QgsSymbolLayerUtils::symbolPreview* methods const, and utilise
internal symbol clones

Rendering symbols is a NON-CONST operation, which can permanently
alter the symbol instance (e.g. via changes made by the symbol
or symbol layer's startRender methods.).

This makes debugging super complex - because methods which look
like they are just generating previews of symbols can actually
change the original symbol instances, resulting in permanent changes
to a layer's style.

Refs #19910 (specifically, me pulling my hair out trying to deduce
seemingly random changes to layer's symbols)
  • Loading branch information
nyalldawson committed Jan 25, 2019
1 parent 431861a commit 4022c5f
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 19 deletions.
Expand Up @@ -159,7 +159,7 @@ Returns the size scaled in pixels according to the uom attribute.

static QPainter::CompositionMode decodeBlendMode( const QString &s );

static QIcon symbolPreviewIcon( QgsSymbol *symbol, QSize size, int padding = 0 );
static QIcon symbolPreviewIcon( const QgsSymbol *symbol, QSize size, int padding = 0 );
%Docstring
Returns an icon preview for a color ramp.

Expand All @@ -170,7 +170,7 @@ Returns an icon preview for a color ramp.
.. seealso:: :py:func:`symbolPreviewPixmap`
%End

static QPixmap symbolPreviewPixmap( QgsSymbol *symbol, QSize size, int padding = 0, QgsRenderContext *customContext = 0 );
static QPixmap symbolPreviewPixmap( const QgsSymbol *symbol, QSize size, int padding = 0, QgsRenderContext *customContext = 0 );
%Docstring
Returns a pixmap preview for a color ramp.

Expand All @@ -186,7 +186,7 @@ Returns a pixmap preview for a color ramp.
.. seealso:: :py:func:`symbolPreviewIcon`
%End

static QPicture symbolLayerPreviewPicture( QgsSymbolLayer *layer, QgsUnitTypes::RenderUnit units, QSize size, const QgsMapUnitScale &scale = QgsMapUnitScale() );
static QPicture symbolLayerPreviewPicture( const QgsSymbolLayer *layer, QgsUnitTypes::RenderUnit units, QSize size, const QgsMapUnitScale &scale = QgsMapUnitScale() );
%Docstring
Draws a symbol layer preview to a QPicture

Expand All @@ -202,7 +202,7 @@ Draws a symbol layer preview to a QPicture
.. versionadded:: 2.9
%End

static QIcon symbolLayerPreviewIcon( QgsSymbolLayer *layer, QgsUnitTypes::RenderUnit u, QSize size, const QgsMapUnitScale &scale = QgsMapUnitScale() );
static QIcon symbolLayerPreviewIcon( const QgsSymbolLayer *layer, QgsUnitTypes::RenderUnit u, QSize size, const QgsMapUnitScale &scale = QgsMapUnitScale() );
%Docstring
Draws a symbol layer preview to an icon.

Expand Down
5 changes: 2 additions & 3 deletions src/app/qgsmaptoolrotatepointsymbols.cpp
Expand Up @@ -220,11 +220,10 @@ void QgsMapToolRotatePointSymbols::createPixmapItem( QgsMarkerSymbol *markerSymb

if ( markerSymbol )
{
QgsSymbol *clone = markerSymbol->clone();
QgsMarkerSymbol *markerClone = static_cast<QgsMarkerSymbol *>( clone );
std::unique_ptr< QgsSymbol > clone( markerSymbol->clone() );
QgsMarkerSymbol *markerClone = static_cast<QgsMarkerSymbol *>( clone.get() );
markerClone->setDataDefinedAngle( QgsProperty() );
pointImage = markerClone->bigSymbolPreviewImage();
delete clone;
}

mRotationItem = new QgsPointRotationItem( mCanvas );
Expand Down
17 changes: 10 additions & 7 deletions src/core/symbology/qgssymbollayerutils.cpp
Expand Up @@ -645,12 +645,12 @@ QPainter::CompositionMode QgsSymbolLayerUtils::decodeBlendMode( const QString &s
return QPainter::CompositionMode_SourceOver; // "Normal"
}

QIcon QgsSymbolLayerUtils::symbolPreviewIcon( QgsSymbol *symbol, QSize size, int padding )
QIcon QgsSymbolLayerUtils::symbolPreviewIcon( const QgsSymbol *symbol, QSize size, int padding )
{
return QIcon( symbolPreviewPixmap( symbol, size, padding ) );
}

QPixmap QgsSymbolLayerUtils::symbolPreviewPixmap( QgsSymbol *symbol, QSize size, int padding, QgsRenderContext *customContext )
QPixmap QgsSymbolLayerUtils::symbolPreviewPixmap( const QgsSymbol *symbol, QSize size, int padding, QgsRenderContext *customContext )
{
Q_ASSERT( symbol );
QPixmap pixmap( size );
Expand Down Expand Up @@ -694,7 +694,8 @@ QPixmap QgsSymbolLayerUtils::symbolPreviewPixmap( QgsSymbol *symbol, QSize size,
}
else
{
symbol->drawPreviewIcon( &painter, size, customContext );
std::unique_ptr<QgsSymbol> symbolClone( symbol->clone( ) );
symbolClone->drawPreviewIcon( &painter, size, customContext );
}

painter.end();
Expand All @@ -714,7 +715,7 @@ double QgsSymbolLayerUtils::estimateMaxSymbolBleed( QgsSymbol *symbol, const Qgs
return maxBleed;
}

QPicture QgsSymbolLayerUtils::symbolLayerPreviewPicture( QgsSymbolLayer *layer, QgsUnitTypes::RenderUnit units, QSize size, const QgsMapUnitScale &scale )
QPicture QgsSymbolLayerUtils::symbolLayerPreviewPicture( const QgsSymbolLayer *layer, QgsUnitTypes::RenderUnit units, QSize size, const QgsMapUnitScale &scale )
{
QPicture picture;
QPainter painter;
Expand All @@ -723,12 +724,13 @@ QPicture QgsSymbolLayerUtils::symbolLayerPreviewPicture( QgsSymbolLayer *layer,
QgsRenderContext renderContext = QgsRenderContext::fromQPainter( &painter );
renderContext.setForceVectorOutput( true );
QgsSymbolRenderContext symbolContext( renderContext, units, 1.0, false, nullptr, nullptr, QgsFields(), scale );
layer->drawPreviewIcon( symbolContext, size );
std::unique_ptr< QgsSymbolLayer > layerClone( layer->clone() );
layerClone->drawPreviewIcon( symbolContext, size );
painter.end();
return picture;
}

QIcon QgsSymbolLayerUtils::symbolLayerPreviewIcon( QgsSymbolLayer *layer, QgsUnitTypes::RenderUnit u, QSize size, const QgsMapUnitScale &scale )
QIcon QgsSymbolLayerUtils::symbolLayerPreviewIcon( const QgsSymbolLayer *layer, QgsUnitTypes::RenderUnit u, QSize size, const QgsMapUnitScale &scale )
{
QPixmap pixmap( size );
pixmap.fill( Qt::transparent );
Expand All @@ -742,7 +744,8 @@ QIcon QgsSymbolLayerUtils::symbolLayerPreviewIcon( QgsSymbolLayer *layer, QgsUni
renderContext.setExpressionContext( expContext );

QgsSymbolRenderContext symbolContext( renderContext, u, 1.0, false, nullptr, nullptr, QgsFields(), scale );
layer->drawPreviewIcon( symbolContext, size );
std::unique_ptr< QgsSymbolLayer > layerClone( layer->clone() );
layerClone->drawPreviewIcon( symbolContext, size );
painter.end();
return QIcon( pixmap );
}
Expand Down
8 changes: 4 additions & 4 deletions src/core/symbology/qgssymbollayerutils.h
Expand Up @@ -178,7 +178,7 @@ class CORE_EXPORT QgsSymbolLayerUtils
* \param padding space between icon edge and symbol
* \see symbolPreviewPixmap()
*/
static QIcon symbolPreviewIcon( QgsSymbol *symbol, QSize size, int padding = 0 );
static QIcon symbolPreviewIcon( const QgsSymbol *symbol, QSize size, int padding = 0 );

/**
* Returns a pixmap preview for a color ramp.
Expand All @@ -189,7 +189,7 @@ class CORE_EXPORT QgsSymbolLayerUtils
* \note Parameter customContext added in QGIS 2.6
* \see symbolPreviewIcon()
*/
static QPixmap symbolPreviewPixmap( QgsSymbol *symbol, QSize size, int padding = 0, QgsRenderContext *customContext = nullptr );
static QPixmap symbolPreviewPixmap( const QgsSymbol *symbol, QSize size, int padding = 0, QgsRenderContext *customContext = nullptr );

/**
* Draws a symbol layer preview to a QPicture
Expand All @@ -201,7 +201,7 @@ class CORE_EXPORT QgsSymbolLayerUtils
* \see symbolLayerPreviewIcon()
* \since QGIS 2.9
*/
static QPicture symbolLayerPreviewPicture( QgsSymbolLayer *layer, QgsUnitTypes::RenderUnit units, QSize size, const QgsMapUnitScale &scale = QgsMapUnitScale() );
static QPicture symbolLayerPreviewPicture( const QgsSymbolLayer *layer, QgsUnitTypes::RenderUnit units, QSize size, const QgsMapUnitScale &scale = QgsMapUnitScale() );

/**
* Draws a symbol layer preview to an icon.
Expand All @@ -212,7 +212,7 @@ class CORE_EXPORT QgsSymbolLayerUtils
* \returns icon containing symbol layer preview
* \see symbolLayerPreviewPicture()
*/
static QIcon symbolLayerPreviewIcon( QgsSymbolLayer *layer, QgsUnitTypes::RenderUnit u, QSize size, const QgsMapUnitScale &scale = QgsMapUnitScale() );
static QIcon symbolLayerPreviewIcon( const QgsSymbolLayer *layer, QgsUnitTypes::RenderUnit u, QSize size, const QgsMapUnitScale &scale = QgsMapUnitScale() );

/**
* Returns an icon preview for a color ramp.
Expand Down
3 changes: 2 additions & 1 deletion src/gui/symbology/qgssymbolselectordialog.cpp
Expand Up @@ -439,7 +439,8 @@ void QgsSymbolSelectorWidget::updateUi()

void QgsSymbolSelectorWidget::updatePreview()
{
QImage preview = mSymbol->bigSymbolPreviewImage( &mPreviewExpressionContext );
std::unique_ptr< QgsSymbol > symbolClone( mSymbol->clone() );
QImage preview = symbolClone->bigSymbolPreviewImage( &mPreviewExpressionContext );
lblPreview->setPixmap( QPixmap::fromImage( preview ) );
// Hope this is a appropriate place
if ( !mBlockModified )
Expand Down

0 comments on commit 4022c5f

Please sign in to comment.