Merge pull request #6807 from m-kuhn/ruleBasedNestedElse

m-kuhn · web-flow · commit fe27b011e44c · 2018-04-17T17:57:40.000+02:00
Fix identify tool with rule renderer and nested else
diff --git a/src/core/symbology/qgsrulebasedrenderer.cpp b/src/core/symbology/qgsrulebasedrenderer.cpp
@@ -549,15 +549,22 @@ bool QgsRuleBasedRenderer::Rule::willRenderFeature( QgsFeature &feat, QgsRenderC
   {
     if ( rule->isElse() )
     {
-      RuleList lst = rulesForFeature( feat, context, false );
-      lst.removeOne( rule );
+      if ( rule->children().isEmpty() )
+      {
+        RuleList lst = rulesForFeature( feat, context, false );
+        lst.removeOne( rule );
 
-      if ( lst.empty() )
+        if ( lst.empty() )
+        {
+          return true;
+        }
+      }
+      else
       {
-        return true;
+        return rule->willRenderFeature( feat, context );
       }
     }
-    else if ( !rule->isElse( ) && rule->willRenderFeature( feat, context ) )
+    else if ( rule->willRenderFeature( feat, context ) )
     {
       return true;
     }
diff --git a/tests/src/python/test_qgsrulebasedrenderer.py b/tests/src/python/test_qgsrulebasedrenderer.py
@@ -125,6 +125,42 @@ def testWillRenderFeature(self):
         renderer.stopRender(ctx)
         assert rendered == True
 
+    def testWillRenderFeatureNestedElse(self):
+        vl = self.mapsettings.layers()[0]
+        ft = vl.getFeature(0) # 'id' = 1
+
+        ctx = QgsRenderContext.fromMapSettings(self.mapsettings)
+        ctx.expressionContext().setFeature(ft)
+
+        # Create rulebased style
+        sym1 = QgsFillSymbol.createSimple({'color': '#fdbf6f', 'outline_color': 'black'})
+        sym2 = QgsFillSymbol.createSimple({'color': '#71bd6c', 'outline_color': 'black'})
+        sym3 = QgsFillSymbol.createSimple({'color': '#1f78b4', 'outline_color': 'black'})
+
+        self.rx1 = QgsRuleBasedRenderer.Rule(sym1, 0, 0, '"id" = 1')
+        self.rx2 = QgsRuleBasedRenderer.Rule(sym2, 0, 0, '"id" = 2')
+        self.rx3 = QgsRuleBasedRenderer.Rule(sym3, 0, 0, 'ELSE')
+
+        self.rx3.appendChild(self.rx1)
+
+        rootrule = QgsRuleBasedRenderer.Rule(None)
+        rootrule.appendChild(self.rx2)
+        rootrule.appendChild(self.rx3)
+
+        vl.setRenderer(QgsRuleBasedRenderer(rootrule))
+        renderer = vl.renderer()
+
+        # Reunder 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
+        renderer.rootRule().children()[1].setActive(False)
+        renderer.startRender(ctx, vl.fields())
+        self.assertFalse(renderer.willRenderFeature(ft, ctx))
+        renderer.stopRender(ctx)
+
     def testFeatureCount(self):
         vl = self.mapsettings.layers()[0]
         ft = vl.getFeature(2) # 'id' = 3 => ELSE

Original file line number	Diff line number	Diff line change
`@@ -549,15 +549,22 @@ bool QgsRuleBasedRenderer::Rule::willRenderFeature( QgsFeature &feat, QgsRenderC`
`549`	`549`	`{`
`550`	`550`	`if ( rule->isElse() )`
`551`	`551`	`{`
`552`		`- RuleList lst = rulesForFeature( feat, context, false );`
`553`		`- lst.removeOne( rule );`
	`552`	`+ if ( rule->children().isEmpty() )`
	`553`	`+ {`
	`554`	`+ RuleList lst = rulesForFeature( feat, context, false );`
	`555`	`+ lst.removeOne( rule );`
`554`	`556`
`555`		`- if ( lst.empty() )`
	`557`	`+ if ( lst.empty() )`
	`558`	`+ {`
	`559`	`+ return true;`
	`560`	`+ }`
	`561`	`+ }`
	`562`	`+ else`
`556`	`563`	`{`
`557`		`- return true;`
	`564`	`+ return rule->willRenderFeature( feat, context );`
`558`	`565`	`}`
`559`	`566`	`}`
`560`		`- else if ( !rule->isElse( ) && rule->willRenderFeature( feat, context ) )`
	`567`	`+ else if ( rule->willRenderFeature( feat, context ) )`
`561`	`568`	`{`
`562`	`569`	`return true;`
`563`	`570`	`}`