Skip to content

Commit 3afe74b

Browse files
committedSep 2, 2020
[labeling] Fix crash when labeling features and showing unplaced
labels and some labels have no candidate positions Fixes #38093
1 parent ac79bfa commit 3afe74b

File tree

3 files changed

+80
-12
lines changed

3 files changed

+80
-12
lines changed
 

‎src/core/pal/problem.cpp

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -656,27 +656,32 @@ void Problem::chain_search()
656656

657657
QList<LabelPosition *> Problem::getSolution( bool returnInactive, QList<LabelPosition *> *unlabeled )
658658
{
659-
QList<LabelPosition *> solList;
659+
QList<LabelPosition *> finalLabelPlacements;
660660

661+
// loop through all features to be labeled
661662
for ( std::size_t i = 0; i < mFeatureCount; i++ )
662663
{
663-
if ( mSol.activeLabelIds[i] != -1 )
664+
const int labelId = mSol.activeLabelIds[i];
665+
const bool foundNonOverlappingPlacement = labelId != -1;
666+
const int startIndexForLabelPlacements = mFeatStartId[i];
667+
const bool foundCandidatesForFeature = startIndexForLabelPlacements < static_cast< int >( mLabelPositions.size() );
668+
669+
if ( foundNonOverlappingPlacement )
664670
{
665-
solList.push_back( mLabelPositions[ mSol.activeLabelIds[i] ].get() ); // active labels
671+
finalLabelPlacements.push_back( mLabelPositions[ labelId ].get() ); // active labels
666672
}
667-
else if ( returnInactive
668-
|| ( mFeatStartId[i] < static_cast< int >( mLabelPositions.size() ) &&
669-
( mLabelPositions.at( mFeatStartId[i] )->getFeaturePart()->layer()->displayAll()
670-
|| mLabelPositions.at( mFeatStartId[i] )->getFeaturePart()->alwaysShow() ) ) )
673+
else if ( foundCandidatesForFeature &&
674+
( returnInactive // allowing any overlapping labels regardless of where they are from
675+
|| mLabelPositions.at( startIndexForLabelPlacements )->getFeaturePart()->layer()->displayAll() // allowing overlapping labels for the layer
676+
|| mLabelPositions.at( startIndexForLabelPlacements )->getFeaturePart()->alwaysShow() ) ) // allowing overlapping labels for the feature
671677
{
672-
solList.push_back( mLabelPositions[ mFeatStartId[i] ].get() ); // unplaced label
678+
finalLabelPlacements.push_back( mLabelPositions[ startIndexForLabelPlacements ].get() ); // unplaced label
673679
}
674680
else if ( unlabeled )
675681
{
676-
const int startPos = mFeatStartId[i];
677682
// need to be careful here -- if the next feature's start id is the same as this one, then this feature had no candidates!
678-
if ( startPos < static_cast< int >( mLabelPositions.size() ) && ( i == mFeatureCount - 1 || startPos != mFeatStartId[i + 1] ) )
679-
unlabeled->push_back( mLabelPositions[ startPos ].get() );
683+
if ( foundCandidatesForFeature && ( i == mFeatureCount - 1 || startIndexForLabelPlacements != mFeatStartId[i + 1] ) )
684+
unlabeled->push_back( mLabelPositions[ startIndexForLabelPlacements ].get() );
680685
}
681686
}
682687

@@ -687,7 +692,7 @@ QList<LabelPosition *> Problem::getSolution( bool returnInactive, QList<LabelPos
687692
unlabeled->append( position.get() );
688693
}
689694

690-
return solList;
695+
return finalLabelPlacements;
691696
}
692697

693698
void Problem::solution_cost()

‎tests/src/core/testqgslabelingengine.cpp

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ class TestQgsLabelingEngine : public QObject
8989
void testLineAnchorCurvedConstraints();
9090
void testLineAnchorHorizontal();
9191
void testLineAnchorHorizontalConstraints();
92+
void testShowAllLabelsWhenALabelHasNoCandidates();
9293

9394
private:
9495
QgsVectorLayer *vl = nullptr;
@@ -3212,5 +3213,67 @@ void TestQgsLabelingEngine::testLineAnchorHorizontalConstraints()
32123213
QVERIFY( imageCheck( QStringLiteral( "horizontal_strict_anchor_end" ), img, 20 ) );
32133214
}
32143215

