Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Rule based renderer: check for scale in rulesForFeature
... when context is not nullptr, also don't crash if it is

With tests

Fixes #21287
  • Loading branch information
elpaso committed Feb 15, 2019
1 parent 7980cef commit 091eeb2
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 4 deletions.
4 changes: 2 additions & 2 deletions src/core/symbology/qgsrulebasedrenderer.cpp
Expand Up @@ -246,7 +246,7 @@ QgsLegendSymbolList QgsRuleBasedRenderer::Rule::legendSymbolItems( int currentLe

bool QgsRuleBasedRenderer::Rule::isFilterOK( const QgsFeature &f, QgsRenderContext *context ) const
{
if ( ! mFilter || mElseRule )
if ( ! mFilter || mElseRule || ! context )
return true;

context->expressionContext().setFeature( f );
Expand Down Expand Up @@ -647,7 +647,7 @@ QSet<QString> QgsRuleBasedRenderer::Rule::legendKeysForFeature( const QgsFeature
QgsRuleBasedRenderer::RuleList QgsRuleBasedRenderer::Rule::rulesForFeature( const QgsFeature &feature, QgsRenderContext *context, bool onlyActive )
{
RuleList lst;
if ( !isFilterOK( feature, context ) )
if ( ! isFilterOK( feature, context ) || ( context && ! isScaleOK( context->rendererScale() ) ) )
return lst;

if ( mSymbol )
Expand Down
43 changes: 41 additions & 2 deletions tests/src/python/test_qgsrulebasedrenderer.py
Expand Up @@ -150,12 +150,12 @@ def testWillRenderFeatureNestedElse(self):
vl.setRenderer(QgsRuleBasedRenderer(rootrule))
renderer = vl.renderer()

# Reunder with else rule and all activated
# Render with else rule and all activated
renderer.startRender(ctx, vl.fields())
self.assertTrue(renderer.willRenderFeature(ft, ctx))
renderer.stopRender(ctx)

# Reunder with else rule where else is deactivated
# Render with else rule where else is deactivated
renderer.rootRule().children()[1].setActive(False)
renderer.startRender(ctx, vl.fields())
self.assertFalse(renderer.willRenderFeature(ft, ctx))
Expand Down Expand Up @@ -333,6 +333,45 @@ def testConvertFromGraduatedRenderer(self):
self.assertEqual(r.rootRule().children()[0].filterExpression(), '"id" >= 0.000000 AND "id" <= 1.000000')
self.assertEqual(r.rootRule().children()[1].filterExpression(), '"id" > 1.000000 AND "id" <= 2.000000')

def testWillRenderFeatureTwoElse(self):
"""Regression #21287, also test rulesForFeature since there were no tests any where and I've found a couple of issues"""

vl = self.mapsettings.layers()[0]
ft = vl.getFeature(0) # 'id' = 1

ctx = QgsRenderContext.fromMapSettings(self.mapsettings)
ctx.expressionContext().setFeature(ft)

# Create rulebased style
sym2 = QgsFillSymbol.createSimple({'color': '#71bd6c', 'outline_color': 'black'})
sym3 = QgsFillSymbol.createSimple({'color': '#1f78b4', 'outline_color': 'black'})
sym4 = QgsFillSymbol.createSimple({'color': '#ff00ff', 'outline_color': 'black'})

self.rx2 = QgsRuleBasedRenderer.Rule(sym2, 0, 0, '"id" = 200')
self.rx3 = QgsRuleBasedRenderer.Rule(sym3, 1000, 100000000, 'ELSE') # <<< - match this!
self.rx4 = QgsRuleBasedRenderer.Rule(sym4, 0.1, 999, 'ELSE')

rootrule = QgsRuleBasedRenderer.Rule(None)
rootrule.appendChild(self.rx2)
rootrule.appendChild(self.rx3)
rootrule.appendChild(self.rx4) # <- failed in regression #21287

vl.setRenderer(QgsRuleBasedRenderer(rootrule))
renderer = vl.renderer()

# Render with else rule and all activated
renderer.startRender(ctx, vl.fields())
self.assertTrue(renderer.willRenderFeature(ft, ctx))

# No context? All rules
self.assertEqual(len(rootrule.rulesForFeature(ft)), 2)
self.assertTrue(set(rootrule.rulesForFeature(ft)), set([self.rx3, self.rx4]))

# With context: only the matching one
self.assertEqual(len(rootrule.rulesForFeature(ft, ctx)), 1)
self.assertEqual(rootrule.rulesForFeature(ft, ctx)[0], self.rx3)
renderer.stopRender(ctx)


if __name__ == '__main__':
unittest.main()

0 comments on commit 091eeb2

Please sign in to comment.