Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RuleBasedRendering] Keep rule key while cloning when needed #60490

Merged
merged 2 commits into from
Feb 7, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -219,9 +219,13 @@ Try to find a rule given its unique key
%End


QgsRuleBasedLabeling::Rule *clone() const /Factory/;
QgsRuleBasedLabeling::Rule *clone( bool resetRuleKey = true ) const /Factory/;
%Docstring
clone this rule, return new instance
clone this rule

:param resetRuleKey: ``True`` if this rule and its children rule key need to be reset to new unique ones.

:return: new instance
%End


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,9 +219,13 @@ Try to find a rule given its unique key
%End


QgsRuleBasedLabeling::Rule *clone() const /Factory/;
QgsRuleBasedLabeling::Rule *clone( bool resetRuleKey = true ) const /Factory/;
%Docstring
clone this rule, return new instance
clone this rule

:param resetRuleKey: ``True`` if this rule and its children rule key need to be reset to new unique ones.

:return: new instance
%End


Expand Down
9 changes: 6 additions & 3 deletions src/core/labeling/qgsrulebasedlabeling.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -233,14 +233,18 @@ QgsRuleBasedLabeling::Rule *QgsRuleBasedLabeling::Rule::findRuleByKey( const QSt
return nullptr;
}

QgsRuleBasedLabeling::Rule *QgsRuleBasedLabeling::Rule::clone() const
QgsRuleBasedLabeling::Rule *QgsRuleBasedLabeling::Rule::clone( bool resetRuleKey ) const
{
QgsPalLayerSettings *s = mSettings.get() ? new QgsPalLayerSettings( *mSettings ) : nullptr;
Rule *newrule = new Rule( s, mMaximumScale, mMinimumScale, mFilterExp, mDescription );
newrule->setActive( mIsActive );
if ( !resetRuleKey )
newrule->setRuleKey( ruleKey() );

// clone children
for ( Rule *rule : mChildren )
newrule->appendChild( rule->clone() );
newrule->appendChild( rule->clone( resetRuleKey ) );

return newrule;
}

Expand Down Expand Up @@ -639,4 +643,3 @@ void QgsRuleBasedLabeling::multiplyOpacity( double opacityFactor )

}
}

8 changes: 6 additions & 2 deletions src/core/labeling/qgsrulebasedlabeling.h
Original file line number Diff line number Diff line change
Expand Up @@ -241,8 +241,12 @@ class CORE_EXPORT QgsRuleBasedLabeling : public QgsAbstractVectorLayerLabeling
*/
QgsRuleBasedLabeling::Rule *findRuleByKey( const QString &key ) SIP_SKIP;

//! clone this rule, return new instance
QgsRuleBasedLabeling::Rule *clone() const SIP_FACTORY;
/**
* clone this rule
* \param resetRuleKey TRUE if this rule and its children rule key need to be reset to new unique ones.
* \returns new instance
*/
QgsRuleBasedLabeling::Rule *clone( bool resetRuleKey = true ) const SIP_FACTORY;

// load / save

