Skip to content

Commit cf9292c

Browse files
nyalldawsonnyalldawson
committedAug 24, 2016
Color ramp dialogs no longer edit ramps in place
Now the dialogs use a copy of the ramp, and the edited ramp is retrieved by calling ramp() on the dialog after it is executed. Avoids pointer lifetime issues by storing and working on a ramp pointer which the dialog does not have ownership on. Also fix a bunch of leaks relating to cloning color ramps.
·
ltr-3_40final-3_0_0
1 parent 7413db7 commit cf9292c

25 files changed

+266
-220
lines changed
 

‎doc/api_break.dox

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,13 @@ variant instead.</li>
295295
<li>GenericDataSourceURI has been renamed to GenericDataSourceUri</li>
296296
</ul>
297297

298+
\subsection qgis_api_break_3_0_QgsColorBrewerColorRampDialog QgsColorBrewerColorRampDialog
299+
300+
<ul>
301+
<li>The dialog no longer edits a color ramp in place. Instead, a copy of the ramp is edited
302+
and the new ramp can be retrieved after executing the dialog by calling ramp().</li>
303+
</ul>
304+
298305
\subsection qgis_api_break_3_0_QgsComposerLabel QgsComposerLabel
299306

300307
<ul>
@@ -389,6 +396,13 @@ plugins calling these methods will need to be updated.</li>
389396
be returned in place of a null pointer.</li>
390397
</ul>
391398

399+
\subsection qgis_api_break_3_0_QgsCptCityColorRampDialog QgsCptCityColorRampDialog
400+
401+
<ul>
402+
<li>The dialog no longer edits a color ramp in place. Instead, a copy of the ramp is edited
403+
and the new ramp can be retrieved after executing the dialog by calling ramp().</li>
404+
</ul>
405+
392406
\subsection qgis_api_break_3_0_QgsCptCitySelectionItem QgsCptCitySelectionItem
393407

394408
<ul>
@@ -559,6 +573,13 @@ method to determine if a geometry is valid.</li>
559573
a QgsGeometry value rather than a pointer.</li>
560574
</ul>
561575

576+
\subsection qgis_api_break_3_0_QgsGradientColorRampDialog QgsGradientColorRampDialog
577+
578+
<ul>
579+
<li>The dialog no longer edits a color ramp in place. Instead, a copy of the ramp is edited
580+
and the new ramp can be retrieved after executing the dialog by calling ramp().</li>
581+
</ul>
582+
562583
\subsection qgis_api_break_3_0_QgsGraphBuilderInterface QgsGraphBuilderInterface
563584

564585
<ul>
@@ -604,6 +625,13 @@ plugins calling this method will need to be updated.</li>
604625
<li>writeCommonXML() has been renamed to writeCommonXml()</li>
605626
</ul>
606627

628+
\subsection qgis_api_break_3_0_QgsLimitedRandomRampDialog QgsLimitedRandomColorRampDialog
629+
630+
<ul>
631+
<li>The dialog no longer edits a color ramp in place. Instead, a copy of the ramp is edited
632+
and the new ramp can be retrieved after executing the dialog by calling ramp().</li>
633+
</ul>
634+
607635
\subsection qgis_api_break_3_0_QgsMapCanvas QgsMapCanvas
608636

609637
<ul>

‎python/core/symbology-ng/qgsstyle.sip

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,10 @@ class QgsStyle : QObject
1616
*/
1717
enum StyleEntity { SymbolEntity, GroupEntity, TagEntity, ColorrampEntity, SmartgroupEntity };
1818

19-
//! add color ramp to style. takes ramp's ownership
20-
/*!
19+
/** Adds a color ramp to the style. Calling this method takes the ramp's ownership.
2120
* \note Adding a color ramp with the name of existing one replaces it.
2221
* \param name is the name of the color ramp being added or updated
23-
* \param colorRamp is the color ramp
22+
* \param colorRamp is the color ramp. Ownership is transferred.
2423
* \param update set to true when the style DB has to be updated, by default it is false
2524
* \return success status of the operation
2625
*/
@@ -71,8 +70,10 @@ class QgsStyle : QObject
7170
//! remove all contents of the style
7271
void clear();
7372

