Fix a memory leak in classify() + add missing annotations/docstrings

wonder-sk · nyalldawson · commit 4c84cfa29adb · 2018-06-22T05:48:46.000+10:00
A pseudo color renderer was getting created in classify() but never deleted. Spotted by @PeterPetrik As a bonus switched a for loop from mixed constBegin()/end() usage to range for.
diff --git a/python/core/auto_generated/raster/qgscolorrampshader.sip.in b/python/core/auto_generated/raster/qgscolorrampshader.sip.in
@@ -35,15 +35,15 @@ A ramp shader will color a raster pixel based on a list of values ranges in a ra
       Quantile
     };
 
-    QgsColorRampShader( double minimumValue = 0.0, double maximumValue = 255.0, QgsColorRamp *colorRamp = 0, Type type = Interpolated, ClassificationMode classificationMode = Continuous );
+    QgsColorRampShader( double minimumValue = 0.0, double maximumValue = 255.0, QgsColorRamp *colorRamp /Transfer/ = 0, Type type = Interpolated, ClassificationMode classificationMode = Continuous );
 %Docstring
 Creates a new color ramp shader.
 
 :param minimumValue: minimum value for the raster shader
 :param maximumValue: maximum value for the raster shader
 :param type: interpolation type used
 :param classificationMode: method used to classify the color ramp shader
-:param colorRamp: vector color ramp used to classify the color ramp shader
+:param colorRamp: vector color ramp used to classify the color ramp shader. Ownership is transferred to the shader.
 
 :return: new QgsColorRampShader
 %End
@@ -105,7 +105,7 @@ Gets the source color ramp
 
     void setSourceColorRamp( QgsColorRamp *colorramp /Transfer/ );
 %Docstring
-Set the source color ramp. Ownership is transferred to the renderer.
+Set the source color ramp. Ownership is transferred to the shader.
 
 .. seealso:: :py:func:`sourceColorRamp`
 
diff --git a/python/core/auto_generated/raster/qgssinglebandpseudocolorrenderer.sip.in b/python/core/auto_generated/raster/qgssinglebandpseudocolorrenderer.sip.in
@@ -56,7 +56,7 @@ Returns the raster shader
    available in Python as constShader
 %End
 
