Merge pull request #4908 from nyalldawson/fix_displace_crash

Fix crash in displacement/distance renderers
qgis · Jul 23, 2017 · d570b43 · d570b43
2 parents b95d432 + 99bf32b
commit d570b43
Show file tree

Hide file tree

Showing 5 changed files with 47 additions and 40 deletions.
diff --git a/python/core/symbology-ng/qgspointdistancerenderer.sip b/python/core/symbology-ng/qgspointdistancerenderer.sip
@@ -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;

diff --git a/src/core/symbology-ng/qgspointclusterrenderer.cpp b/src/core/symbology-ng/qgspointclusterrenderer.cpp
@@ -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 );
   }
 }
 

diff --git a/src/core/symbology-ng/qgspointdisplacementrenderer.cpp b/src/core/symbology-ng/qgspointdisplacementrenderer.cpp
@@ -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() ) );
@@ -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 );
   }
 }
 

diff --git a/src/core/symbology-ng/qgspointdistancerenderer.cpp b/src/core/symbology-ng/qgspointdistancerenderer.cpp
@@ -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 );
@@ -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 );
   }
@@ -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;

diff --git a/src/core/symbology-ng/qgspointdistancerenderer.h b/src/core/symbology-ng/qgspointdistancerenderer.h
@@ -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).