74-
//! return a NEW copy of color ramp
75-
QgsColorRamp* colorRamp( const QString& name ) /Factory/;
73+
/** Returns a new copy of the specified color ramp. The caller
74+
* takes responsibility for deleting the returned object.
75+
*/
76+
QgsColorRamp* colorRamp( const QString& name ) const /Factory/;
7677

7778
//! return count of color ramps
7879
int colorRampCount();

‎python/gui/qgscolorbrewercolorrampdialog.sip

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@ class QgsColorBrewerColorRampDialog : QDialog
55
%End
66

77
public:
8-
QgsColorBrewerColorRampDialog( QgsColorBrewerColorRamp* ramp, QWidget* parent /TransferThis/ = NULL );
8+
QgsColorBrewerColorRampDialog( const QgsColorBrewerColorRamp& ramp, QWidget* parent /TransferThis/ = nullptr );
9+
10+
QgsColorBrewerColorRamp ramp() const;
911

1012
public slots:
1113
void setSchemeName();

‎python/gui/qgsgradientcolorrampdialog.sip

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@ class QgsGradientColorRampDialog : QDialog
55
%End
66

77
public:
8-
QgsGradientColorRampDialog( QgsGradientColorRamp* ramp, QWidget* parent /TransferThis/ = NULL );
8+
QgsGradientColorRampDialog( const QgsGradientColorRamp& ramp, QWidget* parent /TransferThis/ = nullptr );
9+
~QgsGradientColorRampDialog();
10+
11+
QgsGradientColorRamp ramp() const;
912

1013
public slots:
1114
void setColor1( const QColor& color );

‎python/gui/qgslimitedrandomcolorrampdialog.sip

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@ class QgsLimitedRandomColorRampDialog : QDialog
55
%End
66

77
public:
8-
QgsLimitedRandomColorRampDialog( QgsLimitedRandomColorRamp* ramp, QWidget* parent /TransferThis/ = NULL );
8+
QgsLimitedRandomColorRampDialog( const QgsLimitedRandomColorRamp& ramp, QWidget* parent /TransferThis/ = nullptr );
9+
10+
QgsLimitedRandomColorRamp ramp() const;
911

1012
public slots:
1113
void setCount( int val );

‎python/gui/symbology-ng/qgscolorrampcombobox.sip

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,18 @@ class QgsColorRampComboBox : QComboBox
1111
//! initialize the combo box with color ramps from the style
1212
void populate( QgsStyle* style );
1313

14-
//! add/select color ramp which was used previously by the renderer
15-
void setSourceColorRamp( QgsColorRamp* sourceRamp );
14+
/** Adds or selects the current color ramp to show in the combo box. The ramp appears
15+
* in the combo box as the "source" ramp.
16+
* @param sourceRamp color ramp, ownership is transferred.
17+
* @see currentColorRamp()
18+
*/
19+
void setSourceColorRamp( QgsColorRamp* sourceRamp /Transfer/ );
1620

17-
//! return new instance of the current color ramp or NULL if there is no active color ramp
18-
QgsColorRamp* currentColorRamp();
21+
/** Returns a new instance of the current color ramp or NULL if there is no active color ramp.
22+
* The caller takes responsibility for deleting the returned value.
23+
* @see setSourceColorRamp()
24+
*/
25+
QgsColorRamp* currentColorRamp() const /Factory/;
1926

