Skip to content

Commit

Permalink
rule based renderer: skip else rule for disabled items
Browse files Browse the repository at this point in the history
  • Loading branch information
@m-kuhn
m-kuhn committed Sep 2, 2015
1 parent 03d4f63 commit ce1f657
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 23 deletions.
9 changes: 8 additions & 1 deletion python/core/symbology-ng/qgsrulebasedrendererv2.sip
View file
Expand Up @@ -54,6 +54,13 @@ class QgsRuleBasedRendererV2 : QgsFeatureRendererV2
class Rule
{
public:
enum RenderResult
{
Filtered = 0, //!< The rule does not apply
Inactive, //!< The rule is inactive
Rendered //!< Something was rendered
};

//! Constructor takes ownership of the symbol
Rule( QgsSymbolV2* symbol /Transfer/, int scaleMinDenom = 0, int scaleMaxDenom = 0, QString filterExp = QString(),
QString label = QString(), QString description = QString(), bool elseRule = false );
Expand Down Expand Up @@ -114,7 +121,7 @@ class QgsRuleBasedRendererV2 : QgsFeatureRendererV2
//! @note not available in python bindings
// void setNormZLevels( const QMap<int, int>& zLevelsToNormLevels );

bool renderFeature( QgsRuleBasedRendererV2::FeatureToRender& featToRender, QgsRenderContext& context, QgsRuleBasedRendererV2::RenderQueue& renderQueue );
QgsRuleBasedRendererV2::Rule::RenderResult renderFeature( QgsRuleBasedRendererV2::FeatureToRender& featToRender, QgsRenderContext& context, QgsRuleBasedRendererV2::RenderQueue& renderQueue );

//! only tell whether a feature will be rendered without actually rendering it
bool willRenderFeature( QgsFeature& feat, QgsRenderContext* context = 0);
Expand Down
39 changes: 21 additions & 18 deletions src/core/symbology-ng/qgsrulebasedrendererv2.cpp
View file
Expand Up @@ -39,7 +39,7 @@ QgsRuleBasedRendererV2::Rule::Rule( QgsSymbolV2* symbol, int scaleMinDenom, int
, mScaleMinDenom( scaleMinDenom ), mScaleMaxDenom( scaleMaxDenom )
, mFilterExp( filterExp ), mLabel( label ), mDescription( description )
, mElseRule( elseRule )
, mCheckState( true )
, mIsActive( true )
, mFilter( NULL )
{
mRuleKey = QUuid::createUuid().toString();
Expand Down Expand Up @@ -239,7 +239,7 @@ bool QgsRuleBasedRendererV2::Rule::isFilterOK( QgsFeature& f, QgsRenderContext*
return true;

context->expressionContext().setFeature( f );
QVariant res = mFilter->evaluate( context ? &context->expressionContext() : 0 );
QVariant res = mFilter->evaluate( &context->expressionContext() );
return res.toInt() != 0;
}

Expand All @@ -260,7 +260,7 @@ QgsRuleBasedRendererV2::Rule* QgsRuleBasedRendererV2::Rule::clone() const
{
QgsSymbolV2* sym = mSymbol ? mSymbol->clone() : NULL;
Rule* newrule = new Rule( sym, mScaleMinDenom, mScaleMaxDenom, mFilterExp, mLabel, mDescription );
newrule->setCheckState( mCheckState );
newrule->setCheckState( mIsActive );
// clone children
foreach ( Rule* rule, mChildren )
newrule->appendChild( rule->clone() );
Expand All @@ -287,7 +287,7 @@ QDomElement QgsRuleBasedRendererV2::Rule::save( QDomDocument& doc, QgsSymbolV2Ma
ruleElem.setAttribute( "label", mLabel );
if ( !mDescription.isEmpty() )
ruleElem.setAttribute( "description", mDescription );
if ( !mCheckState )
if ( !mIsActive )
ruleElem.setAttribute( "checkstate", 0 );
ruleElem.setAttribute( "key", mRuleKey );

Expand Down Expand Up @@ -401,7 +401,7 @@ bool QgsRuleBasedRendererV2::Rule::startRender( QgsRenderContext& context, const
{
mActiveChildren.clear();

if ( ! mCheckState )
if ( ! mIsActive )
return false;

// filter out rules which are not compatible with this scale
Expand Down Expand Up @@ -505,18 +505,18 @@ void QgsRuleBasedRendererV2::Rule::setNormZLevels( const QMap<int, int>& zLevels
}


bool QgsRuleBasedRendererV2::Rule::renderFeature( QgsRuleBasedRendererV2::FeatureToRender& featToRender, QgsRenderContext& context, QgsRuleBasedRendererV2::RenderQueue& renderQueue )
QgsRuleBasedRendererV2::Rule::RenderResult QgsRuleBasedRendererV2::Rule::renderFeature( QgsRuleBasedRendererV2::FeatureToRender& featToRender, QgsRenderContext& context, QgsRuleBasedRendererV2::RenderQueue& renderQueue )
{
if ( !isFilterOK( featToRender.feat, &context ) )
return false;
return Filtered;

bool rendered = false;

// create job for this feature and this symbol, add to list of jobs
if ( mSymbol )
if ( mSymbol && mIsActive )
{
// add job to the queue: each symbol's zLevel must be added
foreach ( int normZLevel, mSymbolNormZLevels )
Q_FOREACH ( int normZLevel, mSymbolNormZLevels )
{
//QgsDebugMsg(QString("add job at level %1").arg(normZLevel));
renderQueue[normZLevel].jobs.append( new RenderJob( featToRender, mSymbol ) );
Expand All @@ -527,16 +527,15 @@ bool QgsRuleBasedRendererV2::Rule::renderFeature( QgsRuleBasedRendererV2::Featur
bool willrendersomething = false;

// process children
for ( QList<Rule*>::iterator it = mActiveChildren.begin(); it != mActiveChildren.end(); ++it )
Q_FOREACH ( Rule* rule, mChildren )
{
Rule* rule = *it;
if ( rule->isElse() )
// Don't process else rules yet
if ( !rule->isElse() )
{
// Don't process else rules yet
continue;
RenderResult res = rule->renderFeature( featToRender, context, renderQueue );
willrendersomething |= ( res == Rendered );
rendered |= willrendersomething;
}
willrendersomething |= rule->renderFeature( featToRender, context, renderQueue );
rendered |= willrendersomething;
}

// If none of the rules passed then we jump into the else rules and process them.
Expand All @@ -547,8 +546,12 @@ bool QgsRuleBasedRendererV2::Rule::renderFeature( QgsRuleBasedRendererV2::Featur
rendered |= rule->renderFeature( featToRender, context, renderQueue );
}
}

return rendered;
if ( !mIsActive )
return Inactive;
else if ( rendered )
return Rendered;
else
return Filtered;
}

bool QgsRuleBasedRendererV2::Rule::willRenderFeature( QgsFeature& feat, QgsRenderContext *context )
Expand Down
28 changes: 24 additions & 4 deletions src/core/symbology-ng/qgsrulebasedrendererv2.h
View file
Expand Up @@ -84,6 +84,13 @@ class CORE_EXPORT QgsRuleBasedRendererV2 : public QgsFeatureRendererV2
class CORE_EXPORT Rule
{
public:
enum RenderResult
{
Filtered = 0, //!< The rule does not apply
Inactive, //!< The rule is inactive
Rendered //!< Something was rendered
};

//! Constructor takes ownership of the symbol
Rule( QgsSymbolV2* symbol, int scaleMinDenom = 0, int scaleMaxDenom = 0, QString filterExp = QString(),
QString label = QString(), QString description = QString(), bool elseRule = false );
Expand All @@ -107,7 +114,14 @@ class CORE_EXPORT QgsRuleBasedRendererV2 : public QgsFeatureRendererV2
QString filterExpression() const { return mFilterExp; }
QString description() const { return mDescription; }
//! @note added in 2.6
bool checkState() const { return mCheckState; }
//! @deprecated use active instead
bool checkState() const { return mIsActive; }
/**
* Returns if this rule is active
*
* @return True if the rule is active
*/
bool active() const { return mIsActive; }

//! Unique rule identifier (for identification of rule within renderer)
//! @note added in 2.6
Expand All @@ -124,7 +138,13 @@ class CORE_EXPORT QgsRuleBasedRendererV2 : public QgsFeatureRendererV2
void setFilterExpression( QString filterExp ) { mFilterExp = filterExp; initFilter(); }
void setDescription( QString description ) { mDescription = description; }
//! @note added in 2.6
void setCheckState( bool state ) { mCheckState = state; }
//! @deprecated use setActive instead
void setCheckState( bool state ) { mIsActive = state; }
/**
* Sets if this rule is active
* @param state Determines if the rule should be activated or deactivated
*/
void setActive( bool state ) { mIsActive = state; }

//! clone this rule, return new instance
Rule* clone() const;
Expand All @@ -144,7 +164,7 @@ class CORE_EXPORT QgsRuleBasedRendererV2 : public QgsFeatureRendererV2
//! @note not available in python bindings
void setNormZLevels( const QMap<int, int>& zLevelsToNormLevels );

bool renderFeature( FeatureToRender& featToRender, QgsRenderContext& context, RenderQueue& renderQueue );
RenderResult renderFeature( FeatureToRender& featToRender, QgsRenderContext& context, RenderQueue& renderQueue );

//! only tell whether a feature will be rendered without actually rendering it
bool willRenderFeature( QgsFeature& feat, QgsRenderContext* context = 0 );
Expand Down Expand Up @@ -195,7 +215,7 @@ class CORE_EXPORT QgsRuleBasedRendererV2 : public QgsFeatureRendererV2
bool mElseRule;
RuleList mChildren;
RuleList mElseRules;
bool mCheckState; // whether it is enabled or not
bool mIsActive; // whether it is enabled or not

QString mRuleKey; // string used for unique identification of rule within renderer

Expand Down

4 comments on commit ce1f657

@tudorbarascu
Copy link
Member

Choose a reason for hiding this comment

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

Should this also end up in the 2.8 branch?

@m-kuhn
Copy link
Member Author

@m-kuhn m-kuhn commented on ce1f657 Sep 6, 2015

Choose a reason for hiding this comment

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

Do you think it's worth it?

@tudorbarascu
Copy link
Member

Choose a reason for hiding this comment

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

As it's probably not that used, no.
However, I was curious if there's something like a rule that all the fixed bugs also end up in the LTS branch.

@m-kuhn
Copy link
Member Author

@m-kuhn m-kuhn commented on ce1f657 Sep 7, 2015

Choose a reason for hiding this comment

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

I wasn't sure how clearly this classifies as a "bug" and how as a "UX improvement". And it introduces slight changes to the API (although improbable to break anything). If it wasn't for that, I would backport it immediately.

Please sign in to comment.