Fix calculation of point symbol bounds using data defined rotation or…

… offset
qgis · Apr 29, 2016 · a67853f · a67853f
1 parent d20a2ce
commit a67853f
Show file tree

Hide file tree

Showing 11 changed files with 76 additions and 11 deletions.
diff --git a/python/core/symbology-ng/qgssymbolv2.sip b/python/core/symbology-ng/qgssymbolv2.sip
@@ -494,11 +494,16 @@ class QgsMarkerSymbolV2 : QgsSymbolV2
     void renderPoint( QPointF point, const QgsFeature* f, QgsRenderContext& context, int layer = -1, bool selected = false );
 
     /** Returns the approximate bounding box of the marker symbol, which includes the bounding box
-     * of all symbol layers for the symbol.
+     * of all symbol layers for the symbol. It is recommended to use this method only between startRender()
+     * and stopRender() calls, or data defined rotation and offset will not be correctly calculated.
+     * @param point location of rendered point in painter units
+     * @param context render context
+     * @param feature feature being rendered at point (optional). If not specified, the bounds calculation will not
+     * include data defined parameters such as offset and rotation
      * @returns approximate symbol bounds, in painter units
      * @note added in QGIS 2.14
-     */
-    QRectF bounds( QPointF point, QgsRenderContext& context ) const;
+    */
+    QRectF bounds(QPointF point, QgsRenderContext& context, const QgsFeature &feature = QgsFeature() ) const;
 
     virtual QgsMarkerSymbolV2* clone() const /Factory/;
 };

diff --git a/src/core/qgsvectorlayerlabelprovider.cpp b/src/core/qgsvectorlayerlabelprovider.cpp
@@ -353,9 +353,9 @@ QgsGeometry* QgsVectorLayerLabelProvider::getPointObstacleGeometry( QgsFeature&
       if ( symbol->type() == QgsSymbolV2::Marker )
       {
         if ( bounds.isValid() )
-          bounds = bounds.united( static_cast< QgsMarkerSymbolV2* >( symbol )->bounds( pt, context ) );
+          bounds = bounds.united( static_cast< QgsMarkerSymbolV2* >( symbol )->bounds( pt, context, fet ) );
         else
-          bounds = static_cast< QgsMarkerSymbolV2* >( symbol )->bounds( pt, context );
+          bounds = static_cast< QgsMarkerSymbolV2* >( symbol )->bounds( pt, context, fet );
       }
     }
 

diff --git a/src/core/symbology-ng/qgssymbolv2.cpp b/src/core/symbology-ng/qgssymbolv2.cpp
@@ -767,7 +767,7 @@ void QgsSymbolV2::renderFeature( const QgsFeature& feature, QgsRenderContext& co
         //draw debugging rect
         context.painter()->setPen( Qt::red );
         context.painter()->setBrush( QColor( 255, 0, 0, 100 ) );
-        context.painter()->drawRect( static_cast<QgsMarkerSymbolV2*>( this )->bounds( pt, context ) );
+        context.painter()->drawRect( static_cast<QgsMarkerSymbolV2*>( this )->bounds( pt, context, feature ) );
       }
 
       if ( drawVertexMarker )
@@ -1479,9 +1479,9 @@ void QgsMarkerSymbolV2::renderPoint( QPointF point, const QgsFeature* f, QgsRend
   }
 }
 
