diff --git a/python/core/auto_generated/labeling/qgspallabeling.sip.in b/python/core/auto_generated/labeling/qgspallabeling.sip.in index cd5fc9f3e52..d15b13daa7a 100644 --- a/python/core/auto_generated/labeling/qgspallabeling.sip.in +++ b/python/core/auto_generated/labeling/qgspallabeling.sip.in @@ -321,7 +321,7 @@ names which the labeling requires for the rendering. .. versionadded:: 3.8 %End - QSet referencedFields() const; + QSet referencedFields( const QgsRenderContext &context ) const; %Docstring Returns all field names referenced by the configuration (e.g. field name or expression, data defined properties). diff --git a/src/core/labeling/qgspallabeling.cpp b/src/core/labeling/qgspallabeling.cpp index 5923848b4c9..2a2b3c052ab 100644 --- a/src/core/labeling/qgspallabeling.cpp +++ b/src/core/labeling/qgspallabeling.cpp @@ -483,7 +483,7 @@ bool QgsPalLayerSettings::prepare( QgsRenderContext &context, QSet &att return true; } -QSet QgsPalLayerSettings::referencedFields() const +QSet QgsPalLayerSettings::referencedFields( const QgsRenderContext &context ) const { QSet referenced; if ( drawLabels ) @@ -498,7 +498,10 @@ QSet QgsPalLayerSettings::referencedFields() const } } - referenced.unite( mDataDefinedProperties.referencedFields( QgsExpressionContext(), true ) ); + // calling referencedFields() with ignoreContext=true because in our expression context + // we do not have valid QgsFields yet - because of that the field names from expressions + // wouldn't get reported + referenced.unite( mDataDefinedProperties.referencedFields( context.expressionContext(), true ) ); if ( geometryGeneratorEnabled ) { @@ -508,8 +511,7 @@ QSet QgsPalLayerSettings::referencedFields() const if ( mCallout ) { - // TODO: this needs further attention - referenced.unite( mCallout->referencedFields( QgsRenderContext() ) ); + referenced.unite( mCallout->referencedFields( context ) ); } return referenced; diff --git a/src/core/labeling/qgspallabeling.h b/src/core/labeling/qgspallabeling.h index 4ff438bfb12..bbce57d21ca 100644 --- a/src/core/labeling/qgspallabeling.h +++ b/src/core/labeling/qgspallabeling.h @@ -479,7 +479,7 @@ class CORE_EXPORT QgsPalLayerSettings * Returns all field names referenced by the configuration (e.g. field name or expression, data defined properties). * \since QGIS 3.14 */ - QSet referencedFields() const; + QSet referencedFields( const QgsRenderContext &context ) const; /** * Prepares the label settings for rendering. diff --git a/src/core/vectortile/qgsvectortilebasiclabeling.cpp b/src/core/vectortile/qgsvectortilebasiclabeling.cpp index 4cf081c9a3b..ba8e333b073 100644 --- a/src/core/vectortile/qgsvectortilebasiclabeling.cpp +++ b/src/core/vectortile/qgsvectortilebasiclabeling.cpp @@ -123,7 +123,7 @@ QgsVectorTileBasicLabelProvider::QgsVectorTileBasicLabelProvider( QgsVectorTileL } } -QMap > QgsVectorTileBasicLabelProvider::usedAttributes( int tileZoom ) const +QMap > QgsVectorTileBasicLabelProvider::usedAttributes( const QgsRenderContext &context, int tileZoom ) const { QMap > requiredFields; for ( const QgsVectorTileBasicLabelingStyle &layerStyle : qgis::as_const( mStyles ) ) @@ -137,7 +137,7 @@ QMap > QgsVectorTileBasicLabelProvider::usedAttributes( i requiredFields[layerStyle.layerName()].unite( expr.referencedColumns() ); } - requiredFields[layerStyle.layerName()].unite( layerStyle.labelSettings().referencedFields() ); + requiredFields[layerStyle.layerName()].unite( layerStyle.labelSettings().referencedFields( context ) ); } return requiredFields; } diff --git a/src/core/vectortile/qgsvectortilebasiclabeling.h b/src/core/vectortile/qgsvectortilebasiclabeling.h index 42ac7ad8c9b..f8a538427a6 100644 --- a/src/core/vectortile/qgsvectortilebasiclabeling.h +++ b/src/core/vectortile/qgsvectortilebasiclabeling.h @@ -142,8 +142,10 @@ class QgsVectorTileBasicLabelProvider : public QgsVectorTileLabelProvider QList subProviders() override; bool prepare( QgsRenderContext &context, QSet &attributeNames ) override; + + // virtual functions from QgsVectorTileLabelProvider void registerTileFeatures( const QgsVectorTileRendererData &tile, QgsRenderContext &context ) override; - QMap > usedAttributes( int tileZoom ) const override; + QMap > usedAttributes( const QgsRenderContext &context, int tileZoom ) const override; void setFields( const QMap> &requiredFields ) override; private: diff --git a/src/core/vectortile/qgsvectortilelabeling.h b/src/core/vectortile/qgsvectortilelabeling.h index 8c5ba993a50..9b7f2467389 100644 --- a/src/core/vectortile/qgsvectortilelabeling.h +++ b/src/core/vectortile/qgsvectortilelabeling.h @@ -37,7 +37,7 @@ class QgsVectorTileLabelProvider : public QgsVectorLayerLabelProvider explicit QgsVectorTileLabelProvider( QgsVectorTileLayer *layer ); //! Returns field names for each sub-layer that are required for labeling - virtual QMap > usedAttributes( int tileZoom ) const = 0; + virtual QMap > usedAttributes( const QgsRenderContext &context, int tileZoom ) const = 0; //! Sets required fields virtual void setFields( const QMap> &requiredFields ) = 0; diff --git a/src/core/vectortile/qgsvectortilelayerrenderer.cpp b/src/core/vectortile/qgsvectortilelayerrenderer.cpp index 5c458074ab0..b11eed412d8 100644 --- a/src/core/vectortile/qgsvectortilelayerrenderer.cpp +++ b/src/core/vectortile/qgsvectortilelayerrenderer.cpp @@ -124,7 +124,7 @@ bool QgsVectorTileLayerRenderer::render() if ( mLabelProvider ) { - QMap > requiredFieldsLabeling = mLabelProvider->usedAttributes( mTileZoom ); + QMap > requiredFieldsLabeling = mLabelProvider->usedAttributes( ctx, mTileZoom ); for ( QString layerName : requiredFieldsLabeling.keys() ) { requiredFields[layerName].unite( requiredFieldsLabeling[layerName] ); diff --git a/tests/src/core/testqgslabelingengine.cpp b/tests/src/core/testqgslabelingengine.cpp index 10ded7d74f7..97d8e47340f 100644 --- a/tests/src/core/testqgslabelingengine.cpp +++ b/tests/src/core/testqgslabelingengine.cpp @@ -79,6 +79,7 @@ class TestQgsLabelingEngine : public QObject void testRotationBasedOrientationLine(); void testMapUnitLetterSpacing(); void testMapUnitWordSpacing(); + void testReferencedFields(); private: QgsVectorLayer *vl = nullptr; @@ -2631,5 +2632,22 @@ void TestQgsLabelingEngine::testMapUnitWordSpacing() QVERIFY( imageCheck( QStringLiteral( "label_word_spacing_map_units" ), img, 20 ) ); } +void TestQgsLabelingEngine::testReferencedFields() +{ + QgsPalLayerSettings settings; + settings.fieldName = QStringLiteral( "hello+world" ); + settings.isExpression = false; + + QCOMPARE( settings.referencedFields( QgsRenderContext() ), QSet() << QStringLiteral( "hello+world" ) ); + + settings.isExpression = true; + + QCOMPARE( settings.referencedFields( QgsRenderContext() ), QSet() << QStringLiteral( "hello" ) << QStringLiteral( "world" ) ); + + settings.dataDefinedProperties().setProperty( QgsPalLayerSettings::Size, QgsProperty::fromField( QStringLiteral( "my_dd_size" ) ) ); + + QCOMPARE( settings.referencedFields( QgsRenderContext() ), QSet() << QStringLiteral( "hello" ) << QStringLiteral( "world" ) << QStringLiteral( "my_dd_size" ) ); +} + QGSTEST_MAIN( TestQgsLabelingEngine ) #include "testqgslabelingengine.moc" diff --git a/tests/src/core/testqgsproperty.cpp b/tests/src/core/testqgsproperty.cpp index a49f6dfa5e2..e284cff00b8 100644 --- a/tests/src/core/testqgsproperty.cpp +++ b/tests/src/core/testqgsproperty.cpp @@ -94,6 +94,7 @@ class TestQgsProperty : public QObject void curveTransform(); void asVariant(); void isProjectColor(); + void referencedFieldsIgnoreContext(); private: @@ -1803,6 +1804,29 @@ void TestQgsProperty::isProjectColor() QVERIFY( p.isProjectColor() ); } +void TestQgsProperty::referencedFieldsIgnoreContext() +{ + // Currently QgsProperty::referencedFields() for an expression will return field names + // only if those field names are present in the context's fields. The ignoreContext + // argument is a workaround for the case when we don't have fields yet. + + QgsProperty p = QgsProperty::fromExpression( QStringLiteral( "foo + bar" ) ); + QCOMPARE( p.referencedFields( QgsExpressionContext() ), QSet() ); + QCOMPARE( p.referencedFields( QgsExpressionContext(), true ), QSet() << QStringLiteral( "foo" ) << QStringLiteral( "bar" ) ); + + // if the property is from a field, the ignoreContext does not make a difference + QgsProperty p2 = QgsProperty::fromField( QStringLiteral( "boo" ) ); + QCOMPARE( p2.referencedFields( QgsExpressionContext() ), QSet() << QStringLiteral( "boo" ) ); + QCOMPARE( p2.referencedFields( QgsExpressionContext(), true ), QSet() << QStringLiteral( "boo" ) ); + + QgsPropertyCollection collection; + collection.setProperty( 0, p ); + collection.setProperty( 1, p2 ); + + QCOMPARE( collection.referencedFields( QgsExpressionContext() ), QSet() << QStringLiteral( "boo" ) ); + QCOMPARE( collection.referencedFields( QgsExpressionContext(), true ), QSet() << QStringLiteral( "boo" ) << QStringLiteral( "foo" ) << QStringLiteral( "bar" ) ); +} + void TestQgsProperty::checkCurveResult( const QList &controlPoints, const QVector &x, const QVector &y ) { // build transform