Cleanup callout code to remove a lot of duplicated code between

simple line and manhattan line callout classes
qgis · Mar 18, 2021 · e5ab535 · e5ab535
1 parent 4791fc4
commit e5ab535
Show file tree

Hide file tree

Showing 5 changed files with 64 additions and 124 deletions.
diff --git a/python/core/auto_generated/callouts/qgscallout.sip.in b/python/core/auto_generated/callouts/qgscallout.sip.in
@@ -235,7 +235,7 @@ Returns the list of rendered callout positions.
 
     };
 
-    void render( QgsRenderContext &context, QRectF rect, const double angle, const QgsGeometry &anchor, QgsCalloutContext &calloutContext );
+    void render( QgsRenderContext &context, const QRectF &rect, const double angle, const QgsGeometry &anchor, QgsCalloutContext &calloutContext );
 %Docstring
 Renders the callout onto the specified render ``context``.
 
@@ -381,7 +381,7 @@ anchor point.
 
   protected:
 
-    virtual void draw( QgsRenderContext &context, QRectF bodyBoundingBox, const double angle, const QgsGeometry &anchor, QgsCalloutContext &calloutContext ) = 0;
+    virtual void draw( QgsRenderContext &context, const QRectF &bodyBoundingBox, const double angle, const QgsGeometry &anchor, QgsCalloutContext &calloutContext ) = 0;
 %Docstring
 Performs the actual rendering of the callout implementation onto the specified render ``context``.
 
@@ -401,15 +401,15 @@ The ``calloutContext`` argument is used to specify additional contextual informa
 how a callout is being rendered.
 %End
 
- QgsGeometry labelAnchorGeometry( QRectF bodyBoundingBox, const double angle, LabelAnchorPoint anchor ) const /Deprecated/;
+ QgsGeometry labelAnchorGeometry( const QRectF &bodyBoundingBox, const double angle, LabelAnchorPoint anchor ) const /Deprecated/;
 %Docstring
 Returns the anchor point geometry for a label with the given bounding box and ``anchor`` point mode.
 
 .. deprecated::
    QGIS 3.20 use :py:func:`~QgsCallout.calloutLabelPoint` instead
 %End
 
-    QgsGeometry calloutLabelPoint( QRectF bodyBoundingBox, double angle, LabelAnchorPoint anchor, QgsRenderContext &context, const QgsCalloutContext &calloutContext, bool &pinned ) const;
+    QgsGeometry calloutLabelPoint( const QRectF &bodyBoundingBox, double angle, LabelAnchorPoint anchor, QgsRenderContext &context, const QgsCalloutContext &calloutContext, bool &pinned ) const;
 %Docstring
 Returns the anchor point geometry for a label with the given bounding box and ``anchor`` point mode.
 
@@ -678,9 +678,18 @@ Sets whether callout lines should be drawn to all feature parts.
 %End
 
   protected:
-    virtual void draw( QgsRenderContext &context, QRectF bodyBoundingBox, const double angle, const QgsGeometry &anchor, QgsCallout::QgsCalloutContext &calloutContext );
+    virtual void draw( QgsRenderContext &context, const QRectF &bodyBoundingBox, const double angle, const QgsGeometry &anchor, QgsCallout::QgsCalloutContext &calloutContext );
 
 
+    virtual QgsCurve *createCalloutLine( const QgsPoint &start, const QgsPoint &end, QgsRenderContext &context, const QRectF &bodyBoundingBox, const double angle, const QgsGeometry &anchor, QgsCallout::QgsCalloutContext &calloutContext ) const /Factory/;
+%Docstring
+Creates a callout line between ``start`` and ``end`` in the desired style.
+
+The base class method returns a straight line.
+
+.. versionadded:: 3.20
+%End
+
   private:
     QgsSimpleLineCallout( const QgsSimpleLineCallout &other );
     QgsSimpleLineCallout &operator=( const QgsSimpleLineCallout & );
@@ -716,7 +725,7 @@ serialized in the ``properties`` map (corresponding to the output from
 
 
   protected:
-    virtual void draw( QgsRenderContext &context, QRectF bodyBoundingBox, const double angle, const QgsGeometry &anchor, QgsCallout::QgsCalloutContext &calloutContext );
+    virtual QgsCurve *createCalloutLine( const QgsPoint &start, const QgsPoint &end, QgsRenderContext &context, const QRectF &bodyBoundingBox, const double angle, const QgsGeometry &anchor, QgsCallout::QgsCalloutContext &calloutContext ) const /Factory/;
 
 
   private:

diff --git a/src/core/callouts/qgscallout.cpp b/src/core/callouts/qgscallout.cpp
@@ -120,7 +120,7 @@ QgsCallout::DrawOrder QgsCallout::drawOrder() const
   return OrderBelowAllLabels;
 }
 
-void QgsCallout::render( QgsRenderContext &context, QRectF rect, const double angle, const QgsGeometry &anchor, QgsCalloutContext &calloutContext )
+void QgsCallout::render( QgsRenderContext &context, const QRectF &rect, const double angle, const QgsGeometry &anchor, QgsCalloutContext &calloutContext )
 {
   if ( !mEnabled )
     return;
@@ -259,7 +259,7 @@ QgsCallout::LabelAnchorPoint QgsCallout::decodeLabelAnchorPoint( const QString &
   return LabelPointOnExterior;
 }
 
-QgsGeometry QgsCallout::labelAnchorGeometry( QRectF rect, const double angle, LabelAnchorPoint anchor ) const
+QgsGeometry QgsCallout::labelAnchorGeometry( const QRectF &rect, const double angle, LabelAnchorPoint anchor ) const
 {
   QgsGeometry label;
   switch ( anchor )
@@ -309,7 +309,7 @@ QgsGeometry QgsCallout::labelAnchorGeometry( QRectF rect, const double angle, La
   return label;
 }
 
-QgsGeometry QgsCallout::calloutLabelPoint( QRectF rect, const double angle, QgsCallout::LabelAnchorPoint anchor, QgsRenderContext &context, const QgsCallout::QgsCalloutContext &calloutContext, bool &pinned ) const
+QgsGeometry QgsCallout::calloutLabelPoint( const QRectF &rect, const double angle, QgsCallout::LabelAnchorPoint anchor, QgsRenderContext &context, const QgsCallout::QgsCalloutContext &calloutContext, bool &pinned ) const
 {
   pinned = false;
   if ( dataDefinedProperties().isActive( QgsCallout::OriginX ) && dataDefinedProperties().isActive( QgsCallout::OriginY ) )
@@ -470,6 +470,21 @@ QgsGeometry QgsCallout::calloutLineToPart( const QgsGeometry &labelGeometry, con
   return line;
 }
 
+//
+// QgsCallout::QgsCalloutContext
+//
+
+QgsCoordinateTransform QgsCallout::QgsCalloutContext::originalFeatureToMapTransform( const QgsRenderContext &renderContext ) const
+{
+  if ( !mOriginalFeatureToMapTransform.isValid() )
+  {
+    // lazy initialization, only create if needed...
+    mOriginalFeatureToMapTransform = QgsCoordinateTransform( originalFeatureCrs, renderContext.coordinateTransform().destinationCrs(), renderContext.transformContext() );
+  }
+  return mOriginalFeatureToMapTransform;
+}
+
+
 //
 // QgsSimpleLineCallout
 //
@@ -598,7 +613,7 @@ void QgsSimpleLineCallout::setLineSymbol( QgsLineSymbol *symbol )
   mLineSymbol.reset( symbol );
 }
 
-void QgsSimpleLineCallout::draw( QgsRenderContext &context, QRectF rect, const double angle, const QgsGeometry &anchor, QgsCalloutContext &calloutContext )
+void QgsSimpleLineCallout::draw( QgsRenderContext &context, const QRectF &rect, const double angle, const QgsGeometry &anchor, QgsCalloutContext &calloutContext )
 {
   LabelAnchorPoint labelAnchor = labelAnchorPoint();
   if ( dataDefinedProperties().isActive( QgsCallout::LabelAnchorPointPosition ) )
@@ -613,7 +628,7 @@ void QgsSimpleLineCallout::draw( QgsRenderContext &context, QRectF rect, const d
   if ( label.isNull() )
     return;
 
-  auto drawCalloutLine = [this, &context, &calloutContext, &label, originPinned]( const QgsAbstractGeometry * partAnchor )
+  auto drawCalloutLine = [this, &context, &calloutContext, &label, &rect, angle, &anchor, originPinned]( const QgsAbstractGeometry * partAnchor )
   {
     bool destinationPinned = false;
     QgsGeometry line = calloutLineToPart( label, partAnchor, context, calloutContext, destinationPinned );
@@ -634,6 +649,9 @@ void QgsSimpleLineCallout::draw( QgsRenderContext &context, QRectF rect, const d
     if ( minLengthPixels > 0 && lineLength < minLengthPixels )
       return; // too small!
 
+    std::unique_ptr< QgsCurve > calloutCurve( createCalloutLine( qgsgeometry_cast< const QgsLineString * >( line.constGet() )->startPoint(),
+        qgsgeometry_cast< const QgsLineString * >( line.constGet() )->endPoint(), context, rect, angle, anchor, calloutContext ) );
+
     double offsetFromAnchor = mOffsetFromAnchorDistance;
     if ( dataDefinedProperties().isActive( QgsCallout::OffsetFromAnchor ) )
     {
@@ -651,13 +669,10 @@ void QgsSimpleLineCallout::draw( QgsRenderContext &context, QRectF rect, const d
     const double offsetFromLabelPixels = context.convertToPainterUnits( offsetFromLabel, mOffsetFromLabelUnit, mOffsetFromLabelScale );
     if ( offsetFromAnchorPixels > 0 || offsetFromLabelPixels > 0 )
     {
-      if ( const QgsLineString *ls = qgsgeometry_cast< const QgsLineString * >( line.constGet() ) )
-      {
-        line = QgsGeometry( ls->curveSubstring( offsetFromLabelPixels, ls->length() - offsetFromAnchorPixels ) );
-      }
+      calloutCurve.reset( calloutCurve->curveSubstring( offsetFromLabelPixels, calloutCurve->length() - offsetFromAnchorPixels ) );
     }
 
-    const QPolygonF points = line.asQPolygonF();
+    const QPolygonF points = calloutCurve->asQPolygonF();
 
     if ( points.empty() )
       return;
@@ -688,7 +703,10 @@ void QgsSimpleLineCallout::draw( QgsRenderContext &context, QRectF rect, const d
   }
 }
 
-
+QgsCurve *QgsSimpleLineCallout::createCalloutLine( const QgsPoint &start, const QgsPoint &end, QgsRenderContext &, const QRectF &, const double, const QgsGeometry &, QgsCallout::QgsCalloutContext & ) const
+{
+  return new QgsLineString( start, end );
+}
 
 //
 // QgsManhattanLineCallout
@@ -722,104 +740,8 @@ QgsManhattanLineCallout *QgsManhattanLineCallout::clone() const
   return new QgsManhattanLineCallout( *this );
 }
 
-void QgsManhattanLineCallout::draw( QgsRenderContext &context, QRectF rect, const double angle, const QgsGeometry &anchor, QgsCalloutContext &calloutContext )
-{
-  LabelAnchorPoint labelAnchor = labelAnchorPoint();
-  if ( dataDefinedProperties().isActive( QgsCallout::LabelAnchorPointPosition ) )
-  {
-    QString encodedAnchor = encodeLabelAnchorPoint( labelAnchor );
-    context.expressionContext().setOriginalValueVariable( encodedAnchor );
-    labelAnchor = decodeLabelAnchorPoint( dataDefinedProperties().valueAsString( QgsCallout::LabelAnchorPointPosition, context.expressionContext(), encodedAnchor ) );
-  }
-  bool originPinned = false;
-  const QgsGeometry label = calloutLabelPoint( rect, angle, labelAnchor, context, calloutContext, originPinned );
-  if ( label.isNull() )
-    return;
-
-  auto drawCalloutLine = [this, &context, &calloutContext, &label, originPinned]( const QgsAbstractGeometry * partAnchor )
-  {
-    bool destinationPinned = false;
-    QgsGeometry line = calloutLineToPart( label, partAnchor, context, calloutContext, destinationPinned );
-    if ( line.isEmpty() )
-      return;
-
-    const double lineLength = line.length();
-    if ( qgsDoubleNear( lineLength, 0 ) )
-      return;
-
-    double minLength = minimumLength();
-    if ( dataDefinedProperties().isActive( QgsCallout::MinimumCalloutLength ) )
-    {
-      minLength = dataDefinedProperties().valueAsDouble( QgsCallout::MinimumCalloutLength, context.expressionContext(), minLength );
-    }
-    double minLengthPixels = context.convertToPainterUnits( minLength, minimumLengthUnit(), minimumLengthMapUnitScale() );
-    if ( minLengthPixels > 0 && lineLength < minLengthPixels )
-      return; // too small!
-
-    const QgsPoint start = qgsgeometry_cast< const QgsLineString * >( line.constGet() )->startPoint();
-    const QgsPoint end = qgsgeometry_cast< const QgsLineString * >( line.constGet() )->endPoint();
-    QgsPoint mid1 = QgsPoint( start.x(), end.y() );
-
-    line = QgsGeometry::fromPolyline( QgsPolyline() << start << mid1 << end );
-    double offsetFromAnchorDist = offsetFromAnchor();
-    if ( dataDefinedProperties().isActive( QgsCallout::OffsetFromAnchor ) )
-    {
-      offsetFromAnchorDist = dataDefinedProperties().valueAsDouble( QgsCallout::OffsetFromAnchor, context.expressionContext(), offsetFromAnchorDist );
-    }
-    const double offsetFromAnchorPixels = context.convertToPainterUnits( offsetFromAnchorDist, offsetFromAnchorUnit(), offsetFromAnchorMapUnitScale() );
-
-    double offsetFromLabelDist = offsetFromLabel();
-    if ( dataDefinedProperties().isActive( QgsCallout::OffsetFromLabel ) )
-    {
-      offsetFromLabelDist = dataDefinedProperties().valueAsDouble( QgsCallout::OffsetFromLabel, context.expressionContext(), offsetFromLabelDist );
-    }
-    const double offsetFromLabelPixels = context.convertToPainterUnits( offsetFromLabelDist, offsetFromAnchorUnit(), offsetFromAnchorMapUnitScale() );
-
-    if ( offsetFromAnchorPixels > 0 || offsetFromLabelPixels > 0 )
-    {
-      if ( QgsLineString *ls = qgsgeometry_cast< QgsLineString * >( line.get() ) )
-      {
-        line = QgsGeometry( ls->curveSubstring( offsetFromLabelPixels, ls->length() - offsetFromAnchorPixels ) );
-      }
-    }
-
-    const QPolygonF points = line.asQPolygonF();
-
-    if ( points.empty() )
-      return;
-
-    QgsCalloutPosition position;
-    position.setOrigin( context.mapToPixel().toMapCoordinates( points.at( 0 ).x(), points.at( 0 ).y() ).toQPointF() );
-    position.setOriginIsPinned( originPinned );
-    position.setDestination( context.mapToPixel().toMapCoordinates( points.constLast().x(), points.constLast().y() ).toQPointF() );
-    position.setDestinationIsPinned( destinationPinned );
-    calloutContext.addCalloutPosition( position );
-
-    lineSymbol()->renderPolyline( points, nullptr, context );
-  };
-
-  bool toAllParts = drawCalloutToAllParts();
-  if ( dataDefinedProperties().isActive( QgsCallout::DrawCalloutToAllParts ) )
-  {
-    context.expressionContext().setOriginalValueVariable( toAllParts );
-    toAllParts = dataDefinedProperties().valueAsBool( QgsCallout::DrawCalloutToAllParts, context.expressionContext(), toAllParts );
-  }
-
-  if ( calloutContext.allFeaturePartsLabeled || !toAllParts )
-    drawCalloutLine( anchor.constGet() );
-  else
-  {
-    for ( auto it = anchor.const_parts_begin(); it != anchor.const_parts_end(); ++it )
-      drawCalloutLine( *it );
-  }
-}
-
-QgsCoordinateTransform QgsCallout::QgsCalloutContext::originalFeatureToMapTransform( const QgsRenderContext &renderContext ) const
+QgsCurve *QgsManhattanLineCallout::createCalloutLine( const QgsPoint &start, const QgsPoint &end, QgsRenderContext &, const QRectF &, const double, const QgsGeometry &, QgsCallout::QgsCalloutContext & ) const
 {
-  if ( !mOriginalFeatureToMapTransform.isValid() )
-  {
-    // lazy initialization, only create if needed...
-    mOriginalFeatureToMapTransform = QgsCoordinateTransform( originalFeatureCrs, renderContext.coordinateTransform().destinationCrs(), renderContext.transformContext() );
-  }
-  return mOriginalFeatureToMapTransform;
+  QgsPoint mid1 = QgsPoint( start.x(), end.y() );
+  return new QgsLineString( QVector< QgsPoint >() << start << mid1 << end );
 }
diff --git a/src/core/callouts/qgscallout.h b/src/core/callouts/qgscallout.h
@@ -284,7 +284,7 @@ class CORE_EXPORT QgsCallout
      * \warning A prior call to startRender() must have been made before calling this method, and
      * after all render() operations are complete a call to stopRender() must be made.
      */
-    void render( QgsRenderContext &context, QRectF rect, const double angle, const QgsGeometry &anchor, QgsCalloutContext &calloutContext );
+    void render( QgsRenderContext &context, const QRectF &rect, const double angle, const QgsGeometry &anchor, QgsCalloutContext &calloutContext );
 
     /**
      * Returns TRUE if the the callout is enabled.
@@ -414,13 +414,13 @@ class CORE_EXPORT QgsCallout
      * The \a calloutContext argument is used to specify additional contextual information about
      * how a callout is being rendered.
      */
-    virtual void draw( QgsRenderContext &context, QRectF bodyBoundingBox, const double angle, const QgsGeometry &anchor, QgsCalloutContext &calloutContext ) = 0;
+    virtual void draw( QgsRenderContext &context, const QRectF &bodyBoundingBox, const double angle, const QgsGeometry &anchor, QgsCalloutContext &calloutContext ) = 0;
 
     /**
      * Returns the anchor point geometry for a label with the given bounding box and \a anchor point mode.
      * \deprecated QGIS 3.20 use calloutLabelPoint() instead
      */
-    Q_DECL_DEPRECATED QgsGeometry labelAnchorGeometry( QRectF bodyBoundingBox, const double angle, LabelAnchorPoint anchor ) const SIP_DEPRECATED;
+    Q_DECL_DEPRECATED QgsGeometry labelAnchorGeometry( const QRectF &bodyBoundingBox, const double angle, LabelAnchorPoint anchor ) const SIP_DEPRECATED;
 
     /**
      * Returns the anchor point geometry for a label with the given bounding box and \a anchor point mode.
@@ -429,7 +429,7 @@ class CORE_EXPORT QgsCallout
      *
      * \since QGIS 3.20
      */
-    QgsGeometry calloutLabelPoint( QRectF bodyBoundingBox, double angle, LabelAnchorPoint anchor, QgsRenderContext &context, const QgsCalloutContext &calloutContext, bool &pinned ) const;
+    QgsGeometry calloutLabelPoint( const QRectF &bodyBoundingBox, double angle, LabelAnchorPoint anchor, QgsRenderContext &context, const QgsCalloutContext &calloutContext, bool &pinned ) const;
 
     /**
      * Calculates the direct line from a label geometry to an anchor geometry part, respecting the various
@@ -661,7 +661,16 @@ class CORE_EXPORT QgsSimpleLineCallout : public QgsCallout
     void setDrawCalloutToAllParts( bool drawToAllParts ) { mDrawCalloutToAllParts = drawToAllParts; }
 
   protected:
-    void draw( QgsRenderContext &context, QRectF bodyBoundingBox, const double angle, const QgsGeometry &anchor, QgsCallout::QgsCalloutContext &calloutContext ) override;
+    void draw( QgsRenderContext &context, const QRectF &bodyBoundingBox, const double angle, const QgsGeometry &anchor, QgsCallout::QgsCalloutContext &calloutContext ) override;
+
+    /**
+     * Creates a callout line between \a start and \a end in the desired style.
+     *
+     * The base class method returns a straight line.
+     *
+     * \since QGIS 3.20
+     */
+    virtual QgsCurve *createCalloutLine( const QgsPoint &start, const QgsPoint &end, QgsRenderContext &context, const QRectF &bodyBoundingBox, const double angle, const QgsGeometry &anchor, QgsCallout::QgsCalloutContext &calloutContext ) const SIP_FACTORY;
 
   private:
 
@@ -720,7 +729,7 @@ class CORE_EXPORT QgsManhattanLineCallout : public QgsSimpleLineCallout
     QgsManhattanLineCallout *clone() const override;
 
   protected:
-    void draw( QgsRenderContext &context, QRectF bodyBoundingBox, const double angle, const QgsGeometry &anchor, QgsCallout::QgsCalloutContext &calloutContext ) override;
+    QgsCurve *createCalloutLine( const QgsPoint &start, const QgsPoint &end, QgsRenderContext &context, const QRectF &bodyBoundingBox, const double angle, const QgsGeometry &anchor, QgsCallout::QgsCalloutContext &calloutContext ) const override SIP_FACTORY;
 
   private:
 #ifdef SIP_RUN

diff --git a/tests/src/core/testqgscallout.cpp b/tests/src/core/testqgscallout.cpp
@@ -77,7 +77,7 @@ class DummyCallout : public QgsCallout
 
   protected:
 
-    void draw( QgsRenderContext &, QRectF, const double, const QgsGeometry &, QgsCallout::QgsCalloutContext & ) override { }
+    void draw( QgsRenderContext &, const QRectF &, const double, const QgsGeometry &, QgsCallout::QgsCalloutContext & ) override { }
 
   private:
     QString mProp1;

diff --git a/tests/src/core/testqgscalloutregistry.cpp b/tests/src/core/testqgscalloutregistry.cpp
@@ -31,7 +31,7 @@ class DummyCallout : public QgsCallout
     QgsCallout *clone() const override { return new DummyCallout(); }
     static QgsCallout *create( const QVariantMap &, const QgsReadWriteContext & ) { return new DummyCallout(); }
   protected:
-    void draw( QgsRenderContext &, QRectF, const double, const QgsGeometry &, QgsCallout::QgsCalloutContext & ) override {}
+    void draw( QgsRenderContext &, const QRectF &, const double, const QgsGeometry &, QgsCallout::QgsCalloutContext & ) override {}
 
 };