Skip to content

Commit

Permalink
Rule based renderer: check for scale in rulesForFeature
Browse files Browse the repository at this point in the history
... 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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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.