[labeling] Fix missing labels when multilinestring features are used in

conjunction with the "merge connected lines" setting Refs #12173 (cherry picked from commit 7213030)
qgis · Jun 10, 2019 · 2ecda15 · 2ecda15
1 parent 9d317ea
commit 2ecda15
Show file tree

Hide file tree

Showing 5 changed files with 84 additions and 15 deletions.
diff --git a/python/core/auto_generated/qgspallabeling.sip.in b/python/core/auto_generated/qgspallabeling.sip.in
@@ -538,28 +538,30 @@ called to find out whether the layer is used for labeling
 %End
 
 
-    static QgsGeometry prepareGeometry( const QgsGeometry &geometry, QgsRenderContext &context, const QgsCoordinateTransform &ct, const QgsGeometry &clipGeometry = QgsGeometry() ) /Factory/;
+    static QgsGeometry prepareGeometry( const QgsGeometry &geometry, QgsRenderContext &context, const QgsCoordinateTransform &ct, const QgsGeometry &clipGeometry = QgsGeometry(), bool mergeLines = false ) /Factory/;
 %Docstring
 Prepares a geometry for registration with PAL. Handles reprojection, rotation, clipping, etc.
 
 :param geometry: geometry to prepare
 :param context: render context
 :param ct: coordinate transform, or invalid transform if no transformation required
 :param clipGeometry: geometry to clip features to, if applicable
+:param mergeLines: ``True`` if touching lines from this layer will be merged and treated as single features during labeling
 
 :return: prepared geometry
 
 .. versionadded:: 2.9
 %End
 
-    static bool geometryRequiresPreparation( const QgsGeometry &geometry, QgsRenderContext &context, const QgsCoordinateTransform &ct, const QgsGeometry &clipGeometry = QgsGeometry() );
+    static bool geometryRequiresPreparation( const QgsGeometry &geometry, QgsRenderContext &context, const QgsCoordinateTransform &ct, const QgsGeometry &clipGeometry = QgsGeometry(), bool mergeLines = false );
 %Docstring
 Checks whether a geometry requires preparation before registration with PAL
 
 :param geometry: geometry to prepare
 :param context: render context
 :param ct: coordinate transform, or invalid transform if no transformation required
 :param clipGeometry: geometry to clip features to, if applicable
+:param mergeLines: ``True`` if touching lines from this layer will be merged and treated as single features during labeling
 
 :return: true if geometry requires preparation
 