-    void createShader( QgsColorRamp *colorRamp = 0,
+    void createShader( QgsColorRamp *colorRamp /Transfer/ = 0,
                        QgsColorRampShader::Type colorRampType  = QgsColorRampShader::Interpolated,
                        QgsColorRampShader::ClassificationMode classificationMode = QgsColorRampShader::Continuous,
                        int classes = 0,
@@ -65,7 +65,7 @@ Returns the raster shader
 %Docstring
 Creates a color ramp shader
 
-:param colorRamp: vector color ramp
+:param colorRamp: vector color ramp. Ownership is transferred to the shader.
 :param colorRampType: type of color ramp shader
 :param classificationMode: classification mode
 :param classes: number of classes
diff --git a/src/core/raster/qgscolorrampshader.h b/src/core/raster/qgscolorrampshader.h
@@ -63,10 +63,10 @@ class CORE_EXPORT QgsColorRampShader : public QgsRasterShaderFunction
      * \param maximumValue maximum value for the raster shader
      * \param type interpolation type used
      * \param classificationMode method used to classify the color ramp shader
-     * \param colorRamp vector color ramp used to classify the color ramp shader
+     * \param colorRamp vector color ramp used to classify the color ramp shader. Ownership is transferred to the shader.
      * \returns new QgsColorRampShader
      */
-    QgsColorRampShader( double minimumValue = 0.0, double maximumValue = 255.0, QgsColorRamp *colorRamp = nullptr, Type type = Interpolated, ClassificationMode classificationMode = Continuous );
+    QgsColorRampShader( double minimumValue = 0.0, double maximumValue = 255.0, QgsColorRamp *colorRamp SIP_TRANSFER = nullptr, Type type = Interpolated, ClassificationMode classificationMode = Continuous );
 
     /**
      * Copy constructor
@@ -124,7 +124,7 @@ class CORE_EXPORT QgsColorRampShader : public QgsRasterShaderFunction
     QgsColorRamp *sourceColorRamp() const SIP_FACTORY;
 
     /**
-     * Set the source color ramp. Ownership is transferred to the renderer.
+     * Set the source color ramp. Ownership is transferred to the shader.
      * \see sourceColorRamp()
      * \since QGIS 3.0
      */
diff --git a/src/core/raster/qgssinglebandpseudocolorrenderer.h b/src/core/raster/qgssinglebandpseudocolorrenderer.h
@@ -62,14 +62,14 @@ class CORE_EXPORT QgsSingleBandPseudoColorRenderer: public QgsRasterRenderer
 
     /**
      * Creates a color ramp shader
-     * \param colorRamp vector color ramp
+     * \param colorRamp vector color ramp. Ownership is transferred to the shader.
      * \param colorRampType type of color ramp shader
      * \param classificationMode classification mode
      * \param classes number of classes
      * \param clip clip out of range values
      * \param extent extent used in classification (only used in quantile mode)
      */
-    void createShader( QgsColorRamp *colorRamp = nullptr,
+    void createShader( QgsColorRamp *colorRamp SIP_TRANSFER = nullptr,
                        QgsColorRampShader::Type colorRampType  = QgsColorRampShader::Interpolated,
                        QgsColorRampShader::ClassificationMode classificationMode = QgsColorRampShader::Continuous,
                        int classes = 0,
diff --git a/src/gui/raster/qgssinglebandpseudocolorrendererwidget.cpp b/src/gui/raster/qgssinglebandpseudocolorrendererwidget.cpp
@@ -348,10 +348,10 @@ void QgsSingleBandPseudoColorRendererWidget::classify()
     return;
   }
 
-  QgsSingleBandPseudoColorRenderer *pr = new QgsSingleBandPseudoColorRenderer( mRasterLayer->dataProvider(), mBandComboBox->currentBand(), nullptr );
+  std::unique_ptr<QgsSingleBandPseudoColorRenderer> pr( new QgsSingleBandPseudoColorRenderer( mRasterLayer->dataProvider(), mBandComboBox->currentBand(), nullptr ) );
   pr->setClassificationMin( lineEditValue( mMinLineEdit ) );
   pr->setClassificationMax( lineEditValue( mMaxLineEdit ) );
-  pr->createShader( ramp.get(), static_cast< QgsColorRampShader::Type >( mColorInterpolationComboBox->currentData().toInt() ), static_cast< QgsColorRampShader::ClassificationMode >( mClassificationModeComboBox->currentData().toInt() ), mNumberOfEntriesSpinBox->value(), mClipCheckBox->isChecked(), minMaxWidget()->extent() );
+  pr->createShader( ramp.release(), static_cast< QgsColorRampShader::Type >( mColorInterpolationComboBox->currentData().toInt() ), static_cast< QgsColorRampShader::ClassificationMode >( mClassificationModeComboBox->currentData().toInt() ), mNumberOfEntriesSpinBox->value(), mClipCheckBox->isChecked(), minMaxWidget()->extent() );
 
   const QgsRasterShader *rasterShader = pr->shader();
   if ( rasterShader )
@@ -362,13 +362,12 @@ void QgsSingleBandPseudoColorRendererWidget::classify()
       mColormapTreeWidget->clear();
 
       const QList<QgsColorRampShader::ColorRampItem> colorRampItemList = colorRampShader->colorRampItemList();
-      QList<QgsColorRampShader::ColorRampItem>::const_iterator it = colorRampItemList.constBegin();
-      for ( ; it != colorRampItemList.end(); ++it )
+      for ( const QgsColorRampShader::ColorRampItem &item : colorRampItemList )
       {
         QgsTreeWidgetItemObject *newItem = new QgsTreeWidgetItemObject( mColormapTreeWidget );
-        newItem->setText( ValueColumn, QString::number( it->value, 'g', 15 ) );
-        newItem->setBackground( ColorColumn, QBrush( it->color ) );
-        newItem->setText( LabelColumn, it->label );
+        newItem->setText( ValueColumn, QString::number( item.value, 'g', 15 ) );
+        newItem->setBackground( ColorColumn, QBrush( item.color ) );
+        newItem->setText( LabelColumn, item.label );
         newItem->setFlags( Qt::ItemIsEnabled | Qt::ItemIsEditable | Qt::ItemIsSelectable );
         connect( newItem, &QgsTreeWidgetItemObject::itemEdited,
                  this, &QgsSingleBandPseudoColorRendererWidget::mColormapTreeWidget_itemEdited );