Expand Down
3 changes: 1 addition & 2 deletions src/gui/labeling/qgslabelingwidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,7 @@ void QgsLabelingWidget::writeSettingsToLayer()
case ModeRuleBased:
{
const QgsRuleBasedLabeling::Rule *rootRule = qobject_cast<QgsRuleBasedLabelingWidget *>( mWidget )->rootRule();

mLayer->setLabeling( new QgsRuleBasedLabeling( rootRule->clone() ) );
mLayer->setLabeling( new QgsRuleBasedLabeling( rootRule->clone( false ) ) );
mLayer->setLabelsEnabled( true );
break;
}
Expand Down
2 changes: 1 addition & 1 deletion src/gui/labeling/qgsrulebasedlabelingwidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ QgsRuleBasedLabelingWidget::QgsRuleBasedLabelingWidget( QgsVectorLayer *layer, Q
if ( mLayer->labeling() && mLayer->labeling()->type() == QLatin1String( "rule-based" ) )
{
const QgsRuleBasedLabeling *rl = static_cast<const QgsRuleBasedLabeling *>( mLayer->labeling() );
mRootRule = rl->rootRule()->clone();
mRootRule = rl->rootRule()->clone( false );
}
else if ( mLayer->labeling() && mLayer->labeling()->type() == QLatin1String( "simple" ) )
{
Expand Down
1 change: 1 addition & 0 deletions tests/src/gui/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ set(TESTS
testqgscompoundcolorwidget.cpp
testqgsmaskingwidget.cpp
testqgssvgselectorwidget.cpp
testqgslabelingwidget.cpp
)

foreach(TESTSRC ${TESTS})
Expand Down
108 changes: 108 additions & 0 deletions tests/src/gui/testqgslabelingwidget.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
/***************************************************************************
testqgslabelingwidget.cpp
---------------------
begin : 2025/02/06
copyright : (C) 2025 by Julien Cabieces
email : julien dot cabieces at oslandia dot com
***************************************************************************
* *
* This program is free software; you can redistribute it and/or modify *
* it under the terms of the GNU General Public License as published by *
* the Free Software Foundation; either version 2 of the License, or *
* (at your option) any later version. *
* *
***************************************************************************/

#include "qgstest.h"
#include "qgslabelingwidget.h"
#include "qgsvectorlayer.h"
#include "qgssymbollayerreference.h"
#include "qgsrulebasedlabeling.h"

class TestQgsLabelingWidget : public QgsTest
{
Q_OBJECT

public:
TestQgsLabelingWidget()
: QgsTest( QStringLiteral( "Labeling Widget Tests" ) ) {}

private slots:
void initTestCase(); // will be called before the first testfunction is executed.
void cleanupTestCase(); // will be called after the last testfunction was executed.
void init(); // will be called before each testfunction is executed.
void cleanup(); // will be called after every testfunction.

void testRuleKeyPreserved();
};

void TestQgsLabelingWidget::initTestCase()
{
}

void TestQgsLabelingWidget::cleanupTestCase()
{
}

void TestQgsLabelingWidget::init()
{
}

void TestQgsLabelingWidget::cleanup()
{
}

void TestQgsLabelingWidget::testRuleKeyPreserved()
{
// test that rule keys are preserved and not resetted when editing labels with a rule based rendering
nyalldawson marked this conversation as resolved.
Show resolved Hide resolved
QgsVectorLayer layer( QStringLiteral( "Point?field=pk:int" ), QStringLiteral( "layer" ), QStringLiteral( "memory" ) );

QgsFeature ft1( layer.fields() );
ft1.setAttribute( QStringLiteral( "pk" ), 1 );
ft1.setGeometry( QgsGeometry::fromWkt( QStringLiteral( "POINT( 1 1 )" ) ) );
layer.dataProvider()->addFeature( ft1 );

QgsFeature ft2( layer.fields() );
ft2.setAttribute( QStringLiteral( "pk" ), 2 );
ft2.setGeometry( QgsGeometry::fromWkt( QStringLiteral( "POINT( 2 2 )" ) ) );
layer.dataProvider()->addFeature( ft2 );

auto label_settings = std::make_unique<QgsPalLayerSettings>();
label_settings->fieldName = QStringLiteral( "pk" );

QgsTextMaskSettings mask;
mask.setEnabled( true );
mask.setMaskedSymbolLayers( QList<QgsSymbolLayerReference>() << QgsSymbolLayerReference( layer.id(), QStringLiteral( "test_unique_id" ) ) );

QgsTextFormat text_format = label_settings->format();
text_format.setMask( mask );
label_settings->setFormat( text_format );

auto root = std::make_unique<QgsRuleBasedLabeling::Rule>( new QgsPalLayerSettings() );

auto rule = std::make_unique<QgsRuleBasedLabeling::Rule>( label_settings.release() );
rule->setDescription( "test rule" );
rule->setFilterExpression( QStringLiteral( "\"{pk}\" % 2 = 0" ) );
rule->setActive( true );

const QString rootRuleKey = root->ruleKey();
const QString ruleKey = rule->ruleKey();

root->appendChild( rule.release() );

auto ruleSettings = std::make_unique<QgsRuleBasedLabeling>( root.release() );
layer.setLabelsEnabled( true );
layer.setLabeling( ruleSettings.release() );

QgsLabelingWidget widget( &layer, nullptr );
widget.writeSettingsToLayer();

QgsRuleBasedLabeling *labelingAfter = dynamic_cast<QgsRuleBasedLabeling *>( layer.labeling() );
QVERIFY( labelingAfter );
QCOMPARE( labelingAfter->rootRule()->ruleKey(), rootRuleKey );
QCOMPARE( labelingAfter->rootRule()->children().count(), 1 );
QCOMPARE( labelingAfter->rootRule()->children().at( 0 )->ruleKey(), ruleKey );
}

QGSTEST_MAIN( TestQgsLabelingWidget )
#include "testqgslabelingwidget.moc"
Loading