Optimise expression context storage/retrieval of features

Shaves ~10% rendering time off a 1 million point layer
qgis · Feb 22, 2017 · abc6129 · abc6129
1 parent bde4ff9
commit abc6129
Show file tree

Hide file tree

Showing 6 changed files with 71 additions and 33 deletions.
diff --git a/doc/api_break.dox b/doc/api_break.dox
@@ -992,6 +992,12 @@ QgsExpressionItem        {#qgis_api_break_3_0_QgsExpressionItem}
 - CustomSortRole was renamed to CUSTOM_SORT_ROLE
 - ItemTypeRole was renamed to ITEM_TYPE_ROLE
 
+QgsExpressionContext      {#qgis_api_break_3_0_QgsExpressionContext}
+--------------------
+
+- EXPR_FEATURE was removed. Use the direct feature manipulation methods feature(), hasFeature() and setFeature() instead
+
+
 QgsExpressionContextUtils       {#qgis_api_break_3_0_QgsExpressionContextUtils}
 -------------------------
 

diff --git a/python/core/qgsexpressioncontext.sip b/python/core/qgsexpressioncontext.sip
@@ -189,17 +189,12 @@ class QgsExpressionContextScope
      */
     QStringList functionNames() const;
 
-    /** Adds a function to the scope.
-     * @param name function name
-     * @param function function to insert. Ownership is transferred to the scope.
-     * @see addVariable()
-     */
     void addFunction( const QString& name, QgsScopedExpressionFunction* function /Transfer/ );
 
-    /** Convenience function for setting a feature for the scope. Any existing
-     * feature set by the scope will be overwritten.
-     * @param feature feature for scope
-     */
+    bool hasFeature() const;
+
+    QgsFeature feature() const;
+
     void setFeature( const QgsFeature& feature );
 
     /** Convenience function for setting a fields for the scope. Any existing
@@ -414,9 +409,7 @@ class QgsExpressionContext
      */
     void setFeature( const QgsFeature& feature );
 
-    /** Convenience function for retrieving the feature for the context, if set.
-     * @see setFeature
-     */
+    bool hasFeature() const;
     QgsFeature feature() const;
 
     /** Convenience function for setting a fields for the context. The fields
@@ -481,8 +474,6 @@ class QgsExpressionContext
 
     //! Inbuilt variable name for fields storage
     static const QString EXPR_FIELDS;
-    //! Inbuilt variable name for feature storage
-    static const QString EXPR_FEATURE;
     //! Inbuilt variable name for value original value variable
     static const QString EXPR_ORIGINAL_VALUE;
     //! Inbuilt variable name for symbol color variable

diff --git a/src/core/qgsexpression.cpp b/src/core/qgsexpression.cpp
@@ -318,8 +318,8 @@ static QgsFeature getFeature( const QVariant& value, QgsExpression* parent )
   return 0;
 }
 
-#define FEAT_FROM_CONTEXT(c, f) if (!(c) || !(c)->hasVariable(QgsExpressionContext::EXPR_FEATURE)) return QVariant(); \
-  QgsFeature f = qvariant_cast<QgsFeature>( (c)->variable( QgsExpressionContext::EXPR_FEATURE ) );
+#define FEAT_FROM_CONTEXT(c, f) if (!(c) || !(c)->hasFeature() ) return QVariant(); \
+  QgsFeature f = ( c )->feature();
 
 static QgsExpression::Node* getNode( const QVariant& value, QgsExpression* parent )
 {
@@ -5495,7 +5495,7 @@ QVariant QgsExpression::NodeColumnRef::eval( QgsExpression *parent, const QgsExp
     }
   }
 
-  if ( context && context->hasVariable( QgsExpressionContext::EXPR_FEATURE ) )
+  if ( context && context->hasFeature() )
   {
     QgsFeature feature = context->feature();
     if ( index >= 0 )

diff --git a/src/core/qgsexpressioncontext.cpp b/src/core/qgsexpressioncontext.cpp
@@ -34,7 +34,6 @@
 
 
 const QString QgsExpressionContext::EXPR_FIELDS( QStringLiteral( "_fields_" ) );
-const QString QgsExpressionContext::EXPR_FEATURE( QStringLiteral( "_feature_" ) );
 const QString QgsExpressionContext::EXPR_ORIGINAL_VALUE( QStringLiteral( "value" ) );
 const QString QgsExpressionContext::EXPR_SYMBOL_COLOR( QStringLiteral( "symbol_color" ) );
 const QString QgsExpressionContext::EXPR_SYMBOL_ANGLE( QStringLiteral( "symbol_angle" ) );
@@ -58,6 +57,8 @@ QgsExpressionContextScope::QgsExpressionContextScope( const QString& name )
 QgsExpressionContextScope::QgsExpressionContextScope( const QgsExpressionContextScope& other )
     : mName( other.mName )
     , mVariables( other.mVariables )
+    , mHasFeature( other.mHasFeature )
+    , mFeature( other.mFeature )
 {
   QHash<QString, QgsScopedExpressionFunction* >::const_iterator it = other.mFunctions.constBegin();
   for ( ; it != other.mFunctions.constEnd(); ++it )
@@ -70,6 +71,8 @@ QgsExpressionContextScope& QgsExpressionContextScope::operator=( const QgsExpres
 {
   mName = other.mName;
   mVariables = other.mVariables;
+  mHasFeature = other.mHasFeature;
+  mFeature = other.mFeature;
 
   qDeleteAll( mFunctions );
   mFunctions.clear();
@@ -196,10 +199,6 @@ void QgsExpressionContextScope::addFunction( const QString& name, QgsScopedExpre
   mFunctions.insert( name, function );
 }
 
-void QgsExpressionContextScope::setFeature( const QgsFeature &feature )
-{
-  addVariable( StaticVariable( QgsExpressionContext::EXPR_FEATURE, QVariant::fromValue( feature ), true ) );
-}
 
 void QgsExpressionContextScope::setFields( const QgsFields &fields )
 {
@@ -468,9 +467,27 @@ void QgsExpressionContext::setFeature( const QgsFeature &feature )
   mStack.last()->setFeature( feature );
 }
 
+bool QgsExpressionContext::hasFeature() const
+{
+  Q_FOREACH ( const QgsExpressionContextScope* scope, mStack )
+  {
+    if ( scope->hasFeature() )
+      return true;
+  }
+  return false;
+}
+
 QgsFeature QgsExpressionContext::feature() const
 {
-  return qvariant_cast<QgsFeature>( variable( QgsExpressionContext::EXPR_FEATURE ) );
+  //iterate through stack backwards, so that higher priority variables take precedence
+  QList< QgsExpressionContextScope* >::const_iterator it = mStack.constEnd();
+  while ( it != mStack.constBegin() )
+  {
+    --it;
+    if (( *it )->hasFeature() )
+      return ( *it )->feature();
+  }
+  return QgsFeature();
 }
 
 void QgsExpressionContext::setFields( const QgsFields &fields )

diff --git a/src/core/qgsexpressioncontext.h b/src/core/qgsexpressioncontext.h
@@ -21,6 +21,7 @@
 #include <QString>
 #include <QStringList>
 #include <QSet>
+#include "qgsfeature.h"
 #include "qgsexpression.h"
 
 class QgsExpression;
@@ -250,11 +251,26 @@ class CORE_EXPORT QgsExpressionContextScope
      */
     void addFunction( const QString& name, QgsScopedExpressionFunction* function );
 
+    /**
+     * Returns true if the scope has a feature associated with it.
+     * @note added in QGIS 3.0
+     * @see feature()
+     */
+    bool hasFeature() const { return mHasFeature; }
+
+    /**
+     * Sets the feature associated with the scope.
+     * @see setFeature()
+     * @see hasFeature()
+     * @note added in QGIS 3.0
+     */
+    QgsFeature feature() const { return mFeature; }
+
     /** Convenience function for setting a feature for the scope. Any existing
      * feature set by the scope will be overwritten.
      * @param feature feature for scope
      */
-    void setFeature( const QgsFeature& feature );
+    void setFeature( const QgsFeature& feature ) { mHasFeature = true; mFeature = feature; }
 
     /** Convenience function for setting a fields for the scope. Any existing
      * fields set by the scope will be overwritten.
@@ -266,6 +282,8 @@ class CORE_EXPORT QgsExpressionContextScope
     QString mName;
     QHash<QString, StaticVariable> mVariables;
     QHash<QString, QgsScopedExpressionFunction* > mFunctions;
+    bool mHasFeature = false;
+    QgsFeature mFeature;
 
     bool variableNameSort( const QString &a, const QString &b );
 };
@@ -474,6 +492,13 @@ class CORE_EXPORT QgsExpressionContext
      */
     void setFeature( const QgsFeature& feature );
 
+    /**
+     * Returns true if the context has a feature associated with it.
+     * @note added in QGIS 3.0
+     * @see feature()
+     */
+    bool hasFeature() const;
+
     /** Convenience function for retrieving the feature for the context, if set.
      * @see setFeature
      */
@@ -541,8 +566,6 @@ class CORE_EXPORT QgsExpressionContext
 
     //! Inbuilt variable name for fields storage
     static const QString EXPR_FIELDS;
-    //! Inbuilt variable name for feature storage
-    static const QString EXPR_FEATURE;
     //! Inbuilt variable name for value original value variable
     static const QString EXPR_ORIGINAL_VALUE;
     //! Inbuilt variable name for symbol color variable

diff --git a/tests/src/core/testqgsexpressioncontext.cpp b/tests/src/core/testqgsexpressioncontext.cpp
@@ -432,27 +432,28 @@ void TestQgsExpressionContext::evaluate()
 void TestQgsExpressionContext::setFeature()
 {
   QgsFeature feature( 50LL );
+  feature.setValid( true );
   QgsExpressionContextScope scope;
   scope.setFeature( feature );
-  QVERIFY( scope.hasVariable( QgsExpressionContext::EXPR_FEATURE ) );
-  QCOMPARE(( qvariant_cast<QgsFeature>( scope.variable( QgsExpressionContext::EXPR_FEATURE ) ) ).id(), 50LL );
+  QVERIFY( scope.hasFeature() );
+  QCOMPARE( scope.feature().id(), 50LL );
 
   //test setting a feature in a context with no scopes
   QgsExpressionContext emptyContext;
   QVERIFY( !emptyContext.feature().isValid() );
   emptyContext.setFeature( feature );
   //setFeature should have created a scope
   QCOMPARE( emptyContext.scopeCount(), 1 );
-  QVERIFY( emptyContext.hasVariable( QgsExpressionContext::EXPR_FEATURE ) );
-  QCOMPARE(( qvariant_cast<QgsFeature>( emptyContext.variable( QgsExpressionContext::EXPR_FEATURE ) ) ).id(), 50LL );
+  QVERIFY( emptyContext.feature().isValid() );
+  QCOMPARE( emptyContext.feature().id(), 50LL );
   QCOMPARE( emptyContext.feature().id(), 50LL );
 
   QgsExpressionContext contextWithScope;
   contextWithScope << new QgsExpressionContextScope();
   contextWithScope.setFeature( feature );
   QCOMPARE( contextWithScope.scopeCount(), 1 );
-  QVERIFY( contextWithScope.hasVariable( QgsExpressionContext::EXPR_FEATURE ) );
-  QCOMPARE(( qvariant_cast<QgsFeature>( contextWithScope.variable( QgsExpressionContext::EXPR_FEATURE ) ) ).id(), 50LL );
+  QVERIFY( contextWithScope.feature().isValid() );
+  QCOMPARE( contextWithScope.feature().id(), 50LL );
   QCOMPARE( contextWithScope.feature().id(), 50LL );
 }
 
@@ -652,7 +653,7 @@ void TestQgsExpressionContext::featureBasedContext()
 
   QgsExpressionContext context = QgsExpressionContextUtils::createFeatureBasedContext( f, fields );
 
-  QgsFeature evalFeature = qvariant_cast<QgsFeature>( context.variable( QStringLiteral( "_feature_" ) ) );
+  QgsFeature evalFeature = context.feature();
   QgsFields evalFields = qvariant_cast<QgsFields>( context.variable( QStringLiteral( "_fields_" ) ) );
   QCOMPARE( evalFeature.attributes(), f.attributes() );
   QCOMPARE( evalFields, fields );