diff --git a/src/core/qgspallabeling.cpp b/src/core/qgspallabeling.cpp
@@ -1504,9 +1504,9 @@ void QgsPalLayerSettings::registerFeature( QgsFeature &f, QgsRenderContext &cont
   if ( geom.type() == QgsWkbTypes::PolygonGeometry && fitInPolygonOnly )
   {
     permissibleZone = geom;
-    if ( QgsPalLabeling::geometryRequiresPreparation( permissibleZone, context, ct, doClip ? extentGeom : QgsGeometry() ) )
+    if ( QgsPalLabeling::geometryRequiresPreparation( permissibleZone, context, ct, doClip ? extentGeom : QgsGeometry(), mergeLines ) )
     {
-      permissibleZone = QgsPalLabeling::prepareGeometry( permissibleZone, context, ct, doClip ? extentGeom : QgsGeometry() );
+      permissibleZone = QgsPalLabeling::prepareGeometry( permissibleZone, context, ct, doClip ? extentGeom : QgsGeometry(), mergeLines );
     }
   }
 
@@ -1519,9 +1519,9 @@ void QgsPalLayerSettings::registerFeature( QgsFeature &f, QgsRenderContext &cont
   }
 
   geos::unique_ptr geos_geom_clone;
-  if ( QgsPalLabeling::geometryRequiresPreparation( geom, context, ct, doClip ? extentGeom : QgsGeometry() ) )
+  if ( QgsPalLabeling::geometryRequiresPreparation( geom, context, ct, doClip ? extentGeom : QgsGeometry(), mergeLines ) )
   {
-    geom = QgsPalLabeling::prepareGeometry( geom, context, ct, doClip ? extentGeom : QgsGeometry() );
+    geom = QgsPalLabeling::prepareGeometry( geom, context, ct, doClip ? extentGeom : QgsGeometry(), mergeLines );
 
     if ( geom.isNull() )
       return;
@@ -1530,9 +1530,9 @@ void QgsPalLayerSettings::registerFeature( QgsFeature &f, QgsRenderContext &cont
 
   if ( isObstacle )
   {
-    if ( !obstacleGeometry.isNull() && QgsPalLabeling::geometryRequiresPreparation( obstacleGeometry, context, ct, doClip ? extentGeom : QgsGeometry() ) )
+    if ( !obstacleGeometry.isNull() && QgsPalLabeling::geometryRequiresPreparation( obstacleGeometry, context, ct, doClip ? extentGeom : QgsGeometry(), mergeLines ) )
     {
-      obstacleGeometry = QgsGeometry( QgsPalLabeling::prepareGeometry( obstacleGeometry, context, ct, doClip ? extentGeom : QgsGeometry() ) );
+      obstacleGeometry = QgsGeometry( QgsPalLabeling::prepareGeometry( obstacleGeometry, context, ct, doClip ? extentGeom : QgsGeometry(), mergeLines ) );
     }
   }
 
@@ -2029,9 +2029,9 @@ void QgsPalLayerSettings::registerObstacleFeature( QgsFeature &f, QgsRenderConte
   geos::unique_ptr geos_geom_clone;
   std::unique_ptr<QgsGeometry> scopedPreparedGeom;
 
-  if ( QgsPalLabeling::geometryRequiresPreparation( geom, context, ct, extentGeom ) )
+  if ( QgsPalLabeling::geometryRequiresPreparation( geom, context, ct, extentGeom, mergeLines ) )
   {
-    geom = QgsPalLabeling::prepareGeometry( geom, context, ct, extentGeom );
+    geom = QgsPalLabeling::prepareGeometry( geom, context, ct, extentGeom, mergeLines );
   }
   geos_geom_clone = QgsGeos::asGeos( geom );
 
@@ -2843,13 +2843,18 @@ bool QgsPalLabeling::staticWillUseLayer( QgsVectorLayer *layer )
 }
 
 
-bool QgsPalLabeling::geometryRequiresPreparation( const QgsGeometry &geometry, QgsRenderContext &context, const QgsCoordinateTransform &ct, const QgsGeometry &clipGeometry )
+bool QgsPalLabeling::geometryRequiresPreparation( const QgsGeometry &geometry, QgsRenderContext &context, const QgsCoordinateTransform &ct, const QgsGeometry &clipGeometry, bool mergeLines )
 {
   if ( geometry.isNull() )
   {
     return false;
   }
 
+  if ( geometry.type() == QgsWkbTypes::LineGeometry && geometry.isMultipart() && mergeLines )
+  {
+    return true;
+  }
+
   //requires reprojection
   if ( ct.isValid() && !ct.isShortCircuited() )
     return true;
@@ -2915,7 +2920,7 @@ QStringList QgsPalLabeling::splitToGraphemes( const QString &text )
   return graphemes;
 }
 
-QgsGeometry QgsPalLabeling::prepareGeometry( const QgsGeometry &geometry, QgsRenderContext &context, const QgsCoordinateTransform &ct, const QgsGeometry &clipGeometry )
+QgsGeometry QgsPalLabeling::prepareGeometry( const QgsGeometry &geometry, QgsRenderContext &context, const QgsCoordinateTransform &ct, const QgsGeometry &clipGeometry, bool mergeLines )
 {
   if ( geometry.isNull() )
   {
@@ -2925,6 +2930,11 @@ QgsGeometry QgsPalLabeling::prepareGeometry( const QgsGeometry &geometry, QgsRen
   //don't modify the feature's geometry so that geometry based expressions keep working
   QgsGeometry geom = geometry;
 
+  if ( geom.type() == QgsWkbTypes::LineGeometry && geom.isMultipart() && mergeLines )
+  {
+    geom = geom.mergeLines();
+  }
+
   //reproject the geometry if necessary
   if ( ct.isValid() && !ct.isShortCircuited() )
   {

diff --git a/src/core/qgspallabeling.h b/src/core/qgspallabeling.h
@@ -993,21 +993,23 @@ class CORE_EXPORT QgsPalLabeling
      * \param context render context
      * \param ct coordinate transform, or invalid transform if no transformation required
      * \param clipGeometry geometry to clip features to, if applicable
+     * \param mergeLines TRUE if touching lines from this layer will be merged and treated as single features during labeling
      * \returns prepared geometry
      * \since QGIS 2.9
      */
-    static QgsGeometry prepareGeometry( const QgsGeometry &geometry, QgsRenderContext &context, const QgsCoordinateTransform &ct, const QgsGeometry &clipGeometry = QgsGeometry() ) SIP_FACTORY;
+    static QgsGeometry prepareGeometry( const QgsGeometry &geometry, QgsRenderContext &context, const QgsCoordinateTransform &ct, const QgsGeometry &clipGeometry = QgsGeometry(), bool mergeLines = false ) SIP_FACTORY;
 
     /**
      * Checks whether a geometry requires preparation before registration with PAL
      * \param geometry geometry to prepare
      * \param context render context
      * \param ct coordinate transform, or invalid transform if no transformation required
      * \param clipGeometry geometry to clip features to, if applicable
+     * \param mergeLines TRUE if touching lines from this layer will be merged and treated as single features during labeling
      * \returns true if geometry requires preparation
      * \since QGIS 2.9
      */
-    static bool geometryRequiresPreparation( const QgsGeometry &geometry, QgsRenderContext &context, const QgsCoordinateTransform &ct, const QgsGeometry &clipGeometry = QgsGeometry() );
+    static bool geometryRequiresPreparation( const QgsGeometry &geometry, QgsRenderContext &context, const QgsCoordinateTransform &ct, const QgsGeometry &clipGeometry = QgsGeometry(), bool mergeLines = false );
 
     /**
      * Splits a \a text string to a list of separate lines, using a specified wrap character (\a wrapCharacter).

diff --git a/tests/src/core/testqgslabelingengine.cpp b/tests/src/core/testqgslabelingengine.cpp
@@ -53,6 +53,7 @@ class TestQgsLabelingEngine : public QObject
     void testRotateHidePartial();
     void testParallelLabelSmallFeature();
     void testAdjacentParts();
+    void testTouchingParts();
     void testLabelRotationWithReprojection();
 
   private:
@@ -809,7 +810,7 @@ void TestQgsLabelingEngine::testParallelLabelSmallFeature()
 
 void TestQgsLabelingEngine::testAdjacentParts()
 {
-  // test combination of map rotation with reprojected layer
+  // test polygon layer with multipart feature with adjacent parts
   QgsPalLayerSettings settings;
   setDefaultLabelParams( settings );
 
@@ -858,6 +859,60 @@ void TestQgsLabelingEngine::testAdjacentParts()
   QVERIFY( imageCheck( QStringLiteral( "label_adjacent_parts" ), img, 20 ) );
 }
 
+void TestQgsLabelingEngine::testTouchingParts()
+{
+  // test line layer with multipart feature with touching (but unmerged) parts
+  QgsPalLayerSettings settings;
+  setDefaultLabelParams( settings );
+
+  QgsTextFormat format = settings.format();
+  format.setSize( 20 );
+  format.setColor( QColor( 0, 0, 0 ) );
+  settings.setFormat( format );
+
+  settings.fieldName = QStringLiteral( "'XXXXXXXXXXXXXXXXXXXXXXXXXX'" );
+  settings.isExpression = true;
+  settings.placement = QgsPalLayerSettings::Curved;
+  settings.labelPerPart = false;
+  settings.mergeLines = true;
+
+  // if treated individually, none of these parts are long enough for the label to fit -- but the label should be rendered if the mergeLines setting is true,
+  // because the parts should be merged into a single linestring
+  std::unique_ptr< QgsVectorLayer> vl2( new QgsVectorLayer( QStringLiteral( "MultiLineString?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( "MultiLineString ((190000 5000010, 190050 5000000), (190050 5000000, 190100 5000000), (190200 5000000, 190150 5000000), (190150 5000000, 190100 5000000))" ) ) );
+  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<QgsMapLayer *>() << 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_multipart_touching_lines" ), img, 20 ) );
+}
+
 void TestQgsLabelingEngine::testLabelRotationWithReprojection()
 {
   // test combination of map rotation with reprojected layer

diff --git a/...cted_label_multipart_touching_lines/expected_label_multipart_touching_lines.png b/...cted_label_multipart_touching_lines/expected_label_multipart_touching_lines.png