Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge pull request #7336 from mhugo/rulebasedlabeling_fix
Fix for rule based labeling toSld()
  • Loading branch information
Hugo Mercier committed Jul 2, 2018
2 parents 2ef526a + b37a215 commit b6901e2
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 20 deletions.
2 changes: 1 addition & 1 deletion python/core/auto_generated/qgsrulebasedlabeling.sip.in
Expand Up @@ -37,7 +37,7 @@ class QgsRuleBasedLabeling : QgsAbstractVectorLayerLabeling
public:
Rule( QgsPalLayerSettings *settings /Transfer/, int maximumScale = 0, int minimumScale = 0, const QString &filterExp = QString(), const QString &description = QString(), bool elseRule = false );
%Docstring
takes ownership of settings
takes ownership of settings, settings may be None
%End
~Rule();

Expand Down
21 changes: 8 additions & 13 deletions src/core/qgsrulebasedlabeling.cpp
Expand Up @@ -65,25 +65,21 @@ QgsRuleBasedLabeling::Rule::Rule( QgsPalLayerSettings *settings, int scaleMinDen
, mIsActive( true )

{
mRuleKey = QUuid::createUuid().toString();
initFilter();
}

QgsRuleBasedLabeling::Rule::~Rule()
{
delete mSettings;
delete mFilter;
qDeleteAll( mChildren );
// do NOT delete parent
}

void QgsRuleBasedLabeling::Rule::setSettings( QgsPalLayerSettings *settings )
{
if ( mSettings == settings )
if ( mSettings.get() == settings )
return;

delete mSettings;
mSettings = settings;
mSettings.reset( settings );
}

QgsRuleBasedLabeling::RuleList QgsRuleBasedLabeling::Rule::descendants() const
Expand All @@ -102,16 +98,15 @@ void QgsRuleBasedLabeling::Rule::initFilter()
if ( mElseRule || mFilterExp.compare( QLatin1String( "ELSE" ), Qt::CaseInsensitive ) == 0 )
{
mElseRule = true;
mFilter = nullptr;
mFilter.reset( nullptr );
}
else if ( !mFilterExp.isEmpty() )
{
delete mFilter;
mFilter = new QgsExpression( mFilterExp );
mFilter.reset( new QgsExpression( mFilterExp ) );
}
else
{
mFilter = nullptr;
mFilter.reset( nullptr );
}
}

Expand Down Expand Up @@ -204,7 +199,7 @@ QgsRuleBasedLabeling::Rule *QgsRuleBasedLabeling::Rule::findRuleByKey( const QSt

QgsRuleBasedLabeling::Rule *QgsRuleBasedLabeling::Rule::clone() const
{
QgsPalLayerSettings *s = mSettings ? new QgsPalLayerSettings( *mSettings ) : nullptr;
QgsPalLayerSettings *s = mSettings.get() ? new QgsPalLayerSettings( *mSettings ) : nullptr;
Rule *newrule = new Rule( s, mMaximumScale, mMinimumScale, mFilterExp, mDescription );
newrule->setActive( mIsActive );
// clone children
Expand Down Expand Up @@ -286,7 +281,7 @@ void QgsRuleBasedLabeling::Rule::createSubProviders( QgsVectorLayer *layer, QgsR
if ( mSettings )
{
// add provider!
QgsVectorLayerLabelProvider *p = provider->createProvider( layer, mRuleKey, false, mSettings );
QgsVectorLayerLabelProvider *p = provider->createProvider( layer, mRuleKey, false, mSettings.get() );
delete subProviders.value( this, nullptr );
subProviders[this] = p;
}
Expand Down Expand Up @@ -499,7 +494,7 @@ void QgsRuleBasedLabeling::toSld( QDomNode &parent, const QgsStringMap &props )
{
QgsPalLayerSettings *settings = rule->settings();

if ( settings->drawLabels )
if ( settings && settings->drawLabels )
{
QDomDocument doc = parent.ownerDocument();

Expand Down
9 changes: 4 additions & 5 deletions src/core/qgsrulebasedlabeling.h
Expand Up @@ -52,7 +52,7 @@ class CORE_EXPORT QgsRuleBasedLabeling : public QgsAbstractVectorLayerLabeling
class CORE_EXPORT Rule
{
public:
//! takes ownership of settings
//! takes ownership of settings, settings may be nullptr
Rule( QgsPalLayerSettings *settings SIP_TRANSFER, int maximumScale = 0, int minimumScale = 0, const QString &filterExp = QString(), const QString &description = QString(), bool elseRule = false );
~Rule();

Expand All @@ -72,7 +72,7 @@ class CORE_EXPORT QgsRuleBasedLabeling : public QgsAbstractVectorLayerLabeling
/**
* Gets the labeling settings. May return a null pointer.
*/
QgsPalLayerSettings *settings() const { return mSettings; }
QgsPalLayerSettings *settings() const { return mSettings.get(); }

/**
* Determines if scale based labeling is active
Expand Down Expand Up @@ -325,7 +325,7 @@ class CORE_EXPORT QgsRuleBasedLabeling : public QgsAbstractVectorLayerLabeling

private:
Rule *mParent; // parent rule (NULL only for root rule)
QgsPalLayerSettings *mSettings = nullptr;
std::unique_ptr<QgsPalLayerSettings> mSettings;
double mMaximumScale = 0;
double mMinimumScale = 0;
QString mFilterExp, mDescription;
Expand All @@ -336,8 +336,7 @@ class CORE_EXPORT QgsRuleBasedLabeling : public QgsAbstractVectorLayerLabeling

QString mRuleKey; // string used for unique identification of rule within labeling

// temporary
QgsExpression *mFilter = nullptr;
std::unique_ptr<QgsExpression> mFilter;

};

Expand Down
9 changes: 8 additions & 1 deletion tests/src/python/test_qgssymbollayer_createsld.py
Expand Up @@ -32,7 +32,7 @@
QgsFontMarkerSymbolLayer, QgsEllipseSymbolLayer, QgsSimpleLineSymbolLayer,
QgsMarkerLineSymbolLayer, QgsMarkerSymbol, QgsSimpleFillSymbolLayer, QgsSVGFillSymbolLayer,
QgsLinePatternFillSymbolLayer, QgsPointPatternFillSymbolLayer, QgsVectorLayer, QgsVectorLayerSimpleLabeling,
QgsTextBufferSettings, QgsPalLayerSettings, QgsTextBackgroundSettings)
QgsTextBufferSettings, QgsPalLayerSettings, QgsTextBackgroundSettings, QgsRuleBasedLabeling)
from qgis.testing import start_app, unittest
from utilities import unitTestDataPath

Expand Down Expand Up @@ -1053,6 +1053,13 @@ def testRuleBasedLabels(self):
ltValue = gt.childNodes().item(1)
self.assertEqual("1000000", gtValue.toElement().text())

# check that adding a rule without settings does not segfault
xml1 = dom.toString()
layer.labeling().rootRule().appendChild(QgsRuleBasedLabeling.Rule(None))
dom, root = self.layerToSld(layer)
xml2 = dom.toString()
self.assertEqual(xml1, xml2)

def updateLinePlacementProperties(self, layer, linePlacement, distance, repeat, maxAngleInternal=25, maxAngleExternal=-25):
settings = layer.labeling().settings()
settings.placement = linePlacement
Expand Down

0 comments on commit b6901e2

Please sign in to comment.