Skip to content

Commit

Permalink
Fix crash in displacement/distance renderers
Browse files Browse the repository at this point in the history
Individual symbol instances were being rendered multiple times
concurrently
  • Loading branch information
nyalldawson committed Jul 23, 2017
1 parent b95d432 commit 99bf32b
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 40 deletions.
14 changes: 8 additions & 6 deletions python/core/symbology-ng/qgspointdistancerenderer.sip
Original file line number Diff line number Diff line change
Expand Up @@ -29,34 +29,36 @@ class QgsPointDistanceRenderer: QgsFeatureRenderer
struct GroupedFeature
{

GroupedFeature( const QgsFeature &feature, QgsMarkerSymbol *symbol, bool isSelected, const QString &label = QString() );
GroupedFeature( const QgsFeature &feature, QgsMarkerSymbol *symbol /Transfer/, bool isSelected, const QString &label = QString() );
%Docstring
Constructor for GroupedFeature.
\param feature feature
\param symbol base symbol for rendering feature
\param symbol base symbol for rendering feature (owned by GroupedFeature)
\param isSelected set to true if feature is selected and should be rendered in a selected state
\param label optional label text, or empty string for no label
%End

QgsFeature feature;
QgsFeature feature;
%Docstring
Feature
%End

QgsMarkerSymbol *symbol;
QgsMarkerSymbol *symbol() const;
%Docstring
Base symbol for rendering feature
:rtype: QgsMarkerSymbol
%End

bool isSelected;
bool isSelected;
%Docstring
True if feature is selected and should be rendered in a selected state
%End

QString label;
QString label;
%Docstring
Optional label text
%End

};

typedef QList< QgsPointDistanceRenderer::GroupedFeature > ClusteredGroup;
Expand Down
4 changes: 3 additions & 1 deletion src/core/symbology-ng/qgspointclusterrenderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,10 @@ void QgsPointClusterRenderer::drawGroup( QPointF centerPoint, QgsRenderContext &
else
{
//single isolated symbol, draw it untouched
QgsMarkerSymbol *symbol = group.at( 0 ).symbol;
QgsMarkerSymbol *symbol = group.at( 0 ).symbol();
symbol->startRender( context );
symbol->renderPoint( centerPoint, &( group.at( 0 ).feature ), context, -1, group.at( 0 ).isSelected );
symbol->stopRender( context );
}
}

Expand Down
8 changes: 4 additions & 4 deletions src/core/symbology-ng/qgspointdisplacementrenderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ void QgsPointDisplacementRenderer::drawGroup( QPointF centerPoint, QgsRenderCont

Q_FOREACH ( const GroupedFeature &feature, group )
{
if ( QgsMarkerSymbol *symbol = feature.symbol )
if ( QgsMarkerSymbol *symbol = feature.symbol() )
{
diagonal = qMax( diagonal, context.convertToPainterUnits( M_SQRT2 * symbol->size(),
symbol->sizeUnit(), symbol->sizeMapUnitScale() ) );
Expand Down Expand Up @@ -327,9 +327,9 @@ void QgsPointDisplacementRenderer::drawSymbols( const ClusteredGroup &group, Qgs
++symbolPosIt, ++groupIt )
{
context.expressionContext().setFeature( groupIt->feature );
groupIt->symbol->startRender( context );
groupIt->symbol->renderPoint( *symbolPosIt, &( groupIt->feature ), context, -1, groupIt->isSelected );
groupIt->symbol->stopRender( context );
groupIt->symbol()->startRender( context );
groupIt->symbol()->renderPoint( *symbolPosIt, &( groupIt->feature ), context, -1, groupIt->isSelected );
groupIt->symbol()->stopRender( context );
}
}

Expand Down
10 changes: 5 additions & 5 deletions src/core/symbology-ng/qgspointdistancerenderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ bool QgsPointDistanceRenderer::renderFeature( QgsFeature &feature, QgsRenderCont
mSpatialIndex->insertFeature( transformedFeature );
// create new group
ClusteredGroup newGroup;
newGroup << GroupedFeature( transformedFeature, symbol, selected, label );
newGroup << GroupedFeature( transformedFeature, symbol->clone(), selected, label );
mClusteredGroups.push_back( newGroup );
// add to group index
mGroupIndex.insert( transformedFeature.id(), mClusteredGroups.count() - 1 );
Expand Down Expand Up @@ -127,7 +127,7 @@ bool QgsPointDistanceRenderer::renderFeature( QgsFeature &feature, QgsRenderCont
( oldCenter.y() * group.size() + point.y() ) / ( group.size() + 1.0 ) );

// add to a group
group << GroupedFeature( transformedFeature, symbol, selected, label );
group << GroupedFeature( transformedFeature, symbol->clone(), selected, label );
// add to group index
mGroupIndex.insert( transformedFeature.id(), groupIdx );
}
Expand Down Expand Up @@ -425,16 +425,16 @@ QgsExpressionContextScope *QgsPointDistanceRenderer::createGroupScope( const Clu
ClusteredGroup::const_iterator groupIt = group.constBegin();
for ( ; groupIt != group.constEnd(); ++groupIt )
{
if ( !groupIt->symbol )
if ( !groupIt->symbol() )
continue;

if ( !groupColor.isValid() )
{
groupColor = groupIt->symbol->color();
groupColor = groupIt->symbol()->color();
}
else
{
if ( groupColor != groupIt->symbol->color() )
if ( groupColor != groupIt->symbol()->color() )
{
groupColor = QColor();
break;
Expand Down
51 changes: 27 additions & 24 deletions src/core/symbology-ng/qgspointdistancerenderer.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,30 +42,33 @@ class CORE_EXPORT QgsPointDistanceRenderer: public QgsFeatureRenderer
struct GroupedFeature
{

/** Constructor for GroupedFeature.
* \param feature feature
* \param symbol base symbol for rendering feature
* \param isSelected set to true if feature is selected and should be rendered in a selected state
* \param label optional label text, or empty string for no label
*/
GroupedFeature( const QgsFeature &feature, QgsMarkerSymbol *symbol, bool isSelected, const QString &label = QString() )
: feature( feature )
, symbol( symbol )
, isSelected( isSelected )
, label( label )
{}

//! Feature
QgsFeature feature;

//! Base symbol for rendering feature
QgsMarkerSymbol *symbol = nullptr;

//! True if feature is selected and should be rendered in a selected state
bool isSelected;

//! Optional label text
QString label;
/** Constructor for GroupedFeature.
* \param feature feature
* \param symbol base symbol for rendering feature (owned by GroupedFeature)
* \param isSelected set to true if feature is selected and should be rendered in a selected state
* \param label optional label text, or empty string for no label
*/
GroupedFeature( const QgsFeature &feature, QgsMarkerSymbol *symbol SIP_TRANSFER, bool isSelected, const QString &label = QString() )
: feature( feature )
, isSelected( isSelected )
, label( label )
, mSymbol( symbol )
{}

//! Feature
QgsFeature feature;

//! Base symbol for rendering feature
QgsMarkerSymbol *symbol() const { return mSymbol.get(); }

//! True if feature is selected and should be rendered in a selected state
bool isSelected;

//! Optional label text
QString label;

private:
std::shared_ptr< QgsMarkerSymbol > mSymbol;
};

//! A group of clustered points (ie features within the distance tolerance).
Expand Down

0 comments on commit 99bf32b

Please sign in to comment.