3216+
void TestQgsLabelingEngine::testShowAllLabelsWhenALabelHasNoCandidates()
3217+
{
3218+
// test that showing all labels when a label has no candidate placements doesn't
3219+
// result in a crash
3220+
// refs https://github.com/qgis/QGIS/issues/38093
3221+
3222+
QgsPalLayerSettings settings;
3223+
setDefaultLabelParams( settings );
3224+
3225+
QgsTextFormat format = settings.format();
3226+
format.setSize( 20 );
3227+
format.setColor( QColor( 0, 0, 0 ) );
3228+
settings.setFormat( format );
3229+
3230+
settings.fieldName = QStringLiteral( "'xxxxxxxxxxxxxx'" );
3231+
settings.isExpression = true;
3232+
settings.placement = QgsPalLayerSettings::Line;
3233+
settings.lineSettings().setPlacementFlags( QgsLabeling::LinePlacementFlag::OnLine );
3234+
settings.obstacleSettings().setFactor( 10 );
3235+
settings.lineSettings().setOverrunDistance( 50 );
3236+
3237+
std::unique_ptr< QgsVectorLayer> vl2( new QgsVectorLayer( QStringLiteral( "LineString?crs=epsg:23700&field=l:string" ), QStringLiteral( "vl" ), QStringLiteral( "memory" ) ) );
3238+
vl2->setRenderer( new QgsNullSymbolRenderer() );
3239+
3240+
QgsFeature f;
3241+
f.setAttributes( QgsAttributes() << QVariant() );
3242+
3243+
f.setGeometry( QgsGeometry::fromWkt( QStringLiteral( "LineString (-2446233 -5204828, -2342845 -5203825)" ) ) );
3244+
QVERIFY( vl2->dataProvider()->addFeature( f ) );
3245+
3246+
f.setGeometry( QgsGeometry::fromWkt( QStringLiteral( "LineString (-2439207 -5198806, -2331302 -5197802)" ) ) );
3247+
QVERIFY( vl2->dataProvider()->addFeature( f ) );
3248+
3249+
vl2->setLabeling( new QgsVectorLayerSimpleLabeling( settings ) ); // TODO: this should not be necessary!
3250+
vl2->setLabelsEnabled( true );
3251+
3252+
// make a fake render context
3253+
QSize size( 640, 480 );
3254+
QgsMapSettings mapSettings;
3255+
mapSettings.setLabelingEngineSettings( createLabelEngineSettings() );
3256+
mapSettings.setDestinationCrs( vl2->crs() );
3257+
3258+
mapSettings.setOutputSize( size );
3259+
mapSettings.setExtent( QgsRectangle( -3328044.9, -5963176., -1127782.7, -4276844.3 ) );
3260+
mapSettings.setLayers( QList<QgsMapLayer *>() << vl2.get() );
3261+
mapSettings.setOutputDpi( 96 );
3262+
3263+
QgsLabelingEngineSettings engineSettings = mapSettings.labelingEngineSettings();
3264+
engineSettings.setFlag( QgsLabelingEngineSettings::DrawLabelRectOnly, true );
3265+
engineSettings.setFlag( QgsLabelingEngineSettings::UseAllLabels, true );
3266+
engineSettings.setFlag( QgsLabelingEngineSettings::DrawUnplacedLabels, true );
3267+
// engineSettings.setFlag( QgsLabelingEngineSettings::DrawCandidates, true );
3268+
mapSettings.setLabelingEngineSettings( engineSettings );
3269+
3270+
QgsMapRendererSequentialJob job( mapSettings );
3271+
job.start();
3272+
job.waitForFinished();
3273+
3274+
QImage img = job.renderedImage();
3275+
QVERIFY( imageCheck( QStringLiteral( "show_all_labels_when_no_candidates" ), img, 20 ) );
3276+
}
3277+
32153278
QGSTEST_MAIN( TestQgsLabelingEngine )
32163279
#include "testqgslabelingengine.moc"

0 commit comments

Comments
 (0)
Please sign in to comment.