Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Clearer ownership, docs for rule based renderer
  • Loading branch information
nyalldawson committed Mar 15, 2018
1 parent 685adbf commit 7c0665a
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 47 deletions.
9 changes: 6 additions & 3 deletions python/core/symbology/qgsrulebasedrenderer.sip.in
Expand Up @@ -39,7 +39,12 @@ the rules and draws features with symbols from rules that match.
struct RenderJob
{
RenderJob( QgsRuleBasedRenderer::FeatureToRender &_ftr, QgsSymbol *_s );

QgsRuleBasedRenderer::FeatureToRender &ftr;
%Docstring
Feature to render
%End

QgsSymbol *symbol;
};

Expand All @@ -48,6 +53,7 @@ the rules and draws features with symbols from rules that match.
explicit RenderLevel( int z );
~RenderLevel();
int zIndex;

QList<QgsRuleBasedRenderer::RenderJob *> jobs;


Expand Down Expand Up @@ -415,9 +421,6 @@ Check if this rule is an ELSE rule
protected:
void initFilter();




private:
Rule( const QgsRuleBasedRenderer::Rule &rh );
};
Expand Down
27 changes: 10 additions & 17 deletions src/core/symbology/qgsrulebasedrenderer.cpp
Expand Up @@ -56,8 +56,6 @@ QgsRuleBasedRenderer::Rule::Rule( QgsSymbol *symbol, int scaleMinDenom, int scal

QgsRuleBasedRenderer::Rule::~Rule()
{
delete mSymbol;
delete mFilter;
qDeleteAll( mChildren );
// do NOT delete parent
}
Expand All @@ -67,20 +65,17 @@ void QgsRuleBasedRenderer::Rule::initFilter()
if ( mFilterExp.trimmed().compare( QLatin1String( "ELSE" ), Qt::CaseInsensitive ) == 0 )
{
mElseRule = true;
delete mFilter;
mFilter = nullptr;
mFilter.reset();
}
else if ( mFilterExp.trimmed().isEmpty() )
{
mElseRule = false;
delete mFilter;
mFilter = nullptr;
mFilter.reset();
}
else
{
mElseRule = false;
delete mFilter;
mFilter = new QgsExpression( mFilterExp );
mFilter = qgis::make_unique< QgsExpression >( mFilterExp );
}
}

Expand Down Expand Up @@ -157,8 +152,7 @@ void QgsRuleBasedRenderer::Rule::setIsElse( bool iselse )
{
mFilterExp = QStringLiteral( "ELSE" );
mElseRule = iselse;
delete mFilter;
mFilter = nullptr;
mFilter.reset();
}