2027
/** Returns true if the current selection in the combo box is the option for creating
2128
* a new color ramp

‎python/gui/symbology-ng/qgscptcitycolorrampdialog.sip

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,13 @@ class QgsCptCityColorRampDialog : QDialog
55
%End
66

77
public:
8-
QgsCptCityColorRampDialog( QgsCptCityColorRamp* ramp, QWidget* parent /TransferThis/ = NULL );
8+
9+
QgsCptCityColorRampDialog( const QgsCptCityColorRamp& ramp, QWidget* parent /TransferThis/ = nullptr );
910
~QgsCptCityColorRampDialog();
1011

12+
QgsCptCityColorRamp ramp() const;
13+
14+
1115
QString selectedName() const;
1216

1317
bool saveAsGradientRamp() const;

‎src/app/qgsprojectproperties.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1713,10 +1713,9 @@ void QgsProjectProperties::populateStyles()
17131713
for ( int i = 0; i < colorRamps.count(); ++i )
17141714
{
17151715
QString name = colorRamps[i];
1716-
QgsColorRamp* ramp = mStyle->colorRamp( name );
1717-
QIcon icon = QgsSymbolLayerUtils::colorRampPreviewIcon( ramp, cboStyleColorRamp->iconSize() );
1716+
QScopedPointer< QgsColorRamp > ramp( mStyle->colorRamp( name ) );
1717+
QIcon icon = QgsSymbolLayerUtils::colorRampPreviewIcon( ramp.data(), cboStyleColorRamp->iconSize() );
17181718
cboStyleColorRamp->addItem( icon, name );
1719-
delete ramp;
17201719
}
17211720

17221721
// set current index if found

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ bool QgsStyle::removeColorRamp( const QString& name )
254254
return true;
255255
}
256256

257-
QgsColorRamp* QgsStyle::colorRamp( const QString& name )
257+
QgsColorRamp* QgsStyle::colorRamp( const QString& name ) const
258258
{
259259
const QgsColorRamp *ramp = colorRampRef( name );
260260
return ramp ? ramp->clone() : nullptr;
@@ -1536,7 +1536,8 @@ bool QgsStyle::updateSymbol( StyleEntity type, const QString& name )
15361536
return false;
15371537
}
15381538

1539-
symEl = QgsSymbolLayerUtils::saveColorRamp( name, colorRamp( name ), doc );
1539+
QScopedPointer< QgsColorRamp > ramp( colorRamp( name ) );
1540+
symEl = QgsSymbolLayerUtils::saveColorRamp( name, ramp.data(), doc );
15401541
if ( symEl.isNull() )
15411542
{
15421543
QgsDebugMsg( "Couldn't convert color ramp to valid XML!" );

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

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -82,11 +82,10 @@ class CORE_EXPORT QgsStyle : public QObject
8282
*/
8383
enum StyleEntity { SymbolEntity, GroupEntity, TagEntity, ColorrampEntity, SmartgroupEntity };
8484

85-
//! add color ramp to style. takes ramp's ownership
86-
/*!
85+
/** Adds a color ramp to the style. Calling this method takes the ramp's ownership.
8786
* \note Adding a color ramp with the name of existing one replaces it.
8887
* \param name is the name of the color ramp being added or updated
89-
* \param colorRamp is the Vector color ramp
88+
* \param colorRamp is the color ramp. Ownership is transferred.
9089
* \param update set to true when the style DB has to be updated, by default it is false
9190
* \return success status of the operation
9291
*/
@@ -137,8 +136,10 @@ class CORE_EXPORT QgsStyle : public QObject
137136
//! remove all contents of the style
138137
void clear();
139138

140-
//! return a NEW copy of color ramp
141-
QgsColorRamp* colorRamp( const QString& name );
139+
/** Returns a new copy of the specified color ramp. The caller
140+
* takes responsibility for deleting the returned object.
141+
*/
142+
QgsColorRamp* colorRamp( const QString& name ) const;
142143

143144
//! return count of color ramps
144145
int colorRampCount();

