Merge pull request #42654 from nyalldawson/pal_again

Fix some valid curved label placements are rejected

src/core/labeling/qgstextlabelfeature.cpp

@@ -75,8 +75,8 @@ void QgsTextLabelFeature::calculateInfo( bool curvedLabeling, QFontMetricsF *fm,
     maxoutangle = -95.0;
 
   // create label info!
-  double mapScale = xform->mapUnitsPerPixel();
-  double labelHeight = mapScale * fm->height();
+  const double mapScale = xform->mapUnitsPerPixel();
+  const double characterHeight = mapScale * fm->height();
 
   // mLetterSpacing/mWordSpacing = 0.0 is default for non-curved labels
   // (non-curved spacings handled by Qt in QgsPalLayerSettings/QgsPalLabeling)
@@ -104,7 +104,7 @@ void QgsTextLabelFeature::calculateInfo( bool curvedLabeling, QFontMetricsF *fm,
     mClusters = QgsPalLabeling::splitToGraphemes( mLabelText );
   }
 
-  mInfo = new pal::LabelInfo( mClusters.count(), labelHeight, maxinangle, maxoutangle );
+  std::vector< double > characterWidths( mClusters.count() );
   for ( int i = 0; i < mClusters.count(); i++ )
   {
     // reconstruct how Qt creates word spacing, then adjust per individual stored character
@@ -131,9 +131,9 @@ void QgsTextLabelFeature::calculateInfo( bool curvedLabeling, QFontMetricsF *fm,
       charWidth = fm->horizontalAdvance( QString( mClusters[i] ) ) + wordSpaceFix;
     }
 
-    double labelWidth = mapScale * charWidth;
-    mInfo->char_info[i].width = labelWidth;
+    characterWidths[i] = mapScale * charWidth;
   }
+  mInfo = new pal::LabelInfo( characterHeight, std::move( characterWidths ), maxinangle, maxoutangle );
 }
 
 QgsTextDocument QgsTextLabelFeature::document() const

src/core/pal/feature.cpp

@@ -1222,28 +1222,27 @@ std::size_t FeaturePart::createCandidatesAlongLineNearMidpoint( std::vector< std
   return lPos.size();
 }
 