Expand Down Expand Up @@ -215,7 +209,7 @@ QgsSymbolList QgsRuleBasedRenderer::Rule::symbols( const QgsRenderContext &conte
{
QgsSymbolList lst;
if ( mSymbol )
lst.append( mSymbol );
lst.append( mSymbol.get() );

Q_FOREACH ( Rule *rule, mChildren )
{
Expand All @@ -226,8 +220,7 @@ QgsSymbolList QgsRuleBasedRenderer::Rule::symbols( const QgsRenderContext &conte

void QgsRuleBasedRenderer::Rule::setSymbol( QgsSymbol *sym )
{
delete mSymbol;
mSymbol = sym;
mSymbol.reset( sym );
}

void QgsRuleBasedRenderer::Rule::setFilterExpression( const QString &filterExp )
Expand All @@ -241,7 +234,7 @@ QgsLegendSymbolList QgsRuleBasedRenderer::Rule::legendSymbolItems( int currentLe
QgsLegendSymbolList lst;
if ( currentLevel != -1 ) // root rule should not be shown
{
lst << QgsLegendSymbolItem( mSymbol, mLabel, mRuleKey, true, mMaximumScale, mMinimumScale, currentLevel, mParent ? mParent->mRuleKey : QString() );
lst << QgsLegendSymbolItem( mSymbol.get(), mLabel, mRuleKey, true, mMaximumScale, mMinimumScale, currentLevel, mParent ? mParent->mRuleKey : QString() );
}

for ( RuleList::const_iterator it = mChildren.constBegin(); it != mChildren.constEnd(); ++it )
Expand Down Expand Up @@ -294,7 +287,7 @@ QDomElement QgsRuleBasedRenderer::Rule::save( QDomDocument &doc, QgsSymbolMap &s
if ( mSymbol )
{
int symbolIndex = symbolMap.size();
symbolMap[QString::number( symbolIndex )] = mSymbol;
symbolMap[QString::number( symbolIndex )] = mSymbol.get();
ruleElem.setAttribute( QStringLiteral( "symbol" ), symbolIndex );
}
if ( !mFilterExp.isEmpty() )
Expand Down Expand Up @@ -508,7 +501,7 @@ QgsRuleBasedRenderer::Rule::RenderResult QgsRuleBasedRenderer::Rule::renderFeatu
Q_FOREACH ( int normZLevel, mSymbolNormZLevels )
{
//QgsDebugMsg(QString("add job at level %1").arg(normZLevel));
renderQueue[normZLevel].jobs.append( new RenderJob( featToRender, mSymbol ) );
renderQueue[normZLevel].jobs.append( new RenderJob( featToRender, mSymbol.get() ) );
rendered = true;
}
}
Expand Down Expand Up @@ -565,7 +558,7 @@ QgsSymbolList QgsRuleBasedRenderer::Rule::symbolsForFeature( QgsFeature &feat, Q
if ( !isFilterOK( feat, context ) )
return lst;
if ( mSymbol )
lst.append( mSymbol );
lst.append( mSymbol.get() );

Q_FOREACH ( Rule *rule, mActiveChildren )
{
Expand Down
49 changes: 31 additions & 18 deletions src/core/symbology/qgsrulebasedrenderer.h
Expand Up @@ -45,7 +45,10 @@ class CORE_EXPORT QgsRuleBasedRenderer : public QgsFeatureRenderer
FeatDrawMarkers = 2
};

// feature for rendering: QgsFeature and some flags
/**
* Feature for rendering by a QgsRuleBasedRenderer. Contains a QgsFeature and some flags.
* \ingroup core
*/
struct FeatureToRender
{
FeatureToRender( QgsFeature &_f, int _flags )
Expand All @@ -56,25 +59,35 @@ class CORE_EXPORT QgsRuleBasedRenderer : public QgsFeatureRenderer
int flags; // selected and/or draw markers
};

// rendering job: a feature to be rendered with a particular symbol
// (both f, symbol are _not_ owned by this class)
/**
* A QgsRuleBasedRenderer rendering job, consisting of a feature to be rendered with a particular symbol.
* \ingroup core
*/
struct RenderJob
{
RenderJob( QgsRuleBasedRenderer::FeatureToRender &_ftr, QgsSymbol *_s )
: ftr( _ftr )
, symbol( _s )
{}

//! Feature to render
QgsRuleBasedRenderer::FeatureToRender &ftr;

//! Symbol to render feature with (not owned by this object).
QgsSymbol *symbol = nullptr;
};

// render level: a list of jobs to be drawn at particular level
// (jobs are owned by this class)
/**
* Render level: a list of jobs to be drawn at particular level for a QgsRuleBasedRenderer.
* \ingroup core
*/
struct RenderLevel
{
explicit RenderLevel( int z ): zIndex( z ) {}
~RenderLevel() { Q_FOREACH ( RenderJob *j, jobs ) delete j; }
int zIndex;

//! List of jobs to render, owned by this object.
QList<QgsRuleBasedRenderer::RenderJob *> jobs;

QgsRuleBasedRenderer::RenderLevel &operator=( const QgsRuleBasedRenderer::RenderLevel &rh )
Expand All @@ -100,7 +113,7 @@ class CORE_EXPORT QgsRuleBasedRenderer : public QgsFeatureRenderer

};

// rendering queue: a list of rendering levels
//! Rendering queue: a list of rendering levels
typedef QList<QgsRuleBasedRenderer::RenderLevel> RenderQueue;

class Rule;
Expand Down Expand Up @@ -178,7 +191,7 @@ class CORE_EXPORT QgsRuleBasedRenderer : public QgsFeatureRenderer
*/
bool isScaleOK( double scale ) const;

QgsSymbol *symbol() { return mSymbol; }
QgsSymbol *symbol() { return mSymbol.get(); }
QString label() const { return mLabel; }
bool dependsOnScale() const { return mMaximumScale != 0 || mMinimumScale != 0; }

Expand Down Expand Up @@ -206,7 +219,7 @@ class CORE_EXPORT QgsRuleBasedRenderer : public QgsFeatureRenderer
* A filter that will check if this rule applies
* \returns An expression
*/
QgsExpression *filter() const { return mFilter; }
QgsExpression *filter() const { return mFilter.get(); }

/**
* A filter that will check if this rule applies
Expand Down Expand Up @@ -411,29 +424,29 @@ class CORE_EXPORT QgsRuleBasedRenderer : public QgsFeatureRenderer
protected:
void initFilter();

Rule *mParent; // parent rule (NULL only for root rule)
QgsSymbol *mSymbol = nullptr;
private:
#ifdef SIP_RUN
Rule( const QgsRuleBasedRenderer::Rule &rh );
#endif

Rule *mParent = nullptr; // parent rule (NULL only for root rule)
std::unique_ptr< QgsSymbol > mSymbol;
double mMaximumScale = 0;
double mMinimumScale = 0;
QString mFilterExp, mLabel, mDescription;
bool mElseRule;
bool mElseRule = false;
RuleList mChildren;
RuleList mElseRules;
bool mIsActive; // whether it is enabled or not
bool mIsActive = true; // whether it is enabled or not

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

// temporary
QgsExpression *mFilter = nullptr;
std::unique_ptr< QgsExpression > mFilter;
// temporary while rendering
QSet<int> mSymbolNormZLevels;
RuleList mActiveChildren;

private:
#ifdef SIP_RUN
Rule( const QgsRuleBasedRenderer::Rule &rh );
#endif

/**
* Check which child rules are else rules and update the internal list of else rules
*
Expand Down
16 changes: 7 additions & 9 deletions tests/src/python/test_qgsrulebasedrenderer.py
Expand Up @@ -30,6 +30,7 @@
from qgis.PyQt.QtCore import QSize

from qgis.core import (QgsVectorLayer,
QgsMapSettings,
QgsProject,
QgsRectangle,
QgsMultiRenderChecker,
Expand All @@ -42,7 +43,6 @@
QgsRendererRange
)
from qgis.testing import start_app, unittest
from qgis.testing.mocked import get_iface
from utilities import unitTestDataPath

# Convenience instances in case you may need them
Expand All @@ -54,7 +54,6 @@
class TestQgsRulebasedRenderer(unittest.TestCase):

def setUp(self):
self.iface = get_iface()
myShpFile = os.path.join(TEST_DATA_DIR, 'rectangles.shp')
layer = QgsVectorLayer(myShpFile, 'Points', 'ogr')
QgsProject.instance().addMapLayer(layer)
Expand All @@ -68,14 +67,13 @@ def setUp(self):
self.r2 = QgsRuleBasedRenderer.Rule(sym2, 0, 0, '"id" = 2')
self.r3 = QgsRuleBasedRenderer.Rule(sym3, 0, 0, 'ELSE')

self.rootrule = QgsRuleBasedRenderer.Rule(None)
self.rootrule.appendChild(self.r1)
self.rootrule.appendChild(self.r2)
self.rootrule.appendChild(self.r3)
rootrule = QgsRuleBasedRenderer.Rule(None)
rootrule.appendChild(self.r1)
rootrule.appendChild(self.r2)
rootrule.appendChild(self.r3)

self.renderer = QgsRuleBasedRenderer(self.rootrule)
layer.setRenderer(self.renderer)
self.mapsettings = self.iface.mapCanvas().mapSettings()
layer.setRenderer(QgsRuleBasedRenderer(rootrule))
self.mapsettings = QgsMapSettings()
self.mapsettings.setOutputSize(QSize(400, 400))
self.mapsettings.setOutputDpi(96)
self.mapsettings.setExtent(QgsRectangle(-163, 22, -70, 52))
Expand Down

0 comments on commit 7c0665a

Please sign in to comment.