‎src/gui/qgscolorbrewercolorrampdialog.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ static void updateColorButton( QAbstractButton* button, QColor color )
3131
/////////
3232

3333

34-
QgsColorBrewerColorRampDialog::QgsColorBrewerColorRampDialog( QgsColorBrewerColorRamp* ramp, QWidget* parent )
34+
QgsColorBrewerColorRampDialog::QgsColorBrewerColorRampDialog( const QgsColorBrewerColorRamp& ramp, QWidget* parent )
3535
: QDialog( parent )
3636
, mRamp( ramp )
3737
{
@@ -51,9 +51,9 @@ QgsColorBrewerColorRampDialog::QgsColorBrewerColorRampDialog( QgsColorBrewerColo
5151
cboSchemeName->addItem( icon, schemeName );
5252
}
5353

54-
cboSchemeName->setCurrentIndex( cboSchemeName->findText( ramp->schemeName() ) );
54+
cboSchemeName->setCurrentIndex( cboSchemeName->findText( mRamp.schemeName() ) );
5555
populateVariants();
56-
cboColors->setCurrentIndex( cboColors->findText( QString::number( ramp->colors() ) ) );
56+
cboColors->setCurrentIndex( cboColors->findText( QString::number( mRamp.colors() ) ) );
5757

5858
connect( cboSchemeName, SIGNAL( currentIndexChanged( int ) ), this, SLOT( setSchemeName() ) );
5959
connect( cboColors, SIGNAL( currentIndexChanged( int ) ), this, SLOT( setColors() ) );
@@ -86,21 +86,21 @@ void QgsColorBrewerColorRampDialog::populateVariants()
8686
void QgsColorBrewerColorRampDialog::updatePreview()
8787
{
8888
QSize size( 300, 40 );
89-
lblPreview->setPixmap( QgsSymbolLayerUtils::colorRampPreviewPixmap( mRamp, size ) );
89+
lblPreview->setPixmap( QgsSymbolLayerUtils::colorRampPreviewPixmap( &mRamp, size ) );
9090
}
9191

9292
void QgsColorBrewerColorRampDialog::setSchemeName()
9393
{
9494
// populate list of variants
9595
populateVariants();
9696

97-
mRamp->setSchemeName( cboSchemeName->currentText() );
97+
mRamp.setSchemeName( cboSchemeName->currentText() );
9898
updatePreview();
9999
}
100100

101101
void QgsColorBrewerColorRampDialog::setColors()
102102
{
103103
int num = cboColors->currentText().toInt();
104-
mRamp->setColors( num );
104+
mRamp.setColors( num );
105105
updatePreview();
106106
}

‎src/gui/qgscolorbrewercolorrampdialog.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
#include <QDialog>
2020

21+
#include "qgscolorramp.h"
2122
#include "ui_qgscolorbrewercolorrampdialogbase.h"
2223

2324
class QgsColorBrewerColorRamp;
@@ -30,7 +31,9 @@ class GUI_EXPORT QgsColorBrewerColorRampDialog : public QDialog, private Ui::Qgs
3031
Q_OBJECT
3132

3233
public:
33-
QgsColorBrewerColorRampDialog( QgsColorBrewerColorRamp* ramp, QWidget* parent = nullptr );
34+
QgsColorBrewerColorRampDialog( const QgsColorBrewerColorRamp& ramp, QWidget* parent = nullptr );
35+
36+
QgsColorBrewerColorRamp ramp() const { return mRamp; }
3437

3538
public slots:
3639
void setSchemeName();
@@ -42,7 +45,7 @@ class GUI_EXPORT QgsColorBrewerColorRampDialog : public QDialog, private Ui::Qgs
4245

4346
void updatePreview();
4447

45-
QgsColorBrewerColorRamp* mRamp;
48+
QgsColorBrewerColorRamp mRamp;
4649
};
4750

4851
#endif

0 commit comments

Comments
 (0)
Please sign in to comment.