Skip to content

Commit

Permalink
Fix a memory leak in classify() + add missing annotations/docstrings
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@wonder-sk @nyalldawson
wonder-sk authored and nyalldawson committed Jun 21, 2018
1 parent 506f028 commit 4c84cfa
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 17 deletions.
6 changes: 3 additions & 3 deletions python/core/auto_generated/raster/qgscolorrampshader.sip.in
View file
Expand Up @@ -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
Expand Down Expand Up @@ -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`

Expand Down
4 changes: 2 additions & 2 deletions python/core/auto_generated/raster/qgssinglebandpseudocolorrenderer.sip.in
View file
Expand Up @@ -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,
Expand All @@ -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
Expand Down
6 changes: 3 additions & 3 deletions src/core/raster/qgscolorrampshader.h
View file
Expand Up @@ -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
Expand Down Expand Up @@ -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
*/
Expand Down
4 changes: 2 additions & 2 deletions src/core/raster/qgssinglebandpseudocolorrenderer.h
View file
Expand Up @@ -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,
Expand Down
13 changes: 6 additions & 7 deletions src/gui/raster/qgssinglebandpseudocolorrendererwidget.cpp
View file
Expand Up @@ -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 )
Expand All @@ -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 );
Expand Down

0 comments on commit 4c84cfa

Please sign in to comment.