From 4612521c505364fda3adef7097bda318b24d45c8 Mon Sep 17 00:00:00 2001 From: Matthias Kuhn Date: Tue, 5 Feb 2019 17:15:17 +0100 Subject: [PATCH 1/3] Const correctnes for QgsFeature in labeling A feature is not modified while it's sent through the labeling pipeline. --- python/core/auto_generated/qgspallabeling.sip.in | 7 +++---- src/core/dxf/qgsdxfpallabeling.cpp | 2 +- src/core/dxf/qgsdxfpallabeling.h | 2 +- src/core/qgspallabeling.cpp | 6 +++--- src/core/qgspallabeling.h | 9 ++++----- src/core/qgsrulebasedlabeling.cpp | 6 +++--- src/core/qgsrulebasedlabeling.h | 6 +++--- src/core/qgsvectorlayerlabelprovider.cpp | 2 +- src/core/qgsvectorlayerlabelprovider.h | 2 +- 9 files changed, 20 insertions(+), 22 deletions(-) diff --git a/python/core/auto_generated/qgspallabeling.sip.in b/python/core/auto_generated/qgspallabeling.sip.in index 5ad4d53c0ed..833593bf234 100644 --- a/python/core/auto_generated/qgspallabeling.sip.in +++ b/python/core/auto_generated/qgspallabeling.sip.in @@ -386,9 +386,9 @@ Returns the QgsExpression for this label settings. May be None if isExpression i double zIndex; - void calculateLabelSize( const QFontMetricsF *fm, QString text, double &labelX, double &labelY, QgsFeature *f = 0, QgsRenderContext *context = 0 ); + void calculateLabelSize( const QFontMetricsF *fm, QString text, double &labelX, double &labelY, const QgsFeature *f = 0, QgsRenderContext *context = 0 ); - void registerFeature( QgsFeature &f, QgsRenderContext &context ); + void registerFeature( const QgsFeature &f, QgsRenderContext &context ); %Docstring Register a feature for labeling. @@ -457,7 +457,7 @@ Sets the label text formatting settings, e.g., font settings, buffer settings, e .. versionadded:: 3.0 %End - QgsFeature *mCurFeat; + const QgsFeature *mCurFeat; QgsFields mCurFields; int fieldIndex; const QgsMapToPixel *xform; @@ -469,7 +469,6 @@ Sets the label text formatting settings, e.g., font settings, buffer settings, e int mFeaturesToLabel; int mFeatsSendingToPal; int mFeatsRegPal; - }; class QgsLabelCandidate diff --git a/src/core/dxf/qgsdxfpallabeling.cpp b/src/core/dxf/qgsdxfpallabeling.cpp index d414b91d796..cf8d5d5d36e 100644 --- a/src/core/dxf/qgsdxfpallabeling.cpp +++ b/src/core/dxf/qgsdxfpallabeling.cpp @@ -34,7 +34,7 @@ void QgsDxfLabelProvider::drawLabel( QgsRenderContext &context, pal::LabelPositi mDxfExport->drawLabel( layerId(), context, label, mSettings ); } -void QgsDxfLabelProvider::registerDxfFeature( QgsFeature &feature, QgsRenderContext &context, const QString &dxfLayerName ) +void QgsDxfLabelProvider::registerDxfFeature( const QgsFeature &feature, QgsRenderContext &context, const QString &dxfLayerName ) { registerFeature( feature, context ); mDxfExport->registerDxfLayer( layerId(), feature.id(), dxfLayerName ); diff --git a/src/core/dxf/qgsdxfpallabeling.h b/src/core/dxf/qgsdxfpallabeling.h index 6d2c9b22938..fa6df02a0a5 100644 --- a/src/core/dxf/qgsdxfpallabeling.h +++ b/src/core/dxf/qgsdxfpallabeling.h @@ -54,7 +54,7 @@ class QgsDxfLabelProvider : public QgsVectorLayerLabelProvider * \param context render context * \param dxfLayerName name of dxf layer */ - void registerDxfFeature( QgsFeature &feature, QgsRenderContext &context, const QString &dxfLayerName ); + void registerDxfFeature( const QgsFeature &feature, QgsRenderContext &context, const QString &dxfLayerName ); protected: //! pointer to parent DXF export where this instance is used diff --git a/src/core/qgspallabeling.cpp b/src/core/qgspallabeling.cpp index c1c455d718d..3a302945515 100644 --- a/src/core/qgspallabeling.cpp +++ b/src/core/qgspallabeling.cpp @@ -1038,7 +1038,7 @@ bool QgsPalLayerSettings::checkMinimumSizeMM( const QgsRenderContext &ct, const return QgsPalLabeling::checkMinimumSizeMM( ct, geom, minSize ); } -void QgsPalLayerSettings::calculateLabelSize( const QFontMetricsF *fm, QString text, double &labelX, double &labelY, QgsFeature *f, QgsRenderContext *context ) +void QgsPalLayerSettings::calculateLabelSize( const QFontMetricsF *fm, QString text, double &labelX, double &labelY, const QgsFeature *f, QgsRenderContext *context ) { if ( !fm || !f ) { @@ -1186,7 +1186,7 @@ void QgsPalLayerSettings::calculateLabelSize( const QFontMetricsF *fm, QString t #endif } -void QgsPalLayerSettings::registerFeature( QgsFeature &f, QgsRenderContext &context, QgsLabelFeature **labelFeature, QgsGeometry obstacleGeometry ) +void QgsPalLayerSettings::registerFeature( const QgsFeature &f, QgsRenderContext &context, QgsLabelFeature **labelFeature, QgsGeometry obstacleGeometry ) { // either used in QgsPalLabeling (palLayer is set) or in QgsLabelingEngine (labelFeature is set) Q_ASSERT( labelFeature ); @@ -1995,7 +1995,7 @@ void QgsPalLayerSettings::registerFeature( QgsFeature &f, QgsRenderContext &cont lf->setDataDefinedValues( dataDefinedValues ); } -void QgsPalLayerSettings::registerObstacleFeature( QgsFeature &f, QgsRenderContext &context, QgsLabelFeature **obstacleFeature, const QgsGeometry &obstacleGeometry ) +void QgsPalLayerSettings::registerObstacleFeature( const QgsFeature &f, QgsRenderContext &context, QgsLabelFeature **obstacleFeature, const QgsGeometry &obstacleGeometry ) { mCurFeat = &f; diff --git a/src/core/qgspallabeling.h b/src/core/qgspallabeling.h index 255674fa895..6b6e6cd0415 100644 --- a/src/core/qgspallabeling.h +++ b/src/core/qgspallabeling.h @@ -751,7 +751,7 @@ class CORE_EXPORT QgsPalLayerSettings double zIndex; // called from register feature hook - void calculateLabelSize( const QFontMetricsF *fm, QString text, double &labelX, double &labelY, QgsFeature *f = nullptr, QgsRenderContext *context = nullptr ); + void calculateLabelSize( const QFontMetricsF *fm, QString text, double &labelX, double &labelY, const QgsFeature *f = nullptr, QgsRenderContext *context = nullptr ); /** * Register a feature for labeling. @@ -766,7 +766,7 @@ class CORE_EXPORT QgsPalLayerSettings * the feature's original geometry will be used as an obstacle for labels. Not available * in Python bindings. */ - void registerFeature( QgsFeature &f, QgsRenderContext &context, + void registerFeature( const QgsFeature &f, QgsRenderContext &context, QgsLabelFeature **labelFeature SIP_PYARGREMOVE = nullptr, QgsGeometry obstacleGeometry SIP_PYARGREMOVE = QgsGeometry() ); @@ -823,7 +823,7 @@ class CORE_EXPORT QgsPalLayerSettings void setFormat( const QgsTextFormat &format ) { mFormat = format; } // temporary stuff: set when layer gets prepared or labeled - QgsFeature *mCurFeat = nullptr; + const QgsFeature *mCurFeat = nullptr; QgsFields mCurFields; int fieldIndex; const QgsMapToPixel *xform = nullptr; @@ -835,7 +835,6 @@ class CORE_EXPORT QgsPalLayerSettings int mFeaturesToLabel = 0; // total features that will probably be labeled, may be less (figured before PAL) int mFeatsSendingToPal = 0; // total features tested for sending into PAL (relative to maxNumLabels) int mFeatsRegPal = 0; // number of features registered in PAL, when using limitNumLabels - private: friend class QgsVectorLayer; // to allow calling readFromLayerCustomProperties() @@ -899,7 +898,7 @@ class CORE_EXPORT QgsPalLayerSettings /** * Registers a feature as an obstacle only (no label rendered) */ - void registerObstacleFeature( QgsFeature &f, QgsRenderContext &context, QgsLabelFeature **obstacleFeature, const QgsGeometry &obstacleGeometry = QgsGeometry() ); + void registerObstacleFeature( const QgsFeature &f, QgsRenderContext &context, QgsLabelFeature **obstacleFeature, const QgsGeometry &obstacleGeometry = QgsGeometry() ); QMap dataDefinedValues; diff --git a/src/core/qgsrulebasedlabeling.cpp b/src/core/qgsrulebasedlabeling.cpp index c54b7723cce..8911ea17ec5 100644 --- a/src/core/qgsrulebasedlabeling.cpp +++ b/src/core/qgsrulebasedlabeling.cpp @@ -37,7 +37,7 @@ bool QgsRuleBasedLabelProvider::prepare( const QgsRenderContext &context, QSetrootRule()->registerFeature( feature, context, mSubProviders, obstacleGeometry ); @@ -316,7 +316,7 @@ void QgsRuleBasedLabeling::Rule::prepare( const QgsRenderContext &context, QSet< } } -QgsRuleBasedLabeling::Rule::RegisterResult QgsRuleBasedLabeling::Rule::registerFeature( QgsFeature &feature, QgsRenderContext &context, QgsRuleBasedLabeling::RuleToProviderMap &subProviders, const QgsGeometry &obstacleGeometry ) +QgsRuleBasedLabeling::Rule::RegisterResult QgsRuleBasedLabeling::Rule::registerFeature( const QgsFeature &feature, QgsRenderContext &context, QgsRuleBasedLabeling::RuleToProviderMap &subProviders, const QgsGeometry &obstacleGeometry ) { if ( !isFilterOK( feature, context ) || !isScaleOK( context.rendererScale() ) ) @@ -363,7 +363,7 @@ QgsRuleBasedLabeling::Rule::RegisterResult QgsRuleBasedLabeling::Rule::registerF return Filtered; } -bool QgsRuleBasedLabeling::Rule::isFilterOK( QgsFeature &f, QgsRenderContext &context ) const +bool QgsRuleBasedLabeling::Rule::isFilterOK( const QgsFeature &f, QgsRenderContext &context ) const { if ( ! mFilter || mElseRule ) return true; diff --git a/src/core/qgsrulebasedlabeling.h b/src/core/qgsrulebasedlabeling.h index 8f3f2fb0d5a..dd18d87ece1 100644 --- a/src/core/qgsrulebasedlabeling.h +++ b/src/core/qgsrulebasedlabeling.h @@ -282,7 +282,7 @@ class CORE_EXPORT QgsRuleBasedLabeling : public QgsAbstractVectorLayerLabeling * register individual features * \note not available in Python bindings */ - RegisterResult registerFeature( QgsFeature &feature, QgsRenderContext &context, RuleToProviderMap &subProviders, const QgsGeometry &obstacleGeometry = QgsGeometry() ) SIP_SKIP; + RegisterResult registerFeature( const QgsFeature &feature, QgsRenderContext &context, RuleToProviderMap &subProviders, const QgsGeometry &obstacleGeometry = QgsGeometry() ) SIP_SKIP; /** * Returns true if this rule or any of its children requires advanced composition effects @@ -302,7 +302,7 @@ class CORE_EXPORT QgsRuleBasedLabeling : public QgsAbstractVectorLayerLabeling * \param context The context in which the rendering happens * \returns True if the feature shall be rendered */ - bool isFilterOK( QgsFeature &f, QgsRenderContext &context ) const; + bool isFilterOK( const QgsFeature &f, QgsRenderContext &context ) const; /** * Check if this rule applies for a given \a scale. @@ -395,7 +395,7 @@ class CORE_EXPORT QgsRuleBasedLabelProvider : public QgsVectorLayerLabelProvider bool prepare( const QgsRenderContext &context, QSet &attributeNames ) override; - void registerFeature( QgsFeature &feature, QgsRenderContext &context, const QgsGeometry &obstacleGeometry = QgsGeometry() ) override; + void registerFeature( const QgsFeature &feature, QgsRenderContext &context, const QgsGeometry &obstacleGeometry = QgsGeometry() ) override; //! create a label provider virtual QgsVectorLayerLabelProvider *createProvider( QgsVectorLayer *layer, const QString &providerId, bool withFeatureLoop, const QgsPalLayerSettings *settings ); diff --git a/src/core/qgsvectorlayerlabelprovider.cpp b/src/core/qgsvectorlayerlabelprovider.cpp index 075d8984b1f..7810d017114 100644 --- a/src/core/qgsvectorlayerlabelprovider.cpp +++ b/src/core/qgsvectorlayerlabelprovider.cpp @@ -258,7 +258,7 @@ QList QgsVectorLayerLabelProvider::labelFeatures( QgsRenderCo return mLabels; } -void QgsVectorLayerLabelProvider::registerFeature( QgsFeature &feature, QgsRenderContext &context, const QgsGeometry &obstacleGeometry ) +void QgsVectorLayerLabelProvider::registerFeature( const QgsFeature &feature, QgsRenderContext &context, const QgsGeometry &obstacleGeometry ) { QgsLabelFeature *label = nullptr; mSettings.registerFeature( feature, context, &label, obstacleGeometry ); diff --git a/src/core/qgsvectorlayerlabelprovider.h b/src/core/qgsvectorlayerlabelprovider.h index 8b64790a8e7..a08546d588e 100644 --- a/src/core/qgsvectorlayerlabelprovider.h +++ b/src/core/qgsvectorlayerlabelprovider.h @@ -74,7 +74,7 @@ class CORE_EXPORT QgsVectorLayerLabelProvider : public QgsAbstractLabelProvider * symbol, the obstacle geometry should represent the bounds of the offset symbol). If not set, * the feature's original geometry will be used as an obstacle for labels. */ - virtual void registerFeature( QgsFeature &feature, QgsRenderContext &context, const QgsGeometry &obstacleGeometry = QgsGeometry() ); + virtual void registerFeature( const QgsFeature &feature, QgsRenderContext &context, const QgsGeometry &obstacleGeometry = QgsGeometry() ); /** * Returns the geometry for a point feature which should be used as an obstacle for labels. This From 5f1cea13c7e9348cc9f1260f35fe80f2244f306a Mon Sep 17 00:00:00 2001 From: Matthias Kuhn Date: Tue, 5 Feb 2019 17:57:27 +0100 Subject: [PATCH 2/3] Add docstrings and modernize code --- .../core/auto_generated/qgspallabeling.sip.in | 6 +++++- src/core/qgspallabeling.cpp | 18 ++++++++---------- src/core/qgspallabeling.h | 7 +++++-- src/core/qgsvectorlayerlabelprovider.cpp | 1 + 4 files changed, 19 insertions(+), 13 deletions(-) diff --git a/python/core/auto_generated/qgspallabeling.sip.in b/python/core/auto_generated/qgspallabeling.sip.in index 833593bf234..423e30df377 100644 --- a/python/core/auto_generated/qgspallabeling.sip.in +++ b/python/core/auto_generated/qgspallabeling.sip.in @@ -386,7 +386,11 @@ Returns the QgsExpression for this label settings. May be None if isExpression i double zIndex; - void calculateLabelSize( const QFontMetricsF *fm, QString text, double &labelX, double &labelY, const QgsFeature *f = 0, QgsRenderContext *context = 0 ); + void calculateLabelSize( const QFontMetricsF *fm, const QString &text, double &labelX, double &labelY, const QgsFeature *f = 0, QgsRenderContext *context = 0 ); +%Docstring +Calculates the space required to render the provided ``text`` in map units. +Results will be written to ``labelX`` and ``labelY``. +%End void registerFeature( const QgsFeature &f, QgsRenderContext &context ); diff --git a/src/core/qgspallabeling.cpp b/src/core/qgspallabeling.cpp index 3a302945515..e6fd7890e0e 100644 --- a/src/core/qgspallabeling.cpp +++ b/src/core/qgspallabeling.cpp @@ -1038,13 +1038,15 @@ bool QgsPalLayerSettings::checkMinimumSizeMM( const QgsRenderContext &ct, const return QgsPalLabeling::checkMinimumSizeMM( ct, geom, minSize ); } -void QgsPalLayerSettings::calculateLabelSize( const QFontMetricsF *fm, QString text, double &labelX, double &labelY, const QgsFeature *f, QgsRenderContext *context ) +void QgsPalLayerSettings::calculateLabelSize( const QFontMetricsF *fm, const QString &text, double &labelX, double &labelY, const QgsFeature *f, QgsRenderContext *context ) { if ( !fm || !f ) { return; } + QString textCopy( text ); + //try to keep < 2.12 API - handle no passed render context std::unique_ptr< QgsRenderContext > scopedRc; if ( !context ) @@ -1150,29 +1152,25 @@ void QgsPalLayerSettings::calculateLabelSize( const QFontMetricsF *fm, QString t if ( placeDirSymb == QgsPalLayerSettings::SymbolLeftRight ) { - text.append( dirSym ); + textCopy.append( dirSym ); } else { - text.prepend( dirSym + QStringLiteral( "\n" ) ); // SymbolAbove or SymbolBelow + textCopy.prepend( dirSym + QStringLiteral( "\n" ) ); // SymbolAbove or SymbolBelow } } double w = 0.0, h = 0.0; - QStringList multiLineSplit = QgsPalLabeling::splitToLines( text, wrapchr, evalAutoWrapLength, useMaxLineLengthForAutoWrap ); + const QStringList multiLineSplit = QgsPalLabeling::splitToLines( textCopy, wrapchr, evalAutoWrapLength, useMaxLineLengthForAutoWrap ); int lines = multiLineSplit.size(); double labelHeight = fm->ascent() + fm->descent(); // ignore +1 for baseline h += fm->height() + static_cast< double >( ( lines - 1 ) * labelHeight * multilineH ); - for ( int i = 0; i < lines; ++i ) + for ( const QString &line : multiLineSplit ) { - double width = fm->width( multiLineSplit.at( i ) ); - if ( width > w ) - { - w = width; - } + w = qMax( w, fm->width( line ) ); } #if 0 // XXX strk diff --git a/src/core/qgspallabeling.h b/src/core/qgspallabeling.h index 6b6e6cd0415..0678baf4c51 100644 --- a/src/core/qgspallabeling.h +++ b/src/core/qgspallabeling.h @@ -750,8 +750,11 @@ class CORE_EXPORT QgsPalLayerSettings //! Z-Index of label, where labels with a higher z-index are rendered on top of labels with a lower z-index double zIndex; - // called from register feature hook - void calculateLabelSize( const QFontMetricsF *fm, QString text, double &labelX, double &labelY, const QgsFeature *f = nullptr, QgsRenderContext *context = nullptr ); + /** + * Calculates the space required to render the provided \a text in map units. + * Results will be written to \a labelX and \a labelY. + */ + void calculateLabelSize( const QFontMetricsF *fm, const QString &text, double &labelX, double &labelY, const QgsFeature *f = nullptr, QgsRenderContext *context = nullptr ); /** * Register a feature for labeling. diff --git a/src/core/qgsvectorlayerlabelprovider.cpp b/src/core/qgsvectorlayerlabelprovider.cpp index 7810d017114..77bf7fec29f 100644 --- a/src/core/qgsvectorlayerlabelprovider.cpp +++ b/src/core/qgsvectorlayerlabelprovider.cpp @@ -261,6 +261,7 @@ QList QgsVectorLayerLabelProvider::labelFeatures( QgsRenderCo void QgsVectorLayerLabelProvider::registerFeature( const QgsFeature &feature, QgsRenderContext &context, const QgsGeometry &obstacleGeometry ) { QgsLabelFeature *label = nullptr; + mSettings.registerFeature( feature, context, &label, obstacleGeometry ); if ( label ) mLabels << label; From d2885607bd454e95e230f6ad8f023170d28a460e Mon Sep 17 00:00:00 2001 From: Matthias Kuhn Date: Tue, 5 Feb 2019 18:49:17 +0100 Subject: [PATCH 3/3] Banned keywords --- src/core/qgspallabeling.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/qgspallabeling.cpp b/src/core/qgspallabeling.cpp index e6fd7890e0e..030eb50ff67 100644 --- a/src/core/qgspallabeling.cpp +++ b/src/core/qgspallabeling.cpp @@ -1170,7 +1170,7 @@ void QgsPalLayerSettings::calculateLabelSize( const QFontMetricsF *fm, const QSt for ( const QString &line : multiLineSplit ) { - w = qMax( w, fm->width( line ) ); + w = std::max( w, fm->width( line ) ); } #if 0 // XXX strk