From 93db66ed1f5c0dc66db8de840667fa795e40b2f6 Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Tue, 10 Jan 2017 13:33:17 +1000 Subject: [PATCH] Allow properties/collections to be prepared in advance --- python/core/qgsproperty.sip | 9 +++++++ python/core/qgspropertycollection.sip | 10 +++++++ src/core/qgsdiagramrenderer.cpp | 5 ++++ src/core/qgsdiagramrenderer.h | 8 ++++++ src/core/qgsproperty.cpp | 31 ++++++++++++++++++++-- src/core/qgsproperty.h | 21 ++++++++++++--- src/core/qgspropertycollection.cpp | 24 +++++++++++++++++ src/core/qgspropertycollection.h | 12 ++++++++- src/core/qgsvectorlayerdiagramprovider.cpp | 4 ++- tests/src/core/testqgsproperty.cpp | 18 +++++++++++++ 10 files changed, 135 insertions(+), 7 deletions(-) diff --git a/python/core/qgsproperty.sip b/python/core/qgsproperty.sip index 17ca1663b8d..9b61323a915 100644 --- a/python/core/qgsproperty.sip +++ b/python/core/qgsproperty.sip @@ -69,6 +69,13 @@ class QgsAbstractProperty */ void setActive( bool active ); + /** + * Prepares the property against a specified expression context. Calling prepare before evaluating the + * property multiple times allows precalculation of expensive setup tasks such as parsing expressions. + * Returns true if preparation was successful. + */ + virtual bool prepare( const QgsExpressionContext& context = QgsExpressionContext() ) const; + /** Returns the set of any fields referenced by the property. * @param context expression context the property will be evaluated against. */ @@ -237,6 +244,7 @@ class QgsFieldBasedProperty : QgsAbstractProperty */ QString field() const; + bool prepare( const QgsExpressionContext& context = QgsExpressionContext() ) const; virtual QSet< QString > referencedFields( const QgsExpressionContext& context = QgsExpressionContext() ) const; bool writeXml( QDomElement& propertyElem, QDomDocument& doc ) const; @@ -283,6 +291,7 @@ class QgsExpressionBasedProperty : QgsAbstractProperty */ QString expressionString() const; + bool prepare( const QgsExpressionContext& context = QgsExpressionContext() ) const; virtual QSet< QString > referencedFields( const QgsExpressionContext& context = QgsExpressionContext() ) const; bool writeXml( QDomElement& propertyElem, QDomDocument& doc ) const; diff --git a/python/core/qgspropertycollection.sip b/python/core/qgspropertycollection.sip index d20e8836547..9b47dd81ca1 100644 --- a/python/core/qgspropertycollection.sip +++ b/python/core/qgspropertycollection.sip @@ -126,6 +126,13 @@ class QgsAbstractPropertyCollection */ int valueAsInt( int key, const QgsExpressionContext& context, int defaultValue = 0 ) const; + /** + * Prepares the collection against a specified expression context. Calling prepare before evaluating the + * collection's properties multiple times allows precalculation of expensive setup tasks such as parsing expressions. + * Returns true if preparation was successful. + */ + virtual bool prepare( const QgsExpressionContext& context = QgsExpressionContext() ) const = 0; + /** * Returns the set of any fields referenced by the active properties from the collection. * @param context expression context the properties will be evaluated against. @@ -215,6 +222,7 @@ class QgsPropertyCollection : QgsAbstractPropertyCollection bool hasProperty( int key ) const; QgsAbstractProperty* property( int key ); QVariant value( int key, const QgsExpressionContext& context, const QVariant& defaultValue = QVariant() ) const; + bool prepare( const QgsExpressionContext& context = QgsExpressionContext() ) const; QSet< QString > referencedFields( const QgsExpressionContext& context = QgsExpressionContext() ) const; bool isActive( int key ) const; bool hasActiveProperties() const; @@ -336,6 +344,8 @@ class QgsPropertyCollectionStack : QgsAbstractPropertyCollection */ QVariant value( int key, const QgsExpressionContext& context, const QVariant& defaultValue = QVariant() ) const; + bool prepare( const QgsExpressionContext& context = QgsExpressionContext() ) const; + /** Returns the set of any fields referenced by the active properties from the stack. * @param context expression context the properties will be evaluated against. */ diff --git a/src/core/qgsdiagramrenderer.cpp b/src/core/qgsdiagramrenderer.cpp index 50723497d3f..36a0008a917 100644 --- a/src/core/qgsdiagramrenderer.cpp +++ b/src/core/qgsdiagramrenderer.cpp @@ -143,6 +143,11 @@ void QgsDiagramLayerSettings::writeXml( QDomElement& layerElem, QDomDocument& do layerElem.appendChild( diagramLayerElem ); } +bool QgsDiagramLayerSettings::prepare( const QgsExpressionContext& context ) const +{ + return mProperties.prepare( context ); +} + void QgsDiagramLayerSettings::init() { if ( sPropertyNameMap.isEmpty() ) diff --git a/src/core/qgsdiagramrenderer.h b/src/core/qgsdiagramrenderer.h index 167ed8350ee..4f701008947 100644 --- a/src/core/qgsdiagramrenderer.h +++ b/src/core/qgsdiagramrenderer.h @@ -241,6 +241,14 @@ class CORE_EXPORT QgsDiagramLayerSettings void readXml( const QDomElement& elem, const QgsVectorLayer* layer ); void writeXml( QDomElement& layerElem, QDomDocument& doc, const QgsVectorLayer* layer ) const; + /** + * Prepares the diagrams for a specified expression context. Calling prepare before rendering + * multiple diagrams allows precalculation of expensive setup tasks such as parsing expressions. + * Returns true if preparation was successful. + * @note added in QGIS 3.0 + */ + bool prepare( const QgsExpressionContext& context = QgsExpressionContext() ) const; + /** Returns the set of any fields referenced by the layer's diagrams. * @param context expression context the diagrams will be drawn using * @note added in QGIS 2.16 diff --git a/src/core/qgsproperty.cpp b/src/core/qgsproperty.cpp index 3a272763e9c..42e69fc8cde 100644 --- a/src/core/qgsproperty.cpp +++ b/src/core/qgsproperty.cpp @@ -233,8 +233,21 @@ bool QgsStaticProperty::readXml( const QDomElement &propertyElem, const QDomDocu QgsFieldBasedProperty::QgsFieldBasedProperty( const QString& field, bool isActive ) : QgsAbstractProperty( isActive ) , mField( field ) -{ +{} +QgsFieldBasedProperty::QgsFieldBasedProperty( const QgsFieldBasedProperty& other ) + : QgsAbstractProperty( other ) + , mField( other.mField ) + // don't copy cached field index! +{} + +QgsFieldBasedProperty& QgsFieldBasedProperty::operator=( const QgsFieldBasedProperty & other ) +{ + QgsAbstractProperty::operator=( other ); + mActive = other.mActive; + mField = other.mField; + mCachedFieldIdx = -1; + return *this; } QgsFieldBasedProperty* QgsFieldBasedProperty::clone() @@ -242,6 +255,17 @@ QgsFieldBasedProperty* QgsFieldBasedProperty::clone() return new QgsFieldBasedProperty( *this ); } +bool QgsFieldBasedProperty::prepare( const QgsExpressionContext& context ) const +{ + if ( !mActive ) + return true; + + // cache field index to avoid subsequent lookups + QgsFields f = context.fields(); + mCachedFieldIdx = f.lookupField( mField ); + return true; +} + QVariant QgsFieldBasedProperty::propertyValue( const QgsExpressionContext& context, const QVariant& defaultValue ) const { if ( !mActive ) @@ -251,6 +275,10 @@ QVariant QgsFieldBasedProperty::propertyValue( const QgsExpressionContext& conte if ( !f.isValid() ) return defaultValue; + //shortcut the field lookup + if ( mCachedFieldIdx >= 0 ) + return f.attribute( mCachedFieldIdx ); + int fieldIdx = f.fieldNameIndex( mField ); if ( fieldIdx < 0 ) return defaultValue; @@ -296,7 +324,6 @@ bool QgsFieldBasedProperty::readXml( const QDomElement& propertyElem, const QDom QgsExpressionBasedProperty::QgsExpressionBasedProperty( const QString& expression, bool isActive ) : QgsAbstractProperty( isActive ) , mExpressionString( expression ) - , mPrepared( false ) , mExpression( expression ) { diff --git a/src/core/qgsproperty.h b/src/core/qgsproperty.h index 62bc74c0b23..f08b7c9844c 100644 --- a/src/core/qgsproperty.h +++ b/src/core/qgsproperty.h @@ -93,6 +93,13 @@ class CORE_EXPORT QgsAbstractProperty */ void setActive( bool active ) { mActive = active; } + /** + * Prepares the property against a specified expression context. Calling prepare before evaluating the + * property multiple times allows precalculation of expensive setup tasks such as parsing expressions. + * Returns true if preparation was successful. + */ + virtual bool prepare( const QgsExpressionContext& context = QgsExpressionContext() ) const { Q_UNUSED( context ); return true; } + /** * Returns the set of any fields referenced by the property. * @param context expression context the property will be evaluated against. @@ -262,6 +269,13 @@ class CORE_EXPORT QgsFieldBasedProperty : public QgsAbstractProperty */ QgsFieldBasedProperty( const QString& field = QString(), bool isActive = false ); + /** + * Copy constructor + */ + QgsFieldBasedProperty( const QgsFieldBasedProperty& other ); + + QgsFieldBasedProperty& operator=( const QgsFieldBasedProperty& other ); + virtual Type propertyType() const override { return FieldBasedProperty; } virtual QgsFieldBasedProperty* clone() override; @@ -279,6 +293,7 @@ class CORE_EXPORT QgsFieldBasedProperty : public QgsAbstractProperty */ QString field() const { return mField; } + bool prepare( const QgsExpressionContext& context = QgsExpressionContext() ) const override; virtual QSet< QString > referencedFields( const QgsExpressionContext& context = QgsExpressionContext() ) const override; bool writeXml( QDomElement& propertyElem, QDomDocument& doc ) const override; bool readXml( const QDomElement& propertyElem, const QDomDocument& doc ) override; @@ -290,6 +305,7 @@ class CORE_EXPORT QgsFieldBasedProperty : public QgsAbstractProperty private: QString mField; + mutable int mCachedFieldIdx = -1; }; @@ -328,6 +344,7 @@ class CORE_EXPORT QgsExpressionBasedProperty : public QgsAbstractProperty */ QString expressionString() const { return mExpressionString; } + bool prepare( const QgsExpressionContext& context = QgsExpressionContext() ) const override; virtual QSet< QString > referencedFields( const QgsExpressionContext& context = QgsExpressionContext() ) const override; bool writeXml( QDomElement& propertyElem, QDomDocument& doc ) const override; bool readXml( const QDomElement& propertyElem, const QDomDocument& doc ) override; @@ -339,13 +356,11 @@ class CORE_EXPORT QgsExpressionBasedProperty : public QgsAbstractProperty private: QString mExpressionString; - mutable bool mPrepared; + mutable bool mPrepared = false; mutable QgsExpression mExpression; //! Cached set of referenced columns mutable QSet< QString > mReferencedCols; - bool prepare( const QgsExpressionContext& context ) const; - }; /** diff --git a/src/core/qgspropertycollection.cpp b/src/core/qgspropertycollection.cpp index e28abbe9af0..537d1e9f666 100644 --- a/src/core/qgspropertycollection.cpp +++ b/src/core/qgspropertycollection.cpp @@ -171,6 +171,20 @@ QVariant QgsPropertyCollection::value( int key, const QgsExpressionContext& cont return prop->value( context, defaultValue ); } +bool QgsPropertyCollection::prepare( const QgsExpressionContext& context ) const +{ + bool result = true; + QHash::const_iterator it = mProperties.constBegin(); + for ( ; it != mProperties.constEnd(); ++it ) + { + if ( !it.value()->isActive() ) + continue; + + result = result && it.value()->prepare( context ); + } + return result; +} + QSet< QString > QgsPropertyCollection::referencedFields( const QgsExpressionContext &context ) const { QSet< QString > cols; @@ -439,6 +453,16 @@ QSet< QString > QgsPropertyCollectionStack::referencedFields( const QgsExpressio return cols; } +bool QgsPropertyCollectionStack::prepare( const QgsExpressionContext& context ) const +{ + bool result = true; + Q_FOREACH ( QgsPropertyCollection* collection, mStack ) + { + result = result && collection->prepare( context ); + } + return result; +} + QSet QgsPropertyCollectionStack::propertyKeys() const { QSet keys; diff --git a/src/core/qgspropertycollection.h b/src/core/qgspropertycollection.h index 2d3ff09e227..5f7b2eb556d 100644 --- a/src/core/qgspropertycollection.h +++ b/src/core/qgspropertycollection.h @@ -38,6 +38,8 @@ class CORE_EXPORT QgsAbstractPropertyCollection QgsAbstractPropertyCollection( const QString& name = QString() ); + virtual ~QgsAbstractPropertyCollection() = default; + /** * Returns the name of the property collection. * @see setName() @@ -100,7 +102,6 @@ class CORE_EXPORT QgsAbstractPropertyCollection */ virtual QVariant value( int key, const QgsExpressionContext& context, const QVariant& defaultValue = QVariant() ) const = 0; - /** * Calculates the current value of the property with the specified key and interprets it as a color. * @param key integer key for property to return. The intended use case is that a context specific enum is cast to @@ -140,6 +141,13 @@ class CORE_EXPORT QgsAbstractPropertyCollection */ int valueAsInt( int key, const QgsExpressionContext& context, int defaultValue = 0 ) const; + /** + * Prepares the collection against a specified expression context. Calling prepare before evaluating the + * collection's properties multiple times allows precalculation of expensive setup tasks such as parsing expressions. + * Returns true if preparation was successful. + */ + virtual bool prepare( const QgsExpressionContext& context = QgsExpressionContext() ) const = 0; + /** * Returns the set of any fields referenced by the active properties from the collection. * @param context expression context the properties will be evaluated against. @@ -231,6 +239,7 @@ class CORE_EXPORT QgsPropertyCollection : public QgsAbstractPropertyCollection QgsAbstractProperty* property( int key ) override; const QgsAbstractProperty* property( int key ) const override; QVariant value( int key, const QgsExpressionContext& context, const QVariant& defaultValue = QVariant() ) const override; + virtual bool prepare( const QgsExpressionContext& context = QgsExpressionContext() ) const override; QSet< QString > referencedFields( const QgsExpressionContext& context = QgsExpressionContext() ) const override; bool isActive( int key ) const override; bool hasActiveProperties() const override; @@ -386,6 +395,7 @@ class CORE_EXPORT QgsPropertyCollectionStack : public QgsAbstractPropertyCollect * @param context expression context the properties will be evaluated against. */ QSet< QString > referencedFields( const QgsExpressionContext& context = QgsExpressionContext() ) const override; + virtual bool prepare( const QgsExpressionContext& context = QgsExpressionContext() ) const override; QSet propertyKeys() const override; bool hasProperty( int key ) const override; diff --git a/src/core/qgsvectorlayerdiagramprovider.cpp b/src/core/qgsvectorlayerdiagramprovider.cpp index f98cf8ac997..d87e0f3d547 100644 --- a/src/core/qgsvectorlayerdiagramprovider.cpp +++ b/src/core/qgsvectorlayerdiagramprovider.cpp @@ -178,10 +178,12 @@ bool QgsVectorLayerDiagramProvider::prepare( const QgsRenderContext& context, QS s2.setRenderer( mDiagRenderer ); + bool result = s2.prepare( context.expressionContext() ); + //add attributes needed by the diagram renderer attributeNames.unite( s2.referencedFields( context.expressionContext() ) ); - return true; + return result; } diff --git a/tests/src/core/testqgsproperty.cpp b/tests/src/core/testqgsproperty.cpp index 8fa0fbeaf50..1eb153ac3db 100644 --- a/tests/src/core/testqgsproperty.cpp +++ b/tests/src/core/testqgsproperty.cpp @@ -318,6 +318,11 @@ void TestQgsProperty::fieldBasedProperty() QCOMPARE( defaultProperty.value( context, -1 ).toInt(), -1 ); QVERIFY( defaultProperty.referencedFields( context ).isEmpty() ); + //test preparation + QgsFieldBasedProperty property3( QString( "field1" ), true ); + QVERIFY( property3.prepare( context ) ); + QCOMPARE( property3.value( context, -1 ).toInt(), 5 ); + //saving and restoring //create a test dom element @@ -411,6 +416,13 @@ void TestQgsProperty::expressionBasedProperty() QCOMPARE( defaultProperty.value( context, -1 ).toInt(), -1 ); QVERIFY( defaultProperty.referencedFields( context ).isEmpty() ); + //preparation + QgsExpressionBasedProperty property3( QString( "\"field1\" + \"field2\"" ), true ); + QVERIFY( property3.prepare( context ) ); + QCOMPARE( property3.value( context, -1 ).toInt(), 12 ); + QgsExpressionBasedProperty property4( QString( "\"field1\" + " ), true ); + QVERIFY( !property4.prepare( context ) ); + //saving and restoring //create a test dom element @@ -776,6 +788,9 @@ void TestQgsProperty::propertyCollection() QVERIFY( collection.hasActiveProperties() ); QVERIFY( !collection.hasActiveDynamicProperties() ); + //preparation + QVERIFY( collection.prepare( context ) ); + //test bad property QVERIFY( !collection.property( Property2 ) ); QVERIFY( !collection.value( Property2, context ).isValid() ); @@ -995,6 +1010,9 @@ void TestQgsProperty::collectionStack() QVERIFY( !stack.hasActiveDynamicProperties() ); QVERIFY( stack.hasActiveProperties() ); + //preparation + QVERIFY( stack.prepare( context ) ); + //test adding active property later in the stack QgsStaticProperty* property3 = new QgsStaticProperty( "value3", true ); collection2->setProperty( Property1, property3 );