-QRectF QgsMarkerSymbolV2::bounds( QPointF point, QgsRenderContext& context ) const
+QRectF QgsMarkerSymbolV2::bounds( QPointF point, QgsRenderContext& context, const QgsFeature& feature ) const
 {
-  QgsSymbolV2RenderContext symbolContext( context, outputUnit(), mAlpha, false, mRenderHints, nullptr, nullptr, mapUnitScale() );
+  QgsSymbolV2RenderContext symbolContext( context, outputUnit(), mAlpha, false, mRenderHints, &feature, feature.fields(), mapUnitScale() );
 
   QRectF bound;
   Q_FOREACH ( QgsSymbolLayerV2* layer, mLayers )

diff --git a/src/core/symbology-ng/qgssymbolv2.h b/src/core/symbology-ng/qgssymbolv2.h
@@ -565,10 +565,16 @@ class CORE_EXPORT QgsMarkerSymbolV2 : public QgsSymbolV2
     void renderPoint( QPointF point, const QgsFeature* f, QgsRenderContext& context, int layer = -1, bool selected = false );
 
     /** Returns the approximate bounding box of the marker symbol, which includes the bounding box
-     * of all symbol layers for the symbol.
+     * of all symbol layers for the symbol. It is recommended to use this method only between startRender()
+     * and stopRender() calls, or data defined rotation and offset will not be correctly calculated.
+     * @param point location of rendered point in painter units
+     * @param context render context
+     * @param feature feature being rendered at point (optional). If not specified, the bounds calculation will not
+     * include data defined parameters such as offset and rotation
      * @returns approximate symbol bounds, in painter units
-     * @note added in QGIS 2.14     */
-    QRectF bounds( QPointF point, QgsRenderContext& context ) const;
+     * @note added in QGIS 2.14
+    */
+    QRectF bounds( QPointF point, QgsRenderContext& context, const QgsFeature &feature = QgsFeature() ) const;
 
     virtual QgsMarkerSymbolV2* clone() const override;
 

diff --git a/tests/src/core/testqgssimplemarker.cpp b/tests/src/core/testqgssimplemarker.cpp
@@ -63,6 +63,9 @@ class TestQgsSimpleMarkerSymbol : public QObject
     void simpleMarkerSymbolMiterJoin();
     void simpleMarkerSymbolRoundJoin();
     void bounds();
+    void boundsWithOffset();
+    void boundsWithRotation();
+    void boundsWithRotationAndOffset();
 
   private:
     bool mTestHasError;
@@ -194,6 +197,57 @@ void TestQgsSimpleMarkerSymbol::bounds()
   mMapSettings.setFlag( QgsMapSettings::DrawSymbolBounds, true );
   bool result = imageCheck( "simplemarker_bounds" );
   mMapSettings.setFlag( QgsMapSettings::DrawSymbolBounds, false );
+  mSimpleMarkerLayer->removeDataDefinedProperty( "size" );
+  QVERIFY( result );
+}
+
+void TestQgsSimpleMarkerSymbol::boundsWithOffset()
+{
+  mSimpleMarkerLayer->setColor( QColor( 200, 200, 200 ) );
+  mSimpleMarkerLayer->setBorderColor( QColor( 0, 0, 0 ) );
+  mSimpleMarkerLayer->setName( "circle" );
+  mSimpleMarkerLayer->setSize( 5 );
+  mSimpleMarkerLayer->setDataDefinedProperty( "offset", new QgsDataDefined( true, true, "if(importance > 2, '5,10', '10, 5')" ) );
+  mSimpleMarkerLayer->setOutlineWidth( 0.5 );
+
+  mMapSettings.setFlag( QgsMapSettings::DrawSymbolBounds, true );
+  bool result = imageCheck( "simplemarker_boundsoffset" );
+  mMapSettings.setFlag( QgsMapSettings::DrawSymbolBounds, false );
+  mSimpleMarkerLayer->removeDataDefinedProperty( "offset" );
+  QVERIFY( result );
+}
+
+void TestQgsSimpleMarkerSymbol::boundsWithRotation()
+{
+  mSimpleMarkerLayer->setColor( QColor( 200, 200, 200 ) );
+  mSimpleMarkerLayer->setBorderColor( QColor( 0, 0, 0 ) );
+  mSimpleMarkerLayer->setName( "square" );
+  mSimpleMarkerLayer->setSize( 5 );
+  mSimpleMarkerLayer->setDataDefinedProperty( "angle", new QgsDataDefined( true, true, "importance * 20" ) );
+  mSimpleMarkerLayer->setOutlineWidth( 0.5 );
+
+  mMapSettings.setFlag( QgsMapSettings::DrawSymbolBounds, true );
+  bool result = imageCheck( "simplemarker_boundsrotation" );
+  mMapSettings.setFlag( QgsMapSettings::DrawSymbolBounds, false );
+  mSimpleMarkerLayer->removeDataDefinedProperty( "angle" );
+  QVERIFY( result );
+}
+
+void TestQgsSimpleMarkerSymbol::boundsWithRotationAndOffset()
+{
+  mSimpleMarkerLayer->setColor( QColor( 200, 200, 200 ) );
+  mSimpleMarkerLayer->setBorderColor( QColor( 0, 0, 0 ) );
+  mSimpleMarkerLayer->setName( "square" );
+  mSimpleMarkerLayer->setSize( 5 );
+  mSimpleMarkerLayer->setDataDefinedProperty( "offset", new QgsDataDefined( true, true, "if(importance > 2, '5,10', '10, 5')" ) );
+  mSimpleMarkerLayer->setDataDefinedProperty( "angle", new QgsDataDefined( true, false, QString(), "heading" ) );
+  mSimpleMarkerLayer->setOutlineWidth( 0.5 );
+
+  mMapSettings.setFlag( QgsMapSettings::DrawSymbolBounds, true );
+  bool result = imageCheck( "simplemarker_boundsrotationoffset" );
+  mMapSettings.setFlag( QgsMapSettings::DrawSymbolBounds, false );
+  mSimpleMarkerLayer->removeDataDefinedProperty( "offset" );
+  mSimpleMarkerLayer->removeDataDefinedProperty( "angle" );
   QVERIFY( result );
 }
 

diff --git a/...arker/expected_simplemarker_boundsoffset/expected_simplemarker_boundsoffset.png b/...arker/expected_simplemarker_boundsoffset/expected_simplemarker_boundsoffset.png
diff --git a/.../expected_simplemarker_boundsoffset/expected_simplemarker_boundsoffset_mask.png b/.../expected_simplemarker_boundsoffset/expected_simplemarker_boundsoffset_mask.png
diff --git a/...r/expected_simplemarker_boundsrotation/expected_simplemarker_boundsrotation.png b/...r/expected_simplemarker_boundsrotation/expected_simplemarker_boundsrotation.png
diff --git a/...ected_simplemarker_boundsrotation/expected_simplemarker_boundsrotation_mask.png b/...ected_simplemarker_boundsrotation/expected_simplemarker_boundsrotation_mask.png
diff --git a/...implemarker_boundsrotationoffset/expected_simplemarker_boundsrotationoffset.png b/...implemarker_boundsrotationoffset/expected_simplemarker_boundsrotationoffset.png
diff --git a/...marker_boundsrotationoffset/expected_simplemarker_boundsrotationoffset_mask.png b/...marker_boundsrotationoffset/expected_simplemarker_boundsrotationoffset_mask.png