Skip to content

Commit 9adc322

Browse files
authoredMay 11, 2017
Merge pull request #4515 from nyalldawson/render_crash
Fix crash when transform exception occurs during rendering
2 parents 354b667 + 5d50b17 commit 9adc322

File tree

2 files changed

+39
-25
lines changed

2 files changed

+39
-25
lines changed
 

‎src/core/symbology-ng/qgssymbol.cpp

Lines changed: 36 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,6 @@ QgsSymbol::QgsSymbol( SymbolType type, const QgsSymbolLayerList &layers )
8383
, mRenderHints( 0 )
8484
, mClipFeaturesToExtent( true )
8585
, mLayer( nullptr )
86-
, mSymbolRenderContext( nullptr )
8786
{
8887

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

189188
QgsSymbol::~QgsSymbol()
190189
{
191-
delete mSymbolRenderContext;
192190
// delete all symbol layers (we own them, so it's okay)
193191
qDeleteAll( mLayers );
194192
}
@@ -377,14 +375,12 @@ bool QgsSymbol::changeSymbolLayer( int index, QgsSymbolLayer *layer )
377375

378376
void QgsSymbol::startRender( QgsRenderContext &context, const QgsFields &fields )
379377
{
380-
delete mSymbolRenderContext;
381-
mSymbolRenderContext = new QgsSymbolRenderContext( context, outputUnit(), mAlpha, false, mRenderHints, nullptr, fields, mapUnitScale() );
378+
mSymbolRenderContext.reset( new QgsSymbolRenderContext( context, outputUnit(), mAlpha, false, mRenderHints, nullptr, fields, mapUnitScale() ) );
382379

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

385-
QgsExpressionContextScope *scope = QgsExpressionContextUtils::updateSymbolScope( this, new QgsExpressionContextScope() );
386-
387-
mSymbolRenderContext->setExpressionContextScope( scope );
382+
std::unique_ptr< QgsExpressionContextScope > scope( QgsExpressionContextUtils::updateSymbolScope( this, new QgsExpressionContextScope() ) );
383+
mSymbolRenderContext->setExpressionContextScope( scope.release() );
388384

389385
Q_FOREACH ( QgsSymbolLayer *layer, mLayers )
390386
{
@@ -410,8 +406,7 @@ void QgsSymbol::stopRender( QgsRenderContext &context )
410406
}
411407
}
412408

413-
delete mSymbolRenderContext;
414-
mSymbolRenderContext = nullptr;
409+
mSymbolRenderContext.reset( nullptr );
415410

416411
mLayer = nullptr;
417412
}
@@ -643,6 +638,27 @@ bool QgsSymbol::hasDataDefinedProperties() const
643638
return false;
644639
}
645640

641+
///@cond PRIVATE
642+
643+
/**
644+
* RAII class to pop scope from an expression context on destruction
645+
*/
646+
class ExpressionContextScopePopper
647+
{
648+
public:
649+
650+
ExpressionContextScopePopper() = default;
651+
652+
~ExpressionContextScopePopper()
653+
{
654+
if ( context )
655+
context->popScope();
656+
}
657+
658+
QgsExpressionContext *context = nullptr;
659+
};
660+
///@endcond PRIVATE
661+
646662
void QgsSymbol::renderFeature( const QgsFeature &feature, QgsRenderContext &context, int layer, bool selected, bool drawVertexMarker, int currentVertexMarkerType, int currentVertexMarkerSize )
647663
{
648664
QgsGeometry geom = feature.geometry();
@@ -672,9 +688,17 @@ void QgsSymbol::renderFeature( const QgsFeature &feature, QgsRenderContext &cont
672688
mSymbolRenderContext->setGeometryPartCount( segmentizedGeometry.geometry()->partCount() );
673689
mSymbolRenderContext->setGeometryPartNum( 1 );
674690

691+
ExpressionContextScopePopper scopePopper;
675692
if ( mSymbolRenderContext->expressionContextScope() )
676693
{
694+
// this is somewhat nasty - by appending this scope here it's now owned
695+
// by both mSymbolRenderContext AND context.expressionContext()
696+
// the RAII scopePopper is required to make sure it always has ownership transferred back
697+
// from context.expressionContext(), even if exceptions of other early exits occur in this
698+
// function
677699
context.expressionContext().appendScope( mSymbolRenderContext->expressionContextScope() );
700+
scopePopper.context = &context.expressionContext();
701+
678702
QgsExpressionContextUtils::updateSymbolScope( this, mSymbolRenderContext->expressionContextScope() );
679703
mSymbolRenderContext->expressionContextScope()->addVariable( QgsExpressionContextScope::StaticVariable( QgsExpressionContext::EXPR_GEOMETRY_PART_COUNT, mSymbolRenderContext->geometryPartCount(), true ) );
680704
mSymbolRenderContext->expressionContextScope()->addVariable( QgsExpressionContextScope::StaticVariable( QgsExpressionContext::EXPR_GEOMETRY_PART_NUM, 1, true ) );
@@ -952,14 +976,11 @@ void QgsSymbol::renderFeature( const QgsFeature &feature, QgsRenderContext &cont
952976
}
953977
}
954978
}
955-
956-
if ( mSymbolRenderContext->expressionContextScope() )
957-
context.expressionContext().popScope();
958979
}
959980

