Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge pull request #4515 from nyalldawson/render_crash
Fix crash when transform exception occurs during rendering
  • Loading branch information
nyalldawson committed May 11, 2017
2 parents 354b667 + 5d50b17 commit 9adc322
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 25 deletions.
57 changes: 36 additions & 21 deletions src/core/symbology-ng/qgssymbol.cpp
Expand Up @@ -83,7 +83,6 @@ QgsSymbol::QgsSymbol( SymbolType type, const QgsSymbolLayerList &layers )
, mRenderHints( 0 )
, mClipFeaturesToExtent( true )
, mLayer( nullptr )
, mSymbolRenderContext( nullptr )
{

// check they're all correct symbol layers
Expand Down Expand Up @@ -188,7 +187,6 @@ void QgsSymbol::_getPolygon( QPolygonF &pts, QList<QPolygonF> &holes, QgsRenderC

QgsSymbol::~QgsSymbol()
{
delete mSymbolRenderContext;
// delete all symbol layers (we own them, so it's okay)
qDeleteAll( mLayers );
}
Expand Down Expand Up @@ -377,14 +375,12 @@ bool QgsSymbol::changeSymbolLayer( int index, QgsSymbolLayer *layer )

void QgsSymbol::startRender( QgsRenderContext &context, const QgsFields &fields )
{
delete mSymbolRenderContext;
mSymbolRenderContext = new QgsSymbolRenderContext( context, outputUnit(), mAlpha, false, mRenderHints, nullptr, fields, mapUnitScale() );
mSymbolRenderContext.reset( new QgsSymbolRenderContext( context, outputUnit(), mAlpha, false, mRenderHints, nullptr, fields, mapUnitScale() ) );

QgsSymbolRenderContext symbolContext( context, outputUnit(), mAlpha, false, mRenderHints, nullptr, fields, mapUnitScale() );

QgsExpressionContextScope *scope = QgsExpressionContextUtils::updateSymbolScope( this, new QgsExpressionContextScope() );

mSymbolRenderContext->setExpressionContextScope( scope );
std::unique_ptr< QgsExpressionContextScope > scope( QgsExpressionContextUtils::updateSymbolScope( this, new QgsExpressionContextScope() ) );
mSymbolRenderContext->setExpressionContextScope( scope.release() );

Q_FOREACH ( QgsSymbolLayer *layer, mLayers )
{
Expand All @@ -410,8 +406,7 @@ void QgsSymbol::stopRender( QgsRenderContext &context )
}
}

delete mSymbolRenderContext;
mSymbolRenderContext = nullptr;
mSymbolRenderContext.reset( nullptr );

mLayer = nullptr;
}
Expand Down Expand Up @@ -643,6 +638,27 @@ bool QgsSymbol::hasDataDefinedProperties() const
return false;
}

///@cond PRIVATE

/**
* RAII class to pop scope from an expression context on destruction
*/
class ExpressionContextScopePopper
{
public:

ExpressionContextScopePopper() = default;

~ExpressionContextScopePopper()
{
if ( context )
context->popScope();
}

QgsExpressionContext *context = nullptr;
};
///@endcond PRIVATE

void QgsSymbol::renderFeature( const QgsFeature &feature, QgsRenderContext &context, int layer, bool selected, bool drawVertexMarker, int currentVertexMarkerType, int currentVertexMarkerSize )
{
QgsGeometry geom = feature.geometry();
Expand Down Expand Up @@ -672,9 +688,17 @@ void QgsSymbol::renderFeature( const QgsFeature &feature, QgsRenderContext &cont
mSymbolRenderContext->setGeometryPartCount( segmentizedGeometry.geometry()->partCount() );
mSymbolRenderContext->setGeometryPartNum( 1 );

ExpressionContextScopePopper scopePopper;
if ( mSymbolRenderContext->expressionContextScope() )
{
// this is somewhat nasty - by appending this scope here it's now owned
// by both mSymbolRenderContext AND context.expressionContext()
// the RAII scopePopper is required to make sure it always has ownership transferred back
// from context.expressionContext(), even if exceptions of other early exits occur in this
// function
context.expressionContext().appendScope( mSymbolRenderContext->expressionContextScope() );
scopePopper.context = &context.expressionContext();

QgsExpressionContextUtils::updateSymbolScope( this, mSymbolRenderContext->expressionContextScope() );
mSymbolRenderContext->expressionContextScope()->addVariable( QgsExpressionContextScope::StaticVariable( QgsExpressionContext::EXPR_GEOMETRY_PART_COUNT, mSymbolRenderContext->geometryPartCount(), true ) );
mSymbolRenderContext->expressionContextScope()->addVariable( QgsExpressionContextScope::StaticVariable( QgsExpressionContext::EXPR_GEOMETRY_PART_NUM, 1, true ) );
Expand Down Expand Up @@ -952,14 +976,11 @@ void QgsSymbol::renderFeature( const QgsFeature &feature, QgsRenderContext &cont
}
}
}

if ( mSymbolRenderContext->expressionContextScope() )
context.expressionContext().popScope();
}