-
-std::unique_ptr< LabelPosition > FeaturePart::curvedPlacementAtOffset( PointSet *path_positions, double *path_distances, int &orientation, const double offsetAlongLine, bool &reversed, bool &flip, bool applyAngleConstraints )
+std::unique_ptr< LabelPosition > FeaturePart::curvedPlacementAtOffset( PointSet *mapShape, const std::vector< double> &pathDistances, int &orientation, const double offsetAlongLine, bool &reversed, bool &flip, bool applyAngleConstraints )
 {
   double offsetAlongSegment = offsetAlongLine;
   int index = 1;
   // Find index of segment corresponding to starting offset
-  while ( index < path_positions->nbPoints && offsetAlongSegment > path_distances[index] )
+  while ( index < mapShape->nbPoints && offsetAlongSegment > pathDistances[index] )
   {
-    offsetAlongSegment -= path_distances[index];
+    offsetAlongSegment -= pathDistances[index];
     index += 1;
   }
-  if ( index >= path_positions->nbPoints )
+  if ( index >= mapShape->nbPoints )
   {
     return nullptr;
   }
 
   LabelInfo *li = mLF->curvedLabelInfo();
 
-  double string_height = li->label_height;
+  const double characterHeight = li->characterHeight;
 
-  const double segment_length = path_distances[index];
-  if ( qgsDoubleNear( segment_length, 0.0 ) )
+  const double segmentLength = pathDistances[index];
+  if ( qgsDoubleNear( segmentLength, 0.0 ) )
   {
     // Not allowed to place across on 0 length segments or discontinuities
     return nullptr;
@@ -1260,11 +1259,12 @@ std::unique_ptr< LabelPosition > FeaturePart::curvedPlacementAtOffset( PointSet
     double startLabelY = 0;
     double endLabelX = 0;
     double endLabelY = 0;
-    for ( int i = 0; i < li->char_num; i++ )
+    const int characterCount = li->count();
+    for ( int i = 0; i < characterCount; i++ )
     {
-      LabelInfo::CharacterInfo &ci = li->char_info[i];
+      const double characterWidth = li->characterWidth( i );
       double characterStartX, characterStartY;
-      if ( !nextCharPosition( ci.width, path_distances[endindex], path_positions, endindex, _distance, characterStartX, characterStartY, endLabelX, endLabelY ) )
+      if ( !nextCharPosition( characterWidth, pathDistances[endindex], mapShape, endindex, _distance, characterStartX, characterStartY, endLabelX, endLabelY ) )
       {
         return nullptr;
       }
@@ -1298,29 +1298,30 @@ std::unique_ptr< LabelPosition > FeaturePart::curvedPlacementAtOffset( PointSet
   std::unique_ptr< LabelPosition > slp;
   LabelPosition *slp_tmp = nullptr;
 
-  double old_x = path_positions->x[index - 1];
-  double old_y = path_positions->y[index - 1];
+  double old_x = mapShape->x[index - 1];
+  double old_y = mapShape->y[index - 1];
 
-  double new_x = path_positions->x[index];
-  double new_y = path_positions->y[index];
+  double new_x = mapShape->x[index];
+  double new_y = mapShape->y[index];
 
   double dx = new_x - old_x;
   double dy = new_y - old_y;
 
   double angle = std::atan2( -dy, dx );
 
-  for ( int i = 0; i < li->char_num; i++ )
+  const int characterCount = li->count();
+  for ( int i = 0; i < characterCount; i++ )
   {
     double last_character_angle = angle;
 
     // grab the next character according to the orientation
-    LabelInfo::CharacterInfo &ci = ( orientation > 0 ? li->char_info[i] : li->char_info[li->char_num - i - 1] );
-    if ( qgsDoubleNear( ci.width, 0.0 ) )
+    const double characterWidth = ( orientation > 0 ? li->characterWidth( i ) : li->characterWidth( characterCount - i - 1 ) );
+    if ( qgsDoubleNear( characterWidth, 0.0 ) )
       // Certain scripts rely on zero-width character, skip those to prevent failure (see #15801)
       continue;
 
     double start_x, start_y, end_x, end_y;
-    if ( !nextCharPosition( ci.width, path_distances[index], path_positions, index, offsetAlongSegment, start_x, start_y, end_x, end_y ) )
+    if ( !nextCharPosition( characterWidth, pathDistances[index], mapShape, index, offsetAlongSegment, start_x, start_y, end_x, end_y ) )
     {
       return nullptr;
     }
@@ -1331,21 +1332,23 @@ std::unique_ptr< LabelPosition > FeaturePart::curvedPlacementAtOffset( PointSet
     // Test last_character_angle vs angle
     // since our rendering angle has changed then check against our
     // max allowable angle change.
-    double angle_delta = last_character_angle - angle;
+    double angleDelta = last_character_angle - angle;
     // normalise between -180 and 180
-    while ( angle_delta > M_PI ) angle_delta -= 2 * M_PI;
-    while ( angle_delta < -M_PI ) angle_delta += 2 * M_PI;
-    if ( applyAngleConstraints && ( ( li->max_char_angle_inside > 0 && angle_delta > 0
-                                      && angle_delta > li->max_char_angle_inside * ( M_PI / 180 ) )
-                                    || ( li->max_char_angle_outside < 0 && angle_delta < 0
-                                         && angle_delta < li->max_char_angle_outside * ( M_PI / 180 ) ) ) )
+    while ( angleDelta > M_PI )
+      angleDelta -= 2 * M_PI;
+    while ( angleDelta < -M_PI )
+      angleDelta += 2 * M_PI;
+    if ( applyAngleConstraints && ( ( li->maxCharAngleInsideRadians > 0 && angleDelta > 0
+                                      && angleDelta > li->maxCharAngleInsideRadians )
+                                    || ( li->maxCharAngleOutsideRadians < 0 && angleDelta < 0
+                                         && angleDelta < li->maxCharAngleOutsideRadians ) ) )
     {
       return nullptr;
     }
 
     // Shift the character downwards since the draw position is specified at the baseline
     // and we're calculating the mean line here
-    double dist = 0.9 * li->label_height / 2;
+    double dist = 0.9 * li->characterHeight / 2;
     if ( orientation < 0 )
     {
       dist = -dist;
@@ -1366,13 +1369,13 @@ std::unique_ptr< LabelPosition > FeaturePart::curvedPlacementAtOffset( PointSet
     if ( orientation < 0 )
     {
       // rotate in place
-      render_x += ci.width * std::cos( render_angle ); //- (string_height-2)*sin(render_angle);
-      render_y -= ci.width * std::sin( render_angle ); //+ (string_height-2)*cos(render_angle);
+      render_x += characterWidth * std::cos( render_angle ); //- (string_height-2)*sin(render_angle);
+      render_y -= characterWidth * std::sin( render_angle ); //+ (string_height-2)*cos(render_angle);
       render_angle += M_PI;
     }
 
-    std::unique_ptr< LabelPosition > tmp = std::make_unique< LabelPosition >( 0, render_x /*- xBase*/, render_y /*- yBase*/, ci.width, string_height, -render_angle, 0.0001, this, false, LabelPosition::QuadrantOver );
-    tmp->setPartId( orientation > 0 ? i : li->char_num - i - 1 );
+    std::unique_ptr< LabelPosition > tmp = std::make_unique< LabelPosition >( 0, render_x /*- xBase*/, render_y /*- yBase*/, characterWidth, characterHeight, -render_angle, 0.0001, this, false, LabelPosition::QuadrantOver );
+    tmp->setPartId( orientation > 0 ? i : characterCount - i - 1 );
     LabelPosition *next = tmp.get();
     if ( !slp )
       slp = std::move( tmp );
@@ -1401,17 +1404,20 @@ static std::unique_ptr< LabelPosition > _createCurvedCandidate( LabelPosition *l
 std::size_t FeaturePart::createCurvedCandidatesAlongLine( std::vector< std::unique_ptr< LabelPosition > > &lPos, PointSet *mapShape, bool allowOverrun, Pal *pal )
 {
   LabelInfo *li = mLF->curvedLabelInfo();
-
   // label info must be present
-  if ( !li || li->char_num == 0 )
+  if ( !li )
+    return 0;
+
+  const int characterCount = li->count();
+  if ( characterCount == 0 )
     return 0;
 
   // TODO - we may need an explicit penalty for overhanging labels. Currently, they are penalized just because they
   // are further from the line center, so non-overhanding placements are picked where possible.
 
   double totalCharacterWidth = 0;
-  for ( int i = 0; i < li->char_num; ++i )
-    totalCharacterWidth += li->char_info[ i ].width;
+  for ( int i = 0; i < characterCount; ++i )
+    totalCharacterWidth += li->characterWidth( i );
 
   std::unique_ptr< PointSet > expanded;
   double shapeLength = mapShape->length();
@@ -1442,7 +1448,7 @@ std::size_t FeaturePart::createCurvedCandidatesAlongLine( std::vector< std::uniq
   }
 
   // distance calculation
-  std::unique_ptr< double [] > path_distances = std::make_unique<double[]>( mapShape->nbPoints );
+  std::vector< double > path_distances( mapShape->nbPoints );
   double total_distance = 0;
   double old_x = -1.0, old_y = -1.0;
   for ( int i = 0; i < mapShape->nbPoints; i++ )
@@ -1469,7 +1475,7 @@ std::size_t FeaturePart::createCurvedCandidatesAlongLine( std::vector< std::uniq
 
   std::vector< std::unique_ptr< LabelPosition >> positions;
   const std::size_t candidateTargetCount = maximumLineCandidates();
-  double delta = std::max( li->label_height / 6, total_distance / candidateTargetCount );
+  double delta = std::max( li->characterHeight / 6, total_distance / candidateTargetCount );
 
   QgsLabeling::LinePlacementFlags flags = mLF->arrangementFlags();
   if ( flags == 0 )
@@ -1507,18 +1513,18 @@ std::size_t FeaturePart::createCurvedCandidatesAlongLine( std::vector< std::uniq
       orientation = 1;
     }
 
-    std::unique_ptr< LabelPosition > slp = curvedPlacementAtOffset( mapShape, path_distances.get(), orientation, distanceAlongLineToStartCandidate, reversed, flip, !singleCandidateOnly );
+    std::unique_ptr< LabelPosition > slp = curvedPlacementAtOffset( mapShape, path_distances, orientation, distanceAlongLineToStartCandidate, reversed, flip, !singleCandidateOnly );
     if ( !slp )
       continue;
 
     // If we placed too many characters upside down
-    if ( slp->upsideDownCharCount() >= li->char_num / 2.0 )
+    if ( slp->upsideDownCharCount() >= characterCount / 2.0 )
     {
       // if labels should be shown upright then retry with the opposite orientation
       if ( ( showUprightLabels() && !flip ) )
       {
         orientation = -orientation;
-        slp = curvedPlacementAtOffset( mapShape, path_distances.get(), orientation, distanceAlongLineToStartCandidate, reversed, flip, !singleCandidateOnly );
+        slp = curvedPlacementAtOffset( mapShape, path_distances, orientation, distanceAlongLineToStartCandidate, reversed, flip, !singleCandidateOnly );
       }
     }
     if ( !slp )
@@ -1547,7 +1553,7 @@ std::size_t FeaturePart::createCurvedCandidatesAlongLine( std::vector< std::uniq
     // if anchor placement is towards start or end of line, we need to slightly tweak the costs to ensure that the
     // anchor weighting is sufficient to push labels towards start/end
     const bool anchorIsFlexiblePlacement = !singleCandidateOnly && mLF->lineAnchorPercent() > 0.1 && mLF->lineAnchorPercent() < 0.9;
-    double angle_diff_avg = li->char_num > 1 ? ( angle_diff / ( li->char_num - 1 ) ) : 0; // <0, pi> but pi/8 is much already
+    double angle_diff_avg = characterCount > 1 ? ( angle_diff / ( characterCount - 1 ) ) : 0; // <0, pi> but pi/8 is much already
     double cost = angle_diff_avg / 100; // <0, 0.031 > but usually <0, 0.003 >
     if ( cost < 0.0001 )
       cost = 0.0001;
@@ -1559,14 +1565,14 @@ std::size_t FeaturePart::createCurvedCandidatesAlongLine( std::vector< std::uniq
     slp->setCost( cost );
 
     // average angle is calculated with respect to periodicity of angles
-    double angle_avg = std::atan2( sin_avg / li->char_num, cos_avg / li->char_num );
+    double angle_avg = std::atan2( sin_avg / characterCount, cos_avg / characterCount );
     bool localreversed = flip ? !reversed : reversed;
     // displacement - we loop through 3 times, generating above, online then below line placements successively
     for ( int i = 0; i <= 2; ++i )
     {
       std::unique_ptr< LabelPosition > p;
       if ( i == 0 && ( ( !localreversed && ( flags & QgsLabeling::LinePlacementFlag::AboveLine ) ) || ( localreversed && ( flags & QgsLabeling::LinePlacementFlag::BelowLine ) ) ) )
-        p = _createCurvedCandidate( slp.get(), angle_avg, mLF->distLabel() + li->label_height / 2 );
+        p = _createCurvedCandidate( slp.get(), angle_avg, mLF->distLabel() + li->characterHeight / 2 );
       if ( i == 1 && flags & QgsLabeling::LinePlacementFlag::OnLine )
       {
         p = _createCurvedCandidate( slp.get(), angle_avg, 0 );
@@ -1574,7 +1580,7 @@ std::size_t FeaturePart::createCurvedCandidatesAlongLine( std::vector< std::uniq
       }
       if ( i == 2 && ( ( !localreversed && ( flags & QgsLabeling::LinePlacementFlag::BelowLine ) ) || ( localreversed && ( flags & QgsLabeling::LinePlacementFlag::AboveLine ) ) ) )
       {
-        p = _createCurvedCandidate( slp.get(), angle_avg, -li->label_height / 2 - mLF->distLabel() );
+        p = _createCurvedCandidate( slp.get(), angle_avg, -li->characterHeight / 2 - mLF->distLabel() );
         p->setCost( p->cost() + 0.001 );
       }
 

src/core/pal/feature.h

@@ -55,32 +55,59 @@ namespace pal
   class CORE_EXPORT LabelInfo
   {
     public:
-      struct CharacterInfo
-      {
-        double width;
-      };
 
-      LabelInfo( int num, double height, double maxinangle = 20.0, double maxoutangle = -20.0 )
+      /**
+       * Constructor for LabelInfo
+       * \param characterHeight height of characters
+       * \param characterWidths vector of character widths
+       * \param maxinangle maximum acceptable in angle (in degrees)
+       * \param maxoutangle maximum acceptable out angle (in degrees)
+       */
+      LabelInfo( double characterHeight, std::vector< double > characterWidths, double maxinangle = 20.0, double maxoutangle = -20.0 )
+        : maxCharAngleInsideRadians( maxinangle * M_PI / 180 )
+          // outside angle should be negative
+        , maxCharAngleOutsideRadians( ( maxoutangle > 0 ? -maxoutangle : maxoutangle ) * M_PI / 180 )
+        , characterHeight( characterHeight )
+        , mCharacterWidths( characterWidths )
       {
-        max_char_angle_inside = maxinangle;
-        // outside angle should be negative
-        max_char_angle_outside = maxoutangle > 0 ? -maxoutangle : maxoutangle;
-        label_height = height;
-        char_num = num;
-        char_info = new CharacterInfo[num];
       }
-      ~LabelInfo() { delete [] char_info; }
 
       //! LabelInfo cannot be copied
       LabelInfo( const LabelInfo &rh ) = delete;
       //! LabelInfo cannot be copied
       LabelInfo &operator=( const LabelInfo &rh ) = delete;
 
-      double max_char_angle_inside;
-      double max_char_angle_outside;
-      double label_height;
-      int char_num;
-      CharacterInfo *char_info = nullptr;
+      /**
+       * Maximum angle between inside curved label characters (in radians).
+       * \see maxCharAngleOutsideRadians
+       */
+      double maxCharAngleInsideRadians = 0;
+
+      /**
+       * Maximum angle between outside curved label characters (in radians).
+       * \see maxCharAngleInsideRadians
+       */
+      double maxCharAngleOutsideRadians = 0;
+
+      // TODO - maybe individual character height would give better results!
+
+      /**
+       * Character height (actually font metrics height, not individual character height).
+       */
+      double characterHeight = 0;
+
+      /**
+       * Returns the total number of characters.
+       */
+      int count() const { return static_cast< int >( mCharacterWidths.size() ); }
+
+      /**
+       * Returns the width of the character at the specified position.
+       */
+      double characterWidth( int position ) const { return mCharacterWidths[position]; }
+
+    private:
+      std::vector< double > mCharacterWidths;
 
   };
 
@@ -238,8 +265,8 @@ namespace pal
 
       /**
        * Returns the label position for a curved label at a specific offset along a path.
-       * \param path_positions line path to place label on
-       * \param path_distances array of distances to each segment on path
+       * \param mapShape line path to place label on
+       * \param pathDistances array of distances to each segment on path
        * \param orientation can be 0 for automatic calculation of orientation, or -1/+1 for a specific label orientation
        * \param distance distance to offset label along curve by
        * \param reversed if TRUE label is reversed from lefttoright to righttoleft
@@ -247,7 +274,7 @@ namespace pal
        * \param applyAngleConstraints TRUE if label feature character angle constraints should be applied
        * \returns calculated label position
        */
-      std::unique_ptr< LabelPosition > curvedPlacementAtOffset( PointSet *path_positions, double *path_distances,
+      std::unique_ptr< LabelPosition > curvedPlacementAtOffset( PointSet *mapShape, const std::vector<double> &pathDistances,
           int &orientation, double distance, bool &reversed, bool &flip, bool applyAngleConstraints );
 
       /**

src/core/pal/geomfunction.cpp

@@ -378,6 +378,21 @@ void GeomFunction::findLineCircleIntersection( double cx, double cy, double radi
     double x1, double y1, double x2, double y2,
     double &xRes, double &yRes )
 {
+  double multiplier = 1;
+  if ( radius < 10 )
+  {
+    // these calculations get unstable for small coordinates differences, e.g. as a result of map labeling in a geographic
+    // CRS
+    multiplier = 10000;
+    x1 *= multiplier;
+    y1 *= multiplier;
+    x2 *= multiplier;
+    y2 *= multiplier;
+    cx *= multiplier;
+    cy *= multiplier;
+    radius *= multiplier;
+  }
+
   double dx = x2 - x1;
   double dy = y2 - y1;
 
@@ -406,4 +421,10 @@ void GeomFunction::findLineCircleIntersection( double cx, double cy, double radi
     xRes = x1 + t * dx;
     yRes = y1 + t * dy;
   }
+
+  if ( multiplier != 1 )
+  {
+    xRes /= multiplier;
+    yRes /= multiplier;
+  }
 }