Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bugfix] Fixes identify action on deactivated rules for QgsRuleBasedRenderer #6679

Merged
merged 7 commits into from
Mar 30, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions python/core/symbology/qgsrulebasedrenderer.sip.in
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Expand Down Expand Up @@ -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

Expand Down
46 changes: 41 additions & 5 deletions src/core/symbology/qgsrulebasedrenderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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 ) )
Expand All @@ -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;
}
Expand Down
13 changes: 10 additions & 3 deletions src/core/symbology/qgsrulebasedrenderer.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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();
Expand Down
49 changes: 48 additions & 1 deletion tests/src/python/test_qgsrulebasedrenderer.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@
QgsRendererCategory,
QgsCategorizedSymbolRenderer,
QgsGraduatedSymbolRenderer,
QgsRendererRange
QgsRendererRange,
QgsRenderContext
)
from qgis.testing import start_app, unittest
from utilities import unitTestDataPath
Expand Down Expand Up @@ -101,6 +102,52 @@ def testDisabledElse(self):

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a general input, better use the self.assertFalse() etc methods, they give a much better output if they fail.


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)

Expand Down