QgsSymbolRenderContext *QgsSymbol::symbolRenderContext()
{
return mSymbolRenderContext;
return mSymbolRenderContext.get();
}

void QgsSymbol::renderVertexMarker( QPointF pt, QgsRenderContext &context, int currentVertexMarkerType, int currentVertexMarkerSize )
Expand All @@ -972,7 +993,6 @@ void QgsSymbol::renderVertexMarker( QPointF pt, QgsRenderContext &context, int c

QgsSymbolRenderContext::QgsSymbolRenderContext( QgsRenderContext &c, QgsUnitTypes::RenderUnit u, qreal alpha, bool selected, QgsSymbol::RenderHints renderHints, const QgsFeature *f, const QgsFields &fields, const QgsMapUnitScale &mapUnitScale )
: mRenderContext( c )
, mExpressionContextScope( nullptr )
, mOutputUnit( u )
, mMapUnitScale( mapUnitScale )
, mAlpha( alpha )
Expand All @@ -985,11 +1005,6 @@ QgsSymbolRenderContext::QgsSymbolRenderContext( QgsRenderContext &c, QgsUnitType
{
}

QgsSymbolRenderContext::~QgsSymbolRenderContext()
{
delete mExpressionContextScope;
}

void QgsSymbolRenderContext::setOriginalValueVariable( const QVariant &value )
{
mRenderContext.expressionContext().setOriginalValueVariable( value );
Expand Down Expand Up @@ -1017,12 +1032,12 @@ QgsSymbolRenderContext &QgsSymbolRenderContext::operator=( const QgsSymbolRender

QgsExpressionContextScope *QgsSymbolRenderContext::expressionContextScope()
{
return mExpressionContextScope;
return mExpressionContextScope.get();
}

void QgsSymbolRenderContext::setExpressionContextScope( QgsExpressionContextScope *contextScope )
{
mExpressionContextScope = contextScope;
mExpressionContextScope.reset( contextScope );
}

///////////////////
Expand Down
7 changes: 3 additions & 4 deletions src/core/symbology-ng/qgssymbol.h
Expand Up @@ -363,7 +363,7 @@ class CORE_EXPORT QgsSymbol

private:
//! Initialized in startRender, destroyed in stopRender
QgsSymbolRenderContext *mSymbolRenderContext = nullptr;
std::unique_ptr< QgsSymbolRenderContext > mSymbolRenderContext;

Q_DISABLE_COPY( QgsSymbol )

Expand Down Expand Up @@ -391,7 +391,6 @@ class CORE_EXPORT QgsSymbolRenderContext
* \param mapUnitScale
*/
QgsSymbolRenderContext( QgsRenderContext &c, QgsUnitTypes::RenderUnit u, qreal alpha = 1.0, bool selected = false, QgsSymbol::RenderHints renderHints = 0, const QgsFeature *f = nullptr, const QgsFields &fields = QgsFields(), const QgsMapUnitScale &mapUnitScale = QgsMapUnitScale() );
~QgsSymbolRenderContext();

QgsRenderContext &renderContext() { return mRenderContext; }
const QgsRenderContext &renderContext() const { return mRenderContext; }
Expand Down Expand Up @@ -497,11 +496,11 @@ class CORE_EXPORT QgsSymbolRenderContext
*
* \param contextScope An expression scope for details about this symbol
*/
void setExpressionContextScope( QgsExpressionContextScope *contextScope );
void setExpressionContextScope( QgsExpressionContextScope *contextScope SIP_TRANSFER );

private:
QgsRenderContext &mRenderContext;
QgsExpressionContextScope *mExpressionContextScope = nullptr;
std::unique_ptr< QgsExpressionContextScope > mExpressionContextScope;
QgsUnitTypes::RenderUnit mOutputUnit;
QgsMapUnitScale mMapUnitScale;
qreal mAlpha;
Expand Down

0 comments on commit 9adc322

Please sign in to comment.