Skip to content

Commit 2c0dc09

Browse files
nyalldawsonnyalldawson
committedJan 8, 2019
Correctly handle conversion of merged categories to rule based renderer
·
ltr-3_40final-3_6_0
1 parent 1051f9b commit 2c0dc09

File tree

2 files changed

+70
-33
lines changed

2 files changed

+70
-33
lines changed
 

‎src/core/symbology/qgsrulebasedrenderer.cpp

Lines changed: 61 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1239,19 +1239,19 @@ QSet< QString > QgsRuleBasedRenderer::legendKeysForFeature( const QgsFeature &fe
12391239

12401240
QgsRuleBasedRenderer *QgsRuleBasedRenderer::convertFromRenderer( const QgsFeatureRenderer *renderer )
12411241
{
1242-
QgsRuleBasedRenderer *r = nullptr;
1242+
std::unique_ptr< QgsRuleBasedRenderer > r;
12431243
if ( renderer->type() == QLatin1String( "RuleRenderer" ) )
12441244
{
1245-
r = dynamic_cast<QgsRuleBasedRenderer *>( renderer->clone() );
1245+
r.reset( dynamic_cast<QgsRuleBasedRenderer *>( renderer->clone() ) );
12461246
}
12471247
else if ( renderer->type() == QLatin1String( "singleSymbol" ) )
12481248
{
12491249
const QgsSingleSymbolRenderer *singleSymbolRenderer = dynamic_cast<const QgsSingleSymbolRenderer *>( renderer );
12501250
if ( !singleSymbolRenderer )
12511251
return nullptr;
12521252

1253-
QgsSymbol *origSymbol = singleSymbolRenderer->symbol()->clone();
1254-
r = new QgsRuleBasedRenderer( origSymbol );
1253+
std::unique_ptr< QgsSymbol > origSymbol( singleSymbolRenderer->symbol()->clone() );
1254+
r = qgis::make_unique< QgsRuleBasedRenderer >( origSymbol.release() );
12551255
}
12561256
else if ( renderer->type() == QLatin1String( "categorizedSymbol" ) )
12571257
{
@@ -1269,51 +1269,79 @@ QgsRuleBasedRenderer *QgsRuleBasedRenderer::convertFromRenderer( const QgsFeatur
12691269
attr = QgsExpression::quotedColumnRef( attr );
12701270
}
12711271

1272-
QgsRuleBasedRenderer::Rule *rootrule = new QgsRuleBasedRenderer::Rule( nullptr );
1272+
std::unique_ptr< QgsRuleBasedRenderer::Rule > rootrule = qgis::make_unique< QgsRuleBasedRenderer::Rule >( nullptr );
12731273

12741274
QString expression;
12751275
QString value;
12761276
QgsRendererCategory category;
1277-
for ( int i = 0; i < categorizedRenderer->categories().size(); ++i )
1277+
for ( const QgsRendererCategory &category : categorizedRenderer->categories() )
12781278
{
1279-
category = categorizedRenderer->categories().value( i );
1280-
QgsRuleBasedRenderer::Rule *rule = new QgsRuleBasedRenderer::Rule( nullptr );
1279+
std::unique_ptr< QgsRuleBasedRenderer::Rule > rule = qgis::make_unique< QgsRuleBasedRenderer::Rule >( nullptr );
12811280

12821281
rule->setLabel( category.label() );
12831282

12841283
//We first define the rule corresponding to the category
1285-
//If the value is a number, we can use it directly, otherwise we need to quote it in the rule
1286-
if ( QVariant( category.value() ).convert( QVariant::Double ) )
1284+
if ( category.value().type() == QVariant::List )
12871285
{
1288-
value = category.value().toString();
1289-
}
1290-
else
1291-
{
1292-
value = QgsExpression::quotedString( category.value().toString() );
1293-
}
1286+
QStringList values;
1287+
const QVariantList list = category.value().toList();
1288+
for ( const QVariant &v : list )
1289+
{
1290+
//If the value is a number, we can use it directly, otherwise we need to quote it in the rule
1291+
if ( QVariant( v ).convert( QVariant::Double ) )
1292+
{
1293+
values << v.toString();
1294+
}
1295+
else
1296+
{
1297+
values << QgsExpression::quotedString( v.toString() );
1298+
}
1299+
}
12941300

1295-
//An empty category is equivalent to the ELSE keyword
1296-
if ( value == QLatin1String( "''" ) )
1297-
{
1298-
expression = QStringLiteral( "ELSE" );
1301+
if ( values.empty() )
1302+
{
1303+
expression = QStringLiteral( "ELSE" );
1304+
}
1305+
else
1306+
{
1307+
expression = QStringLiteral( "%1 IN (%2)" ).arg( attr, values.join( ',' ) );
1308+
}
12991309
}
13001310
else
13011311
{
1302-
expression = QStringLiteral( "%1 = %2" ).arg( attr, value );
1312+
//If the value is a number, we can use it directly, otherwise we need to quote it in the rule
1313+
if ( category.value().convert( QVariant::Double ) )
1314+
{
1315+
value = category.value().toString();
1316+
}
1317+
else
1318+
{
1319+
value = QgsExpression::quotedString( category.value().toString() );
1320+
}
1321+
1322+
//An empty category is equivalent to the ELSE keyword
1323+
if ( value == QLatin1String( "''" ) )
1324+
{
1325+
expression = QStringLiteral( "ELSE" );
1326+
}
1327+
else
1328+
{
1329+
expression = QStringLiteral( "%1 = %2" ).arg( attr, value );
1330+
}
13031331
}
13041332
rule->setFilterExpression( expression );
13051333

13061334
//Then we construct an equivalent symbol.
13071335
//Ideally we could simply copy the symbol, but the categorized renderer allows a separate interface to specify
13081336
//data dependent area and rotation, so we need to convert these to obtain the same rendering
13091337

1310-
QgsSymbol *origSymbol = category.symbol()->clone();
1311-
rule->setSymbol( origSymbol );
1338+
std::unique_ptr< QgsSymbol > origSymbol( category.symbol()->clone() );
1339+
rule->setSymbol( origSymbol.release() );
13121340

1313-
rootrule->appendChild( rule );
1341+
rootrule->appendChild( rule.release() );
13141342
}
13151343

1316-
r = new QgsRuleBasedRenderer( rootrule );
1344+
r = qgis::make_unique< QgsRuleBasedRenderer >( rootrule.release() );
13171345
}
13181346
else if ( renderer->type() == QLatin1String( "graduatedSymbol" ) )
13191347
{
@@ -1336,14 +1364,14 @@ QgsRuleBasedRenderer *QgsRuleBasedRenderer::convertFromRenderer( const QgsFeatur
13361364
attr = QStringLiteral( "(%1)" ).arg( attr );
13371365
}
13381366

1339-
QgsRuleBasedRenderer::Rule *rootrule = new QgsRuleBasedRenderer::Rule( nullptr );
1367+
std::unique_ptr< QgsRuleBasedRenderer::Rule > rootrule = qgis::make_unique< QgsRuleBasedRenderer::Rule >( nullptr );
13401368

13411369
QString expression;
13421370
QgsRendererRange range;
13431371
for ( int i = 0; i < graduatedRenderer->ranges().size(); ++i )
13441372
{
13451373
range = graduatedRenderer->ranges().value( i );
1346-
QgsRuleBasedRenderer::Rule *rule = new QgsRuleBasedRenderer::Rule( nullptr );
1374+
std::unique_ptr< QgsRuleBasedRenderer::Rule > rule = qgis::make_unique< QgsRuleBasedRenderer::Rule >( nullptr );
13471375
rule->setLabel( range.label() );
13481376
if ( i == 0 )//The lower boundary of the first range is included, while it is excluded for the others
13491377
{
@@ -1361,13 +1389,13 @@ QgsRuleBasedRenderer *QgsRuleBasedRenderer::convertFromRenderer( const QgsFeatur
13611389
//Ideally we could simply copy the symbol, but the graduated renderer allows a separate interface to specify
13621390
//data dependent area and rotation, so we need to convert these to obtain the same rendering
13631391

1364-
QgsSymbol *symbol = range.symbol()->clone();
1365-
rule->setSymbol( symbol );
1392+
std::unique_ptr< QgsSymbol > symbol( range.symbol()->clone() );
1393+
rule->setSymbol( symbol.release() );
13661394

1367-
rootrule->appendChild( rule );
1395+
rootrule->appendChild( rule.release() );
13681396
}
13691397

1370-
r = new QgsRuleBasedRenderer( rootrule );
1398+
r = qgis::make_unique< QgsRuleBasedRenderer >( rootrule.release() );
13711399
}
13721400
else if ( renderer->type() == QLatin1String( "pointDisplacement" ) || renderer->type() == QLatin1String( "pointCluster" ) )
13731401
{
@@ -1379,7 +1407,7 @@ QgsRuleBasedRenderer *QgsRuleBasedRenderer::convertFromRenderer( const QgsFeatur
13791407
{
13801408
const QgsInvertedPolygonRenderer *invertedPolygonRenderer = dynamic_cast<const QgsInvertedPolygonRenderer *>( renderer );
13811409
if ( invertedPolygonRenderer )
1382-
r = convertFromRenderer( invertedPolygonRenderer->embeddedRenderer() );
1410+
r.reset( convertFromRenderer( invertedPolygonRenderer->embeddedRenderer() ) );
13831411
}
13841412

13851413
if ( r )
@@ -1388,7 +1416,7 @@ QgsRuleBasedRenderer *QgsRuleBasedRenderer::convertFromRenderer( const QgsFeatur
13881416
r->setOrderByEnabled( renderer->orderByEnabled() );
13891417
}
13901418

1391-
return r;
1419+
return r.release();
13921420
}
13931421

13941422
void QgsRuleBasedRenderer::convertToDataDefinedSymbology( QgsSymbol *symbol, const QString &sizeScaleField, const QString &rotationField )

‎tests/src/python/test_qgsrulebasedrenderer.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,35 +261,44 @@ def testConvertFromCategorisedRenderer(self):
261261
cats.append(QgsRendererCategory('a\nb', QgsMarkerSymbol(), "id a\\nb"))
262262
cats.append(QgsRendererCategory('a\\b', QgsMarkerSymbol(), "id a\\\\b"))
263263
cats.append(QgsRendererCategory('a\tb', QgsMarkerSymbol(), "id a\\tb"))
264+
cats.append(QgsRendererCategory(['c', 'd'], QgsMarkerSymbol(), "c/d"))
264265
c = QgsCategorizedSymbolRenderer("id", cats)
265266

266267
r = QgsRuleBasedRenderer.convertFromRenderer(c)
268+
self.assertEqual(len(r.rootRule().children()), 7)
267269
self.assertEqual(r.rootRule().children()[0].filterExpression(), '"id" = 1')
268270
self.assertEqual(r.rootRule().children()[1].filterExpression(), '"id" = 2')
269271
self.assertEqual(r.rootRule().children()[2].filterExpression(), '"id" = \'a\'\'b\'')
270272
self.assertEqual(r.rootRule().children()[3].filterExpression(), '"id" = \'a\\nb\'')
271273
self.assertEqual(r.rootRule().children()[4].filterExpression(), '"id" = \'a\\\\b\'')
272274
self.assertEqual(r.rootRule().children()[5].filterExpression(), '"id" = \'a\\tb\'')
275+
self.assertEqual(r.rootRule().children()[6].filterExpression(), '"id" IN (\'c\',\'d\')')
273276

274277
# Next try with an expression based category
275278
cats = []
276279
cats.append(QgsRendererCategory(1, QgsMarkerSymbol(), "result 1"))
277280
cats.append(QgsRendererCategory(2, QgsMarkerSymbol(), "result 2"))
281+
cats.append(QgsRendererCategory([3, 4], QgsMarkerSymbol(), "result 3/4"))
278282
c = QgsCategorizedSymbolRenderer("id + 1", cats)
279283

280284
r = QgsRuleBasedRenderer.convertFromRenderer(c)
285+
self.assertEqual(len(r.rootRule().children()), 3)
281286
self.assertEqual(r.rootRule().children()[0].filterExpression(), 'id + 1 = 1')
282287
self.assertEqual(r.rootRule().children()[1].filterExpression(), 'id + 1 = 2')
288+
self.assertEqual(r.rootRule().children()[2].filterExpression(), 'id + 1 IN (3,4)')
283289

284290
# Last try with an expression which is just a quoted field name
285291
cats = []
286292
cats.append(QgsRendererCategory(1, QgsMarkerSymbol(), "result 1"))
287293
cats.append(QgsRendererCategory(2, QgsMarkerSymbol(), "result 2"))
294+
cats.append(QgsRendererCategory([3, 4], QgsMarkerSymbol(), "result 3/4"))
288295
c = QgsCategorizedSymbolRenderer('"id"', cats)
289296

290297
r = QgsRuleBasedRenderer.convertFromRenderer(c)
298+
self.assertEqual(len(r.rootRule().children()), 3)
291299
self.assertEqual(r.rootRule().children()[0].filterExpression(), '"id" = 1')
292300
self.assertEqual(r.rootRule().children()[1].filterExpression(), '"id" = 2')
301+
self.assertEqual(r.rootRule().children()[2].filterExpression(), '"id" IN (3,4)')
293302

294303
def testConvertFromGraduatedRenderer(self):
295304
# Test converting graduated renderer to rule based

0 commit comments

Comments
 (0)
Please sign in to comment.