Skip to content
Permalink
Browse files

Merge pull request #7356 from mhugo/release-3_2

[3.2] Fix for rule based labeling toSld()
  • Loading branch information
Hugo Mercier
Hugo Mercier committed Jul 3, 2018
2 parents 9e5dccd + c48388b commit 1c5c9de5d5ab9fd177c8f54a81a6e7656f36f2aa
@@ -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();

@@ -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
@@ -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 );
}
}

@@ -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
@@ -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;
}
@@ -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();

@@ -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();

@@ -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
@@ -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;
@@ -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;

};

@@ -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

@@ -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

0 comments on commit 1c5c9de

Please sign in to comment.
You can’t perform that action at this time.