960981
QgsSymbolRenderContext *QgsSymbol::symbolRenderContext()
961982
{
962-
return mSymbolRenderContext;
983+
return mSymbolRenderContext.get();
963984
}
964985

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

973994
QgsSymbolRenderContext::QgsSymbolRenderContext( QgsRenderContext &c, QgsUnitTypes::RenderUnit u, qreal alpha, bool selected, QgsSymbol::RenderHints renderHints, const QgsFeature *f, const QgsFields &fields, const QgsMapUnitScale &mapUnitScale )
974995
: mRenderContext( c )
975-
, mExpressionContextScope( nullptr )
976996
, mOutputUnit( u )
977997
, mMapUnitScale( mapUnitScale )
978998
, mAlpha( alpha )
@@ -985,11 +1005,6 @@ QgsSymbolRenderContext::QgsSymbolRenderContext( QgsRenderContext &c, QgsUnitType
9851005
{
9861006
}
9871007

988-
QgsSymbolRenderContext::~QgsSymbolRenderContext()
989-
{
990-
delete mExpressionContextScope;
991-
}
992-
9931008
void QgsSymbolRenderContext::setOriginalValueVariable( const QVariant &value )
9941009
{
9951010
mRenderContext.expressionContext().setOriginalValueVariable( value );
@@ -1017,12 +1032,12 @@ QgsSymbolRenderContext &QgsSymbolRenderContext::operator=( const QgsSymbolRender
10171032

10181033
QgsExpressionContextScope *QgsSymbolRenderContext::expressionContextScope()
10191034
{
1020-
return mExpressionContextScope;
1035+
return mExpressionContextScope.get();
10211036
}
10221037

10231038
void QgsSymbolRenderContext::setExpressionContextScope( QgsExpressionContextScope *contextScope )
10241039
{
1025-
mExpressionContextScope = contextScope;
1040+
mExpressionContextScope.reset( contextScope );
10261041
}
10271042

10281043
///////////////////

‎src/core/symbology-ng/qgssymbol.h

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,7 @@ class CORE_EXPORT QgsSymbol
363363

364364
private:
365365
//! Initialized in startRender, destroyed in stopRender
366-
QgsSymbolRenderContext *mSymbolRenderContext = nullptr;
366+
std::unique_ptr< QgsSymbolRenderContext > mSymbolRenderContext;
367367

368368
Q_DISABLE_COPY( QgsSymbol )
369369

@@ -391,7 +391,6 @@ class CORE_EXPORT QgsSymbolRenderContext
391391
* \param mapUnitScale
392392
*/
393393
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() );
394-
~QgsSymbolRenderContext();
395394

396395
QgsRenderContext &renderContext() { return mRenderContext; }
397396
const QgsRenderContext &renderContext() const { return mRenderContext; }
@@ -497,11 +496,11 @@ class CORE_EXPORT QgsSymbolRenderContext
497496
*
498497
* \param contextScope An expression scope for details about this symbol
499498
*/
500-
void setExpressionContextScope( QgsExpressionContextScope *contextScope );
499+
void setExpressionContextScope( QgsExpressionContextScope *contextScope SIP_TRANSFER );
501500

502501
private:
503502
QgsRenderContext &mRenderContext;
504-
QgsExpressionContextScope *mExpressionContextScope = nullptr;
503+
std::unique_ptr< QgsExpressionContextScope > mExpressionContextScope;
505504
QgsUnitTypes::RenderUnit mOutputUnit;
506505
QgsMapUnitScale mMapUnitScale;
507506
qreal mAlpha;

0 commit comments

Comments
 (0)
Please sign in to comment.