Skip to content

Commit abc6129

Browse files
nyalldawsonnyalldawson
committedFeb 22, 2017
Optimise expression context storage/retrieval of features
Shaves ~10% rendering time off a 1 million point layer
1 parent bde4ff9 commit abc6129

File tree

6 files changed

+71
-33
lines changed

6 files changed

+71
-33
lines changed
 

‎doc/api_break.dox

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -992,6 +992,12 @@ QgsExpressionItem {#qgis_api_break_3_0_QgsExpressionItem}
992992
- CustomSortRole was renamed to CUSTOM_SORT_ROLE
993993
- ItemTypeRole was renamed to ITEM_TYPE_ROLE
994994

995+
QgsExpressionContext {#qgis_api_break_3_0_QgsExpressionContext}
996+
--------------------
997+
998+
- EXPR_FEATURE was removed. Use the direct feature manipulation methods feature(), hasFeature() and setFeature() instead
999+
1000+
9951001
QgsExpressionContextUtils {#qgis_api_break_3_0_QgsExpressionContextUtils}
9961002
-------------------------
9971003

‎python/core/qgsexpressioncontext.sip

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -189,17 +189,12 @@ class QgsExpressionContextScope
189189
*/
190190
QStringList functionNames() const;
191191

192-
/** Adds a function to the scope.
193-
* @param name function name
194-
* @param function function to insert. Ownership is transferred to the scope.
195-
* @see addVariable()
196-
*/
197192
void addFunction( const QString& name, QgsScopedExpressionFunction* function /Transfer/ );
198193

199-
/** Convenience function for setting a feature for the scope. Any existing
200-
* feature set by the scope will be overwritten.
201-
* @param feature feature for scope
202-
*/
194+
bool hasFeature() const;
195+
196+
QgsFeature feature() const;
197+
203198
void setFeature( const QgsFeature& feature );
204199

205200
/** Convenience function for setting a fields for the scope. Any existing
@@ -414,9 +409,7 @@ class QgsExpressionContext
414409
*/
415410
void setFeature( const QgsFeature& feature );
416411

417-
/** Convenience function for retrieving the feature for the context, if set.
418-
* @see setFeature
419-
*/
412+
bool hasFeature() const;
420413
QgsFeature feature() const;
421414

422415
/** Convenience function for setting a fields for the context. The fields
@@ -481,8 +474,6 @@ class QgsExpressionContext
481474

482475
//! Inbuilt variable name for fields storage
483476
static const QString EXPR_FIELDS;
484-
//! Inbuilt variable name for feature storage
485-
static const QString EXPR_FEATURE;
486477
//! Inbuilt variable name for value original value variable
487478
static const QString EXPR_ORIGINAL_VALUE;
488479
//! Inbuilt variable name for symbol color variable

‎src/core/qgsexpression.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -318,8 +318,8 @@ static QgsFeature getFeature( const QVariant& value, QgsExpression* parent )
318318
return 0;
319319
}
320320

321-
#define FEAT_FROM_CONTEXT(c, f) if (!(c) || !(c)->hasVariable(QgsExpressionContext::EXPR_FEATURE)) return QVariant(); \
322-
QgsFeature f = qvariant_cast<QgsFeature>( (c)->variable( QgsExpressionContext::EXPR_FEATURE ) );
321+
#define FEAT_FROM_CONTEXT(c, f) if (!(c) || !(c)->hasFeature() ) return QVariant(); \
322+
QgsFeature f = ( c )->feature();
323323

324324
static QgsExpression::Node* getNode( const QVariant& value, QgsExpression* parent )
325325
{
@@ -5495,7 +5495,7 @@ QVariant QgsExpression::NodeColumnRef::eval( QgsExpression *parent, const QgsExp
54955495
}
54965496
}
54975497

5498-
if ( context && context->hasVariable( QgsExpressionContext::EXPR_FEATURE ) )
5498+
if ( context && context->hasFeature() )
54995499
{
55005500
QgsFeature feature = context->feature();
55015501
if ( index >= 0 )

‎src/core/qgsexpressioncontext.cpp

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@
3434

3535

3636
const QString QgsExpressionContext::EXPR_FIELDS( QStringLiteral( "_fields_" ) );
37-
const QString QgsExpressionContext::EXPR_FEATURE( QStringLiteral( "_feature_" ) );
3837
const QString QgsExpressionContext::EXPR_ORIGINAL_VALUE( QStringLiteral( "value" ) );
3938
const QString QgsExpressionContext::EXPR_SYMBOL_COLOR( QStringLiteral( "symbol_color" ) );
4039
const QString QgsExpressionContext::EXPR_SYMBOL_ANGLE( QStringLiteral( "symbol_angle" ) );
@@ -58,6 +57,8 @@ QgsExpressionContextScope::QgsExpressionContextScope( const QString& name )
5857
QgsExpressionContextScope::QgsExpressionContextScope( const QgsExpressionContextScope& other )
5958
: mName( other.mName )
6059
, mVariables( other.mVariables )
60+
, mHasFeature( other.mHasFeature )
61+
, mFeature( other.mFeature )
6162
{
6263
QHash<QString, QgsScopedExpressionFunction* >::const_iterator it = other.mFunctions.constBegin();
6364
for ( ; it != other.mFunctions.constEnd(); ++it )
@@ -70,6 +71,8 @@ QgsExpressionContextScope& QgsExpressionContextScope::operator=( const QgsExpres
7071
{
7172
mName = other.mName;
7273
mVariables = other.mVariables;
74+
mHasFeature = other.mHasFeature;
75+
mFeature = other.mFeature;
7376

7477
qDeleteAll( mFunctions );
7578
mFunctions.clear();
@@ -196,10 +199,6 @@ void QgsExpressionContextScope::addFunction( const QString& name, QgsScopedExpre
196199
mFunctions.insert( name, function );
197200
}
198201

199-
void QgsExpressionContextScope::setFeature( const QgsFeature &feature )
200-
{
201-
addVariable( StaticVariable( QgsExpressionContext::EXPR_FEATURE, QVariant::fromValue( feature ), true ) );
202-
}
203202

204203
void QgsExpressionContextScope::setFields( const QgsFields &fields )
205204
{
@@ -468,9 +467,27 @@ void QgsExpressionContext::setFeature( const QgsFeature &feature )
468467
mStack.last()->setFeature( feature );
469468
}
470469

470+
bool QgsExpressionContext::hasFeature() const
471+
{
472+
Q_FOREACH ( const QgsExpressionContextScope* scope, mStack )
473+
{
474+
if ( scope->hasFeature() )
475+
return true;
476+
}
477+
return false;
478+
}
479+
471480
QgsFeature QgsExpressionContext::feature() const
472481
{
473-
return qvariant_cast<QgsFeature>( variable( QgsExpressionContext::EXPR_FEATURE ) );
482+
//iterate through stack backwards, so that higher priority variables take precedence
483+
QList< QgsExpressionContextScope* >::const_iterator it = mStack.constEnd();
484+
while ( it != mStack.constBegin() )
485+
{
486+
--it;
487+
if (( *it )->hasFeature() )
488+
return ( *it )->feature();
489+
}
490+
return QgsFeature();
474491
}
475492

476493
void QgsExpressionContext::setFields( const QgsFields &fields )

‎src/core/qgsexpressioncontext.h

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include <QString>
2222
#include <QStringList>
2323
#include <QSet>
24+
#include "qgsfeature.h"
2425
#include "qgsexpression.h"
2526

2627
class QgsExpression;
@@ -250,11 +251,26 @@ class CORE_EXPORT QgsExpressionContextScope
250251
*/
251252
void addFunction( const QString& name, QgsScopedExpressionFunction* function );
252253

254+
/**
255+
* Returns true if the scope has a feature associated with it.
256+
* @note added in QGIS 3.0
257+
* @see feature()
258+
*/
259+
bool hasFeature() const { return mHasFeature; }
260+
261+
/**
262+
* Sets the feature associated with the scope.
263+
* @see setFeature()
264+
* @see hasFeature()
265+
* @note added in QGIS 3.0
266+
*/
267+
QgsFeature feature() const { return mFeature; }
268+
253269
/** Convenience function for setting a feature for the scope. Any existing
254270
* feature set by the scope will be overwritten.
255271
* @param feature feature for scope
256272
*/
257-
void setFeature( const QgsFeature& feature );
273+
void setFeature( const QgsFeature& feature ) { mHasFeature = true; mFeature = feature; }
258274

259275
/** Convenience function for setting a fields for the scope. Any existing
260276
* fields set by the scope will be overwritten.
@@ -266,6 +282,8 @@ class CORE_EXPORT QgsExpressionContextScope
266282
QString mName;
267283
QHash<QString, StaticVariable> mVariables;
268284
QHash<QString, QgsScopedExpressionFunction* > mFunctions;
285+
bool mHasFeature = false;
286+
QgsFeature mFeature;
269287

270288
bool variableNameSort( const QString &a, const QString &b );
271289
};
@@ -474,6 +492,13 @@ class CORE_EXPORT QgsExpressionContext
474492
*/
475493
void setFeature( const QgsFeature& feature );
476494

495+
/**
496+
* Returns true if the context has a feature associated with it.
497+
* @note added in QGIS 3.0
498+
* @see feature()
499+
*/
500+
bool hasFeature() const;
501+
477502
/** Convenience function for retrieving the feature for the context, if set.
478503
* @see setFeature
479504
*/
@@ -541,8 +566,6 @@ class CORE_EXPORT QgsExpressionContext
541566

542567
//! Inbuilt variable name for fields storage
543568
static const QString EXPR_FIELDS;
544-
//! Inbuilt variable name for feature storage
545-
static const QString EXPR_FEATURE;
546569
//! Inbuilt variable name for value original value variable
547570
static const QString EXPR_ORIGINAL_VALUE;
548571
//! Inbuilt variable name for symbol color variable

‎tests/src/core/testqgsexpressioncontext.cpp

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -432,27 +432,28 @@ void TestQgsExpressionContext::evaluate()
432432
void TestQgsExpressionContext::setFeature()
433433
{
434434
QgsFeature feature( 50LL );
435+
feature.setValid( true );
435436
QgsExpressionContextScope scope;
436437
scope.setFeature( feature );
437-
QVERIFY( scope.hasVariable( QgsExpressionContext::EXPR_FEATURE ) );
438-
QCOMPARE(( qvariant_cast<QgsFeature>( scope.variable( QgsExpressionContext::EXPR_FEATURE ) ) ).id(), 50LL );
438+
QVERIFY( scope.hasFeature() );
439+
QCOMPARE( scope.feature().id(), 50LL );
439440

440441
//test setting a feature in a context with no scopes
441442
QgsExpressionContext emptyContext;
442443
QVERIFY( !emptyContext.feature().isValid() );
443444
emptyContext.setFeature( feature );
444445
//setFeature should have created a scope
445446
QCOMPARE( emptyContext.scopeCount(), 1 );
446-
QVERIFY( emptyContext.hasVariable( QgsExpressionContext::EXPR_FEATURE ) );
447-
QCOMPARE(( qvariant_cast<QgsFeature>( emptyContext.variable( QgsExpressionContext::EXPR_FEATURE ) ) ).id(), 50LL );
447+
QVERIFY( emptyContext.feature().isValid() );
448+
QCOMPARE( emptyContext.feature().id(), 50LL );
448449
QCOMPARE( emptyContext.feature().id(), 50LL );
449450

450451
QgsExpressionContext contextWithScope;
451452
contextWithScope << new QgsExpressionContextScope();
452453
contextWithScope.setFeature( feature );
453454
QCOMPARE( contextWithScope.scopeCount(), 1 );
454-
QVERIFY( contextWithScope.hasVariable( QgsExpressionContext::EXPR_FEATURE ) );
455-
QCOMPARE(( qvariant_cast<QgsFeature>( contextWithScope.variable( QgsExpressionContext::EXPR_FEATURE ) ) ).id(), 50LL );
455+
QVERIFY( contextWithScope.feature().isValid() );
456+
QCOMPARE( contextWithScope.feature().id(), 50LL );
456457
QCOMPARE( contextWithScope.feature().id(), 50LL );
457458
}
458459

@@ -652,7 +653,7 @@ void TestQgsExpressionContext::featureBasedContext()
652653

653654
QgsExpressionContext context = QgsExpressionContextUtils::createFeatureBasedContext( f, fields );
654655

655-
QgsFeature evalFeature = qvariant_cast<QgsFeature>( context.variable( QStringLiteral( "_feature_" ) ) );
656+
QgsFeature evalFeature = context.feature();
656657
QgsFields evalFields = qvariant_cast<QgsFields>( context.variable( QStringLiteral( "_fields_" ) ) );
657658
QCOMPARE( evalFeature.attributes(), f.attributes() );
658659
QCOMPARE( evalFields, fields );

0 commit comments

Comments
 (0)