Fix labeling hide partial labels setting when map is rotated

Previously partially hidden labels would be shown when a map is
rotated, even if the project was set to hide partial labels.

To achieve this pal has been refactored to allow an arbitrary
QgsGeometry specifying the map boundary (instead of the previous
rectangular only extents). The good news is that this paves the
way for a future release to have non-rectangular layout map items,
where the actual map item shape will be correctly handled by
the labeling engine...

Author: nyalldawson

src/core/pal/feature.cpp

@@ -1512,16 +1512,9 @@ int FeaturePart::createCandidatesForPolygon( QList< LabelPosition *> &lPos, Poin
 }
 
 int FeaturePart::createCandidates( QList< LabelPosition *> &lPos,
-                                   double bboxMin[2], double bboxMax[2],
+                                   const GEOSPreparedGeometry *mapBoundary,
                                    PointSet *mapShape, RTree<LabelPosition *, double, 2, double> *candidates )
 {
-  double bbox[4];
-
-  bbox[0] = bboxMin[0];
-  bbox[1] = bboxMin[1];
-  bbox[2] = bboxMax[0];
-  bbox[3] = bboxMax[1];
-
   double angle = mLF->hasFixedAngle() ? mLF->fixedAngle() : 0.0;
 
   if ( mLF->hasFixedPosition() )
@@ -1579,10 +1572,11 @@ int FeaturePart::createCandidates( QList< LabelPosition *> &lPos,
   {
     LabelPosition *pos = i.next();
     bool outside = false;
+
     if ( mLF->layer()->pal->getShowPartial() )
-      outside = !pos->isIntersect( bbox );
+      outside = !pos->intersects( mapBoundary );
     else
-      outside = !pos->isInside( bbox );
+      outside = !pos->within( mapBoundary );
     if ( outside )
     {
       i.remove();

src/core/pal/feature.h

@@ -130,13 +130,12 @@ namespace pal
       /**
        * Generic method to generate label candidates for the feature.
        * \param lPos pointer to an array of candidates, will be filled by generated candidates
-       * \param bboxMin min values of the map extent
-       * \param bboxMax max values of the map extent
+       * \param mapBoundary map boundary geometry
        * \param mapShape generate candidates for this spatial entity
        * \param candidates index for candidates
        * \returns the number of candidates generated in lPos
        */
-      int createCandidates( QList<LabelPosition *> &lPos, double bboxMin[2], double bboxMax[2], PointSet *mapShape, RTree<LabelPosition *, double, 2, double> *candidates );
+      int createCandidates( QList<LabelPosition *> &lPos, const GEOSPreparedGeometry *mapBoundary, PointSet *mapShape, RTree<LabelPosition *, double, 2, double> *candidates );
 
       /**
        * Generate candidates for point feature, located around a specified point.

src/core/pal/pal.cpp

@@ -124,8 +124,7 @@ typedef struct _featCbackCtx
   QLinkedList<Feats *> *fFeats;
   RTree<FeaturePart *, double, 2, double> *obstacles;
   RTree<LabelPosition *, double, 2, double> *candidates;
-  double bbox_min[2];
-  double bbox_max[2];
+  const GEOSPreparedGeometry *mapBoundary = nullptr;
 } FeatCallBackCtx;
 
 
@@ -154,7 +153,7 @@ bool extractFeatCallback( FeaturePart *ft_ptr, void *ctx )
 
   // generate candidates for the feature part
   QList< LabelPosition * > lPos;
-  if ( ft_ptr->createCandidates( lPos, context->bbox_min, context->bbox_max, ft_ptr, context->candidates ) )
+  if ( ft_ptr->createCandidates( lPos, context->mapBoundary, ft_ptr, context->candidates ) )
   {
     // valid features are added to fFeats
     Feats *ft = new Feats();
@@ -241,23 +240,25 @@ std::unique_ptr<Problem> Pal::extract( const QgsRectangle &extent, const QgsGeom
 
   LabelPosition *lp = nullptr;
 
-  bbx[0] = bbx[3] = amin[0] = prob->bbox[0] = lambda_min;
-  bby[0] = bby[1] = amin[1] = prob->bbox[1] = phi_min;
-  bbx[1] = bbx[2] = amax[0] = prob->bbox[2] = lambda_max;
-  bby[2] = bby[3] = amax[1] = prob->bbox[3] = phi_max;
+  bbx[0] = bbx[3] = amin[0] = prob->bbox[0] = extent.xMinimum();
+  bby[0] = bby[1] = amin[1] = prob->bbox[1] = extent.yMinimum();
+  bbx[1] = bbx[2] = amax[0] = prob->bbox[2] = extent.xMaximum();
+  bby[2] = bby[3] = amax[1] = prob->bbox[3] = extent.yMaximum();
 
   prob->pal = this;
 
   QLinkedList<Feats *> *fFeats = new QLinkedList<Feats *>;
 
   FeatCallBackCtx context;
+
+  // prepare map boundary
+  geos::unique_ptr mapBoundaryGeos( mapBoundary.exportToGeos() );
+  geos::prepared_unique_ptr mapBoundaryPrepared( GEOSPrepare_r( geosContext(), mapBoundaryGeos.get() ) );
+
   context.fFeats = fFeats;
   context.obstacles = obstacles;
   context.candidates = prob->candidates;
-  context.bbox_min[0] = amin[0];
-  context.bbox_min[1] = amin[1];
-  context.bbox_max[0] = amax[0];
-  context.bbox_max[1] = amax[1];
+  context.mapBoundary = mapBoundaryPrepared.get();
 
   ObstacleCallBackCtx obstacleContext;
   obstacleContext.obstacles = obstacles;
@@ -451,9 +452,9 @@ void Pal::registerCancelationCallback( Pal::FnIsCanceled fnCanceled, void *conte
   fnIsCanceledContext = context;
 }
 
-Problem *Pal::extractProblem( double bbox[4] )
+std::unique_ptr<Problem> Pal::extractProblem( const QgsRectangle &extent, const QgsGeometry &mapBoundary )
 {
-  return extract( bbox[0], bbox[1], bbox[2], bbox[3] );
+  return extract( extent, mapBoundary );
 }
 
 QList<LabelPosition *> Pal::solveProblem( Problem *prob, bool displayAll )

src/core/pal/pal.h

@@ -138,7 +138,14 @@ namespace pal
       //! Check whether the job has been canceled
       inline bool isCanceled() { return fnIsCanceled ? fnIsCanceled( fnIsCanceledContext ) : false; }
 
-      Problem *extractProblem( double bbox[4] );
+      /**
+       * Extracts the labeling problem for the specified map \a extent - only features within this
+       * extent will be considered. The \a mapBoundary argument specifies the actual geometry of the map
+       * boundary, which will be used to detect whether a label is visible (or partially visible) in
+       * the rendered map. This may differ from \a extent in the case of rotated or non-rectangular
+       * maps.
+       */
+      std::unique_ptr< Problem > extractProblem( const QgsRectangle &extent, const QgsGeometry &mapBoundary );
 
       QList<LabelPosition *> solveProblem( Problem *prob, bool displayAll );
 
@@ -266,8 +273,7 @@ namespace pal
        * \param lambda_max xMax bounding-box
        * \param phi_max yMax bounding-box
        */
-      Problem *extract( double lambda_min, double phi_min,
-                        double lambda_max, double phi_max );
+      std::unique_ptr< Problem > extract( const QgsRectangle &extent, const QgsGeometry &mapBoundary );
 
 
       /**

src/core/qgslabelingengine.cpp

@@ -242,14 +242,21 @@ void QgsLabelingEngine::run( QgsRenderContext &context )
   QPainter *painter = context.painter();
 
   QgsGeometry extentGeom = QgsGeometry::fromRect( mMapSettings.visibleExtent() );
+  QPolygonF visiblePoly = mMapSettings.visiblePolygon();
+  visiblePoly.append( visiblePoly.at( 0 ) ); //close polygon
+  QgsGeometry mapBoundaryGeom = QgsGeometry::fromQPolygonF( visiblePoly );
+
   if ( !qgsDoubleNear( mMapSettings.rotation(), 0.0 ) )
   {
     //PAL features are prerotated, so extent also needs to be unrotated
     extentGeom.rotate( -mMapSettings.rotation(), mMapSettings.visibleExtent().center() );
+    // yes - this is rotated in the opposite direction... phew, this is confusing!
+    mapBoundaryGeom.rotate( mMapSettings.rotation(), mMapSettings.visibleExtent().center() );
   }
 
   QgsRectangle extent = extentGeom.boundingBox();
 
+
   p.registerCancelationCallback( &_palIsCanceled, reinterpret_cast< void * >( &context ) );
 
   QTime t;
@@ -259,7 +266,7 @@ void QgsLabelingEngine::run( QgsRenderContext &context )
   std::unique_ptr< pal::Problem > problem;
   try
   {
-    problem = p.extractProblem( bbox );
+    problem = p.extractProblem( extent, mapBoundaryGeom );
   }
   catch ( std::exception &e )
   {