From 4cb1213efd245ea05b166950d050e1780c340c9f Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Tue, 28 May 2019 11:37:32 +1000 Subject: [PATCH] Fix labeling ignores "label per part" setting when geometry parts are adjacent Fixes #26763 --- src/core/qgspallabeling.cpp | 35 ++++++++++-- tests/src/core/testqgslabelingengine.cpp | 52 ++++++++++++++++++ .../expected_label_adjacent_parts.png | Bin 0 -> 2115 bytes 3 files changed, 81 insertions(+), 6 deletions(-) create mode 100644 tests/testdata/control_images/labelingengine/expected_label_adjacent_parts/expected_label_adjacent_parts.png diff --git a/src/core/qgspallabeling.cpp b/src/core/qgspallabeling.cpp index 13ff522a57b..0432e6ce948 100644 --- a/src/core/qgspallabeling.cpp +++ b/src/core/qgspallabeling.cpp @@ -60,6 +60,7 @@ #include "qgsmaptopixelgeometrysimplifier.h" #include "qgscurvepolygon.h" #include "qgsmessagelog.h" +#include "qgsgeometrycollection.h" #include using namespace pal; @@ -3115,15 +3116,37 @@ QgsGeometry QgsPalLabeling::prepareGeometry( const QgsGeometry &geometry, QgsRen } // fix invalid polygons - if ( geom.type() == QgsWkbTypes::PolygonGeometry && !geom.isGeosValid() ) + if ( geom.type() == QgsWkbTypes::PolygonGeometry ) { - QgsGeometry bufferGeom = geom.buffer( 0, 0 ); - if ( bufferGeom.isNull() ) + if ( geom.isMultipart() ) { - QgsDebugMsg( QStringLiteral( "Could not repair geometry: %1" ).arg( bufferGeom.lastError() ) ); - return QgsGeometry(); + // important -- we need to treat ever part in isolation here. We can't test the validity of the whole geometry + // at once, because touching parts would result in an invalid geometry, and buffering this "dissolves" the parts. + // because the actual label engine treats parts as separate entities, we aren't bound by the usual "touching parts are invalid" rule + // see https://github.com/qgis/QGIS/issues/26763 + QVector< QgsGeometry> parts; + parts.reserve( qgsgeometry_cast< const QgsGeometryCollection * >( geom.constGet() )->numGeometries() ); + for ( auto it = geom.const_parts_begin(); it != geom.const_parts_end(); ++it ) + { + QgsGeometry partGeom( ( *it )->clone() ); + if ( !partGeom.isGeosValid() ) + { + partGeom = partGeom.buffer( 0, 0 ); + } + parts.append( partGeom ); + } + geom = QgsGeometry::collectGeometry( parts ); + } + else if ( !geom.isGeosValid() ) + { + QgsGeometry bufferGeom = geom.buffer( 0, 0 ); + if ( bufferGeom.isNull() ) + { + QgsDebugMsg( QStringLiteral( "Could not repair geometry: %1" ).arg( bufferGeom.lastError() ) ); + return QgsGeometry(); + } + geom = bufferGeom; } - geom = bufferGeom; } if ( !clipGeometry.isNull() && diff --git a/tests/src/core/testqgslabelingengine.cpp b/tests/src/core/testqgslabelingengine.cpp index d686c74142b..00c3efba177 100644 --- a/tests/src/core/testqgslabelingengine.cpp +++ b/tests/src/core/testqgslabelingengine.cpp @@ -52,6 +52,7 @@ class TestQgsLabelingEngine : public QObject void testRegisterFeatureUnprojectible(); void testRotateHidePartial(); void testParallelLabelSmallFeature(); + void testAdjacentParts(); void testLabelBoundary(); void testLabelBlockingRegion(); @@ -807,6 +808,57 @@ void TestQgsLabelingEngine::testParallelLabelSmallFeature() // QVERIFY( imageCheck( "label_rotate_hide_partial", img, 20 ) ); } +void TestQgsLabelingEngine::testAdjacentParts() +{ + // test combination of map rotation with reprojected layer + QgsPalLayerSettings settings; + setDefaultLabelParams( settings ); + + QgsTextFormat format = settings.format(); + format.setSize( 20 ); + format.setColor( QColor( 0, 0, 0 ) ); + settings.setFormat( format ); + + settings.fieldName = QStringLiteral( "'X'" ); + settings.isExpression = true; + settings.placement = QgsPalLayerSettings::OverPoint; + settings.labelPerPart = true; + + std::unique_ptr< QgsVectorLayer> vl2( new QgsVectorLayer( QStringLiteral( "Polygon?crs=epsg:3946&field=id:integer" ), QStringLiteral( "vl" ), QStringLiteral( "memory" ) ) ); + vl2->setRenderer( new QgsNullSymbolRenderer() ); + + QgsFeature f; + f.setAttributes( QgsAttributes() << 1 ); + f.setGeometry( QgsGeometry::fromWkt( QStringLiteral( "MultiPolygon (((1967901.6872910603415221 5162590.11975561361759901, 1967905.31832842249423265 5162591.80023225769400597, 1967907.63076798897236586 5162586.43503414187580347, 1967903.84105980419553816 5162584.57283254805952311, 1967901.6872910603415221 5162590.11975561361759901)),((1967901.64785283687524498 5162598.3270823871716857, 1967904.82891705213114619 5162601.06552503909915686, 1967910.82140435534529388 5162587.99774718284606934, 1967907.63076798897236586 5162586.43503414187580347, 1967905.31832842249423265 5162591.80023225769400597, 1967901.6872910603415221 5162590.11975561361759901, 1967899.27472299290820956 5162596.28855143301188946, 1967901.64785283687524498 5162598.3270823871716857)),((1967904.82891705213114619 5162601.06552503909915686, 1967901.64785283687524498 5162598.3270823871716857, 1967884.28552994946949184 5162626.09785370342433453, 1967895.81538487318903208 5162633.84423183929175138, 1967901.64141261484473944 5162624.63927845563739538, 1967906.47453573765233159 5162616.87410452589392662, 1967913.7844126324634999 5162604.47178338281810284, 1967909.58057221467606723 5162602.89022256527096033, 1967904.82891705213114619 5162601.06552503909915686)))" ) ) ); + QVERIFY( vl2->dataProvider()->addFeature( f ) ); + + vl2->setLabeling( new QgsVectorLayerSimpleLabeling( settings ) ); // TODO: this should not be necessary! + vl2->setLabelsEnabled( true ); + + // make a fake render context + QSize size( 640, 480 ); + QgsMapSettings mapSettings; + mapSettings.setDestinationCrs( vl2->crs() ); + + mapSettings.setOutputSize( size ); + mapSettings.setExtent( f.geometry().boundingBox() ); + mapSettings.setLayers( QList() << vl2.get() ); + mapSettings.setOutputDpi( 96 ); + + QgsLabelingEngineSettings engineSettings = mapSettings.labelingEngineSettings(); + engineSettings.setFlag( QgsLabelingEngineSettings::UsePartialCandidates, false ); + engineSettings.setFlag( QgsLabelingEngineSettings::DrawLabelRectOnly, true ); + //engineSettings.setFlag( QgsLabelingEngineSettings::DrawCandidates, true ); + mapSettings.setLabelingEngineSettings( engineSettings ); + + QgsMapRendererSequentialJob job( mapSettings ); + job.start(); + job.waitForFinished(); + + QImage img = job.renderedImage(); + QVERIFY( imageCheck( QStringLiteral( "label_adjacent_parts" ), img, 20 ) ); +} + void TestQgsLabelingEngine::testLabelBoundary() { // test that no labels are drawn outside of the specified label boundary diff --git a/tests/testdata/control_images/labelingengine/expected_label_adjacent_parts/expected_label_adjacent_parts.png b/tests/testdata/control_images/labelingengine/expected_label_adjacent_parts/expected_label_adjacent_parts.png new file mode 100644 index 0000000000000000000000000000000000000000..6dadd1dc8ab6efb20372f00d02818fc26e381683 GIT binary patch literal 2115 zcmeAS@N?(olHy`uVBq!ia0y~yU}|7sV0^&A1Qgk|*?TjP;wb| z85r2Vdb&7VZH@&q8X0Fc7Pd6rI&jlnVd|Pa(np_v`G4;E z-I;sOSh6#GsNZ{)nc;ybrvU>Civ$A`6AuF;BU?iQ1G9re1EYe%0R{nq1O^U{QR&fO zAtyOBE|p`bd2E&!|2X^mPpfOccgXKxx0u6lSie;E`rYlfZ}7!1{b7l}FfgesVI@1m z{oBXF{2A&lq@Ih+XK0xIm|JQz6O3m5AzJ8dxWe49|AASa|GonG@6OW>L~?MCkhA~lW!Q>=qZfj@%%A8*v|?)!Q`wSa57W%^!5hPXcc3@X+wAY10#KK`TP z*`MMXxvebcTpJqGV~>~Kf0%637t5snXrS`8&R2#9cdl*BOTJxO{YJuK;^{e??=djw h9~U>Fw9@^@C{d)`Ym&Wc1F%)V;OXk;vd$@?2>@mSZhim& literal 0 HcmV?d00001