[labeling] Fix repeated line labels gravitate to start of line

Avoids situations like 1. Line length of 3cm 2. Repeat distance of 2cm 3. Label size is 1.5 cm 2cm 1cm /--Label--/----/ i.e. the labels are off center and gravitate toward line starts Instead, we first calculate how many complete repeats we can fit in, and then divide the line into even sections of this length, avoiding the situation where some leftover segment of the line end isn't big enough for the label to fit.
qgis · Jul 25, 2019 · eac5cc4 · eac5cc4
1 parent 95eb77c
commit eac5cc4
Show file tree

Hide file tree

Showing 3 changed files with 83 additions and 5 deletions.
diff --git a/src/core/pal/layer.cpp b/src/core/pal/layer.cpp
@@ -383,20 +383,44 @@ void Layer::chopFeaturesAtRepeatDistance()
     std::unique_ptr< FeaturePart > fpart( mFeatureParts.takeFirst() );
     const GEOSGeometry *geom = fpart->geos();
     double chopInterval = fpart->repeatDistance();
-    bool shouldChop = false;
+
+    // whether we CAN chop
+    bool canChop = false;
+    double featureLen = 0;
     if ( chopInterval != 0. && GEOSGeomTypeId_r( geosctxt, geom ) == GEOS_LINESTRING )
     {
-      double featureLen = 0;
       ( void )GEOSLength_r( geosctxt, geom, &featureLen );
       if ( featureLen > chopInterval )
-        shouldChop = true;
+        canChop = true;
     }
 
-    if ( shouldChop )
+    // whether we SHOULD chop
+    bool shouldChop = canChop;
+    if ( canChop )
     {
-      QList<FeaturePart *> repeatParts;
+      // never chop into segments smaller than required for the actual label text
       chopInterval *= std::ceil( fpart->getLabelWidth() / fpart->repeatDistance() );
 
+      // now work out how many full segments we could chop this line into
+      int possibleSegments = static_cast< int >( std::floor( featureLen / chopInterval ) );
+
+      // ... and use this to work out the actual chop distance for this line. Otherwise, we risk the
+      // situation of:
+      // 1. Line length of 3cm
+      // 2. Repeat distance of 2cm
+      // 3. Label size is 1.5 cm
+      //
+      //      2cm    1cm
+      // /--Label--/----/
+      //
+      // i.e. the labels would be off center and gravitate toward line starts
+      chopInterval = featureLen / possibleSegments;
+
+      shouldChop = possibleSegments > 1;
+    }
+
+    if ( shouldChop )
+    {
       double bmin[2], bmax[2];
       fpart->getBoundingBox( bmin, bmax );
       mFeatureIndex->Remove( bmin, bmax, fpart.get() );

diff --git a/tests/src/core/testqgslabelingengine.cpp b/tests/src/core/testqgslabelingengine.cpp
@@ -60,6 +60,7 @@ class TestQgsLabelingEngine : public QObject
     void testCurvedLabelCorrectLinePlacement();
     void testCurvedLabelNegativeDistance();
     void testCurvedLabelOnSmallLineNearCenter();
+    void testRepeatDistanceWithSmallLine();
     void testParallelPlacementPreferAbove();
     void testLabelBoundary();
     void testLabelBlockingRegion();
@@ -1228,6 +1229,59 @@ void TestQgsLabelingEngine::testCurvedLabelOnSmallLineNearCenter()
   QVERIFY( imageCheck( QStringLiteral( "label_curved_small_feature_centered" ), img, 20 ) );
 }
 
+void TestQgsLabelingEngine::testRepeatDistanceWithSmallLine()
+{
+  // test a small line relative to label size still gives sufficient candidates to ensure more centered placements
+  // are found
+  QgsPalLayerSettings settings;
+  setDefaultLabelParams( settings );
+
+  QgsTextFormat format = settings.format();
+  format.setSize( 20 );
+  format.setColor( QColor( 0, 0, 0 ) );
+  settings.setFormat( format );
+
+  settings.fieldName = QStringLiteral( "'XXXXXXX'" );
+  settings.isExpression = true;
+  settings.placement = QgsPalLayerSettings::Curved;
+  settings.labelPerPart = false;
+  settings.repeatDistance = 55;
+
+  std::unique_ptr< QgsVectorLayer> vl2( new QgsVectorLayer( QStringLiteral( "LineString?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( "LineString (190050 5000000, 190150 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( QgsRectangle( 190000, 5000000, 190200, 5000010 ) );
+  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_repeat_distance_with_small_line" ), img, 20 ) );
+}
+
 void TestQgsLabelingEngine::testParallelPlacementPreferAbove()
 {
   // given the choice of above or below placement, labels should always be placed above

diff --git a/...eat_distance_with_small_line/expected_label_repeat_distance_with_small_line.png b/...eat_distance_with_small_line/expected_label_repeat_distance_with_small_line.png