Skip to content

Commit

Permalink
Merge pull request #9461 from m-kuhn/pal_cleanup
Browse files Browse the repository at this point in the history
Some cleanup in labeling and pal
  • Loading branch information
m-kuhn authored Mar 11, 2019
2 parents 7757ffc + a133bca commit f575656
Show file tree
Hide file tree
Showing 9 changed files with 35 additions and 31 deletions.
4 changes: 2 additions & 2 deletions python/core/auto_generated/qgspallabeling.sip.in
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,14 @@ class QgsLabelPosition
#include "qgspallabeling.h"
%End
public:
QgsLabelPosition( int id, double r, const QVector< QgsPointXY > &corners, const QgsRectangle &rect, double w, double h, const QString &layer, const QString &labeltext, const QFont &labelfont, bool upside_down, bool diagram = false, bool pinned = false, const QString &providerId = QString() );
QgsLabelPosition( QgsFeatureId id, double r, const QVector< QgsPointXY > &corners, const QgsRectangle &rect, double w, double h, const QString &layer, const QString &labeltext, const QFont &labelfont, bool upside_down, bool diagram = false, bool pinned = false, const QString &providerId = QString() );

QgsLabelPosition();
%Docstring
Constructor for QgsLabelPosition
%End

int featureId;
QgsFeatureId featureId;
double rotation;
QVector< QgsPointXY > cornerPoints;
QgsRectangle labelRect;
Expand Down
6 changes: 2 additions & 4 deletions src/core/pal/layer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ bool Layer::registerFeature( QgsLabelFeature *lf )
std::unique_ptr<FeaturePart> biggest_part;

// break the (possibly multi-part) geometry into simple geometries
QLinkedList<const GEOSGeometry *> *simpleGeometries = Util::unmulti( lf->geometry() );
std::unique_ptr<QLinkedList<const GEOSGeometry *>> simpleGeometries( Util::unmulti( lf->geometry() ) );
if ( !simpleGeometries ) // unmulti() failed?
{
throw InternalException::UnknownGeometry();
Expand Down Expand Up @@ -203,12 +203,11 @@ bool Layer::registerFeature( QgsLabelFeature *lf )
addFeaturePart( fpart.release(), lf->labelText() );
addedFeature = true;
}
delete simpleGeometries;

if ( !featureGeomIsObstacleGeom )
{
//do the same for the obstacle geometry
simpleGeometries = Util::unmulti( lf->obstacleGeometry() );
simpleGeometries.reset( Util::unmulti( lf->obstacleGeometry() ) );
if ( !simpleGeometries ) // unmulti() failed?
{
throw InternalException::UnknownGeometry();
Expand Down Expand Up @@ -249,7 +248,6 @@ bool Layer::registerFeature( QgsLabelFeature *lf )
// feature part is ready!
addObstaclePart( fpart.release() );
}
delete simpleGeometries;
}

locker.unlock();
Expand Down
5 changes: 1 addition & 4 deletions src/core/pal/problem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2138,10 +2138,7 @@ void Problem::chain_search()

featWrap = nullptr;

for ( i = 0; i < nbft; i++ )
{
ok[i] = false;
}
std::fill( ok, ok + nbft, false );

//initialization();
init_sol_falp();
Expand Down
2 changes: 1 addition & 1 deletion src/core/qgslabelsearchtree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ void QgsLabelSearchTree::labelsInRect( const QgsRectangle &r, QList<QgsLabelPosi
}
}

bool QgsLabelSearchTree::insertLabel( pal::LabelPosition *labelPos, int featureId, const QString &layerName, const QString &labeltext, const QFont &labelfont, bool diagram, bool pinned, const QString &providerId )
bool QgsLabelSearchTree::insertLabel( pal::LabelPosition *labelPos, QgsFeatureId featureId, const QString &layerName, const QString &labeltext, const QFont &labelfont, bool diagram, bool pinned, const QString &providerId )
{
if ( !labelPos )
{
Expand Down
2 changes: 1 addition & 1 deletion src/core/qgslabelsearchtree.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ class CORE_EXPORT QgsLabelSearchTree
* \returns TRUE in case of success
* \note not available in Python bindings
*/
bool insertLabel( pal::LabelPosition *labelPos, int featureId, const QString &layerName, const QString &labeltext, const QFont &labelfont, bool diagram = false, bool pinned = false, const QString &providerId = QString() ) SIP_SKIP;
bool insertLabel( pal::LabelPosition *labelPos, QgsFeatureId featureId, const QString &layerName, const QString &labeltext, const QFont &labelfont, bool diagram = false, bool pinned = false, const QString &providerId = QString() ) SIP_SKIP;

private:
// set as mutable because RTree template is not const-correct
Expand Down
4 changes: 2 additions & 2 deletions src/core/qgspallabeling.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ class QgsExpressionContext;
class CORE_EXPORT QgsLabelPosition
{
public:
QgsLabelPosition( int id, double r, const QVector< QgsPointXY > &corners, const QgsRectangle &rect, double w, double h, const QString &layer, const QString &labeltext, const QFont &labelfont, bool upside_down, bool diagram = false, bool pinned = false, const QString &providerId = QString() )
QgsLabelPosition( QgsFeatureId id, double r, const QVector< QgsPointXY > &corners, const QgsRectangle &rect, double w, double h, const QString &layer, const QString &labeltext, const QFont &labelfont, bool upside_down, bool diagram = false, bool pinned = false, const QString &providerId = QString() )
: featureId( id )
, rotation( r )
, cornerPoints( corners )
Expand All @@ -98,7 +98,7 @@ class CORE_EXPORT QgsLabelPosition
//! Constructor for QgsLabelPosition
QgsLabelPosition() = default;

int featureId = -1;
QgsFeatureId featureId = FID_NULL;
double rotation = 0;
QVector< QgsPointXY > cornerPoints;
QgsRectangle labelRect;
Expand Down
35 changes: 22 additions & 13 deletions src/core/qgsrulebasedlabeling.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ QgsVectorLayerLabelProvider *QgsRuleBasedLabelProvider::createProvider( QgsVecto

bool QgsRuleBasedLabelProvider::prepare( const QgsRenderContext &context, QSet<QString> &attributeNames )
{
Q_FOREACH ( QgsVectorLayerLabelProvider *provider, mSubProviders )
for ( QgsVectorLayerLabelProvider *provider : qgis::as_const( mSubProviders ) )
provider->setEngine( mEngine );

// populate sub-providers
Expand All @@ -46,7 +46,7 @@ void QgsRuleBasedLabelProvider::registerFeature( const QgsFeature &feature, QgsR
QList<QgsAbstractLabelProvider *> QgsRuleBasedLabelProvider::subProviders()
{
QList<QgsAbstractLabelProvider *> lst;
Q_FOREACH ( QgsVectorLayerLabelProvider *subprovider, mSubProviders )
for ( QgsVectorLayerLabelProvider *subprovider : qgis::as_const( mSubProviders ) )
lst << subprovider;
return lst;
}
Expand Down Expand Up @@ -111,7 +111,7 @@ void QgsRuleBasedLabeling::Rule::initFilter()
void QgsRuleBasedLabeling::Rule::updateElseRules()
{
mElseRules.clear();
Q_FOREACH ( Rule *rule, mChildren )
for ( Rule *rule : qgis::as_const( mChildren ) )
{
if ( rule->isElse() )
mElseRules << rule;
Expand All @@ -123,7 +123,7 @@ bool QgsRuleBasedLabeling::Rule::requiresAdvancedEffects() const
if ( mSettings && mSettings->format().containsAdvancedEffects() )
return true;

Q_FOREACH ( Rule *rule, mChildren )
for ( Rule *rule : qgis::as_const( mChildren ) )
{
if ( rule->requiresAdvancedEffects() )
return true;
Expand All @@ -134,7 +134,7 @@ bool QgsRuleBasedLabeling::Rule::requiresAdvancedEffects() const

void QgsRuleBasedLabeling::Rule::subProviderIds( QStringList &list ) const
{
Q_FOREACH ( const Rule *rule, mChildren )
for ( const Rule *rule : qgis::as_const( mChildren ) )
{
if ( rule->settings() )
list << rule->ruleKey();
Expand Down Expand Up @@ -172,7 +172,7 @@ const QgsRuleBasedLabeling::Rule *QgsRuleBasedLabeling::Rule::findRuleByKey( con
if ( key == mRuleKey )
return this;

Q_FOREACH ( Rule *rule, mChildren )
for ( Rule *rule : mChildren )
{
const Rule *r = rule->findRuleByKey( key );
if ( r )
Expand Down Expand Up @@ -201,7 +201,7 @@ QgsRuleBasedLabeling::Rule *QgsRuleBasedLabeling::Rule::clone() const
Rule *newrule = new Rule( s, mMaximumScale, mMinimumScale, mFilterExp, mDescription );
newrule->setActive( mIsActive );
// clone children
Q_FOREACH ( Rule *rule, mChildren )
for ( Rule *rule : mChildren )
newrule->appendChild( rule->clone() );
return newrule;
}
Expand Down Expand Up @@ -285,7 +285,7 @@ void QgsRuleBasedLabeling::Rule::createSubProviders( QgsVectorLayer *layer, QgsR
}

// call recursively
Q_FOREACH ( Rule *rule, mChildren )
for ( Rule *rule : qgis::as_const( mChildren ) )
{
rule->createSubProviders( layer, subProviders, provider );
}
Expand All @@ -310,7 +310,7 @@ void QgsRuleBasedLabeling::Rule::prepare( const QgsRenderContext &context, QSet<
}

// call recursively
Q_FOREACH ( Rule *rule, mChildren )
for ( Rule *rule : qgis::as_const( mChildren ) )
{
rule->prepare( context, attributeNames, subProviders );
}
Expand All @@ -334,7 +334,7 @@ QgsRuleBasedLabeling::Rule::RegisterResult QgsRuleBasedLabeling::Rule::registerF
bool willRegisterSomething = false;

// call recursively
Q_FOREACH ( Rule *rule, mChildren )
for ( Rule *rule : qgis::as_const( mChildren ) )
{
// Don't process else rules yet
if ( !rule->isElse() )
Expand All @@ -349,7 +349,7 @@ QgsRuleBasedLabeling::Rule::RegisterResult QgsRuleBasedLabeling::Rule::registerF
// If none of the rules passed then we jump into the else rules and process them.
if ( !willRegisterSomething )
{
Q_FOREACH ( Rule *rule, mElseRules )
for ( Rule *rule : qgis::as_const( mElseRules ) )
{
registered |= rule->registerFeature( feature, context, subProviders, obstacleGeometry ) != Filtered;
}
Expand All @@ -370,7 +370,7 @@ bool QgsRuleBasedLabeling::Rule::isFilterOK( const QgsFeature &f, QgsRenderConte

context.expressionContext().setFeature( f );
QVariant res = mFilter->evaluate( &context.expressionContext() );
return res.toInt() != 0;
return res.toBool();
}

bool QgsRuleBasedLabeling::Rule::isScaleOK( double scale ) const
Expand Down Expand Up @@ -411,7 +411,16 @@ QgsRuleBasedLabeling *QgsRuleBasedLabeling::clone() const

QgsRuleBasedLabeling::~QgsRuleBasedLabeling()
{
delete mRootRule;
}

QgsRuleBasedLabeling::Rule *QgsRuleBasedLabeling::rootRule()
{
return mRootRule.get();
}

const QgsRuleBasedLabeling::Rule *QgsRuleBasedLabeling::rootRule() const
{
return mRootRule.get();
}


Expand Down
6 changes: 3 additions & 3 deletions src/core/qgsrulebasedlabeling.h
Original file line number Diff line number Diff line change
Expand Up @@ -346,8 +346,8 @@ class CORE_EXPORT QgsRuleBasedLabeling : public QgsAbstractVectorLayerLabeling
explicit QgsRuleBasedLabeling( QgsRuleBasedLabeling::Rule *root SIP_TRANSFER );
~QgsRuleBasedLabeling() override;

QgsRuleBasedLabeling::Rule *rootRule() { return mRootRule; }
const Rule *rootRule() const SIP_SKIP { return mRootRule; }
QgsRuleBasedLabeling::Rule *rootRule();
const Rule *rootRule() const SIP_SKIP;

//! Create the instance from a DOM element with saved configuration
static QgsRuleBasedLabeling *create( const QDomElement &element, const QgsReadWriteContext &context ) SIP_FACTORY;
Expand Down Expand Up @@ -375,7 +375,7 @@ class CORE_EXPORT QgsRuleBasedLabeling : public QgsAbstractVectorLayerLabeling
void toSld( QDomNode &parent, const QgsStringMap &props ) const override;

protected:
Rule *mRootRule = nullptr;
std::unique_ptr<Rule> mRootRule;
};

#ifndef SIP_RUN
Expand Down
2 changes: 1 addition & 1 deletion tests/code_layout/acceptable_missing_doc.py
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@
"QgsInterpolator": ['QgsInterpolator(const QList< QgsInterpolator::LayerData > &layerData)'],
"QgsLUDialog": ['QgsLUDialog(QWidget *parent=nullptr, Qt::WindowFlags fl=QgsGuiUtils::ModalDialogFlags)', 'lowerValue() const', 'setLowerValue(const QString &val)', 'setUpperValue(const QString &val)', 'upperValue() const'],
"QgsLabelCandidate": ['QgsLabelCandidate(const QRectF &r, double c)'],
"QgsLabelPosition": ['QgsLabelPosition(int id, double r, const QVector< QgsPointXY > &corners, const QgsRectangle &rect, double w, double h, const QString &layer, const QString &labeltext, const QFont &labelfont, bool upside_down, bool diagram=false, bool pinned=false, const QString &providerId=QString())'],
"QgsLabelPosition": ['QgsLabelPosition(QgsFeatureId id, double r, const QVector< QgsPointXY > &corners, const QgsRectangle &rect, double w, double h, const QString &layer, const QString &labeltext, const QFont &labelfont, bool upside_down, bool diagram=false, bool pinned=false, const QString &providerId=QString())'],
"QgsLabelSorter": ['QgsLabelSorter(const QgsMapSettings &mapSettings)', 'operator()(pal::LabelPosition *lp1, pal::LabelPosition *lp2) const'],
"QgsLabelingEngine": ['processProvider(QgsAbstractLabelProvider *provider, QgsRenderContext &context, pal::Pal &p)'],
"QgsLayerItem": ['LayerType', 'QgsLayerItem(QgsDataItem *parent, const QString &name, const QString &path, const QString &uri, LayerType layerType, const QString &providerKey)', 'iconDefault()', 'iconLine()', 'iconPoint()', 'iconPolygon()', 'iconRaster()', 'iconTable()'],
Expand Down

0 comments on commit f575656

Please sign in to comment.