Merge pull request #6679 from pblottiere/bugfix_will_render_feature

[bugfix] Fixes identify action on deactivated rules for QgsRuleBasedRenderer

python/core/symbology/qgsrulebasedrenderer.sip.in

@@ -324,9 +324,14 @@ Returns which legend keys match the feature
 .. versionadded:: 2.14
 %End
 
-        QgsRuleBasedRenderer::RuleList rulesForFeature( QgsFeature &feat, QgsRenderContext *context = 0 );
+        QgsRuleBasedRenderer::RuleList rulesForFeature( QgsFeature &feat, QgsRenderContext *context = 0, bool onlyActive = true );
 %Docstring
-tell which rules will be used to render the feature
+Returns the list of rules used to render the feature in a specific
+context.
+
+:param feat: The feature for which rules have to be find
+:param context: The rendering context
+:param onlyActive: True to search for active rules only, false otherwise
 %End
 
         void stopRender( QgsRenderContext &context );
@@ -411,7 +416,7 @@ Sets if this rule is an ELSE rule
 :param iselse: If true, this rule is an ELSE rule
 %End
 
-        bool isElse();
+        bool isElse() const;
 %Docstring
 Check if this rule is an ELSE rule
 

src/core/symbology/qgsrulebasedrenderer.cpp

@@ -541,13 +541,26 @@ bool QgsRuleBasedRenderer::Rule::willRenderFeature( QgsFeature &feat, QgsRenderC
 {
   if ( !isFilterOK( feat, context ) )
     return false;
+
   if ( mSymbol )
     return true;
 
   Q_FOREACH ( Rule *rule, mActiveChildren )
   {
-    if ( rule->willRenderFeature( feat, context ) )
+    if ( rule->isElse() )
+    {
+      RuleList lst = rulesForFeature( feat, context, false );
+      lst.removeOne( rule );
+
+      if ( lst.empty() )
+      {
+        return true;
+      }
+    }
+    else if ( !rule->isElse( ) && rule->willRenderFeature( feat, context ) )
+    {
       return true;
+    }
   }
   return false;
 }
@@ -576,12 +589,31 @@ QSet<QString> QgsRuleBasedRenderer::Rule::legendKeysForFeature( QgsFeature &feat
 
   Q_FOREACH ( Rule *rule, mActiveChildren )
   {
-    lst.unite( rule->legendKeysForFeature( feat, context ) );
+    bool validKey = false;
+    if ( rule->isElse() )
+    {
+      RuleList lst = rulesForFeature( feat, context, false );
+      lst.removeOne( rule );
+
+      if ( lst.empty() )
+      {
+        validKey = true;
+      }
+    }
+    else if ( !rule->isElse( ) && rule->willRenderFeature( feat, context ) )
+    {
+      validKey = true;
+    }
+
+    if ( validKey )
+    {
+      lst.unite( rule->legendKeysForFeature( feat, context ) );
+    }
   }
   return lst;
 }
 
-QgsRuleBasedRenderer::RuleList QgsRuleBasedRenderer::Rule::rulesForFeature( QgsFeature &feat, QgsRenderContext *context )
+QgsRuleBasedRenderer::RuleList QgsRuleBasedRenderer::Rule::rulesForFeature( QgsFeature &feat, QgsRenderContext *context, bool onlyActive )
 {
   RuleList lst;
   if ( !isFilterOK( feat, context ) )
@@ -590,9 +622,13 @@ QgsRuleBasedRenderer::RuleList QgsRuleBasedRenderer::Rule::rulesForFeature( QgsF
   if ( mSymbol )
     lst.append( this );
 
-  Q_FOREACH ( Rule *rule, mActiveChildren )
+  RuleList listChildren = children();
+  if ( onlyActive )
+    listChildren = mActiveChildren;
+
+  Q_FOREACH ( Rule *rule, listChildren )
   {
-    lst += rule->rulesForFeature( feat, context );
+    lst += rule->rulesForFeature( feat, context, onlyActive );
   }
   return lst;
 }

src/core/symbology/qgsrulebasedrenderer.h

@@ -342,8 +342,15 @@ class CORE_EXPORT QgsRuleBasedRenderer : public QgsFeatureRenderer
          */
         QSet< QString > legendKeysForFeature( QgsFeature &feat, QgsRenderContext *context = nullptr );
 
-        //! tell which rules will be used to render the feature
-        QgsRuleBasedRenderer::RuleList rulesForFeature( QgsFeature &feat, QgsRenderContext *context = nullptr );
+        /**
+         * Returns the list of rules used to render the feature in a specific
+         * context.
+         *
+         * \param feat The feature for which rules have to be find
+         * \param context The rendering context
+         * \param onlyActive True to search for active rules only, false otherwise
+         */
+        QgsRuleBasedRenderer::RuleList rulesForFeature( QgsFeature &feat, QgsRenderContext *context = nullptr, bool onlyActive = true );
 
         /**
          * Stop a rendering process. Used to clean up the internal state of this rule
@@ -419,7 +426,7 @@ class CORE_EXPORT QgsRuleBasedRenderer : public QgsFeatureRenderer
          *
          * \returns True if this rule is an else rule
          */
-        bool isElse() { return mElseRule; }
+        bool isElse() const { return mElseRule; }
 
       protected:
         void initFilter();

tests/src/python/test_qgsrulebasedrenderer.py

@@ -40,7 +40,8 @@ from qgis.core import (QgsVectorLayer,
                        QgsRendererCategory,
                        QgsCategorizedSymbolRenderer,
                        QgsGraduatedSymbolRenderer,
-                       QgsRendererRange
+                       QgsRendererRange,
+                       QgsRenderContext
                        )
 from qgis.testing import start_app, unittest
 from utilities import unitTestDataPath
@@ -101,6 +102,52 @@ class TestQgsRulebasedRenderer(unittest.TestCase):
 
         assert result
 
+    def testWillRenderFeature(self):
+        vl = self.mapsettings.layers()[0]
+        ft = vl.getFeature(0) # 'id' = 1
+        renderer = vl.renderer()
+
+        ctx = QgsRenderContext.fromMapSettings(self.mapsettings)
+        ctx.expressionContext().setFeature(ft)
+
+        renderer.rootRule().children()[0].setActive(False)
+        renderer.rootRule().children()[1].setActive(True)
+        renderer.rootRule().children()[2].setActive(True)
+
+        renderer.startRender(ctx, vl.fields()) # build mActiveChlidren
+        rendered = renderer.willRenderFeature(ft, ctx)
+        renderer.stopRender(ctx)
+        renderer.rootRule().children()[0].setActive(True)
+        assert rendered == False
+
+        renderer.startRender(ctx, vl.fields()) # build mActiveChlidren
+        rendered = renderer.willRenderFeature(ft, ctx)
+        renderer.stopRender(ctx)
+        assert rendered == True
+
+    def testFeatureCount(self):
+        vl = self.mapsettings.layers()[0]
+        ft = vl.getFeature(2) # 'id' = 3 => ELSE
+        renderer = vl.renderer()
+
+        ctx = QgsRenderContext.fromMapSettings(self.mapsettings)
+        ctx.expressionContext().setFeature(ft)
+
+        counter = vl.countSymbolFeatures()
+        counter.waitForFinished()
+
+        renderer.startRender(ctx, vl.fields())
+
+        elseRule = None
+        for rule in renderer.rootRule().children():
+            if rule.filterExpression() == 'ELSE':
+                elseRule = rule
+
+        assert elseRule != None
+
+        cnt = counter.featureCount(elseRule.ruleKey())
+        assert cnt == 1
+
     def testRefineWithCategories(self):
         # Test refining rule with categories (refs #10815)