Fix correct frame bounding rects for shapes and node items

python/core/layout/qgslayoutitemnodeitem.sip

@@ -108,6 +108,12 @@ Returns the number of nodes in the shape.
  Deselects any selected nodes.
 %End
 
+    virtual QRectF boundingRect() const;
+
+
+    virtual double estimatedFrameBleed() const;
+
+
   protected:
 
     QgsLayoutNodesItem( QgsLayout *layout );
@@ -133,6 +139,7 @@ Returns the number of nodes in the shape.
 
 
 
+
     virtual bool _addNode( const int nodeIndex, QPointF newNode, const double radius ) = 0;
 %Docstring
 Method called in addNode.

python/core/layout/qgslayoutitemshape.sip

@@ -81,6 +81,9 @@ class QgsLayoutItemShape : QgsLayoutItem
     virtual QRectF boundingRect() const;
 
 
+    virtual double estimatedFrameBleed() const;
+
+
   protected:
 
     virtual void draw( QgsRenderContext &context, const QStyleOptionGraphicsItem *itemStyle = 0 );

src/core/layout/qgslayoutitemnodeitem.cpp

@@ -29,6 +29,16 @@ void QgsLayoutNodesItem::setNodes( const QPolygonF &nodes )
   updateSceneRect();
 }
 
+QRectF QgsLayoutNodesItem::boundingRect() const
+{
+  return mCurrentRectangle;
+}
+
+double QgsLayoutNodesItem::estimatedFrameBleed() const
+{
+  return mMaxSymbolBleed;
+}
+
 QgsLayoutNodesItem::QgsLayoutNodesItem( QgsLayout *layout )
   : QgsLayoutItem( layout )
 {
@@ -55,6 +65,8 @@ void QgsLayoutNodesItem::init()
   setCacheMode( QGraphicsItem::NoCache );
   setBackgroundEnabled( false );
   setFrameEnabled( false );
+
+  connect( this, &QgsLayoutNodesItem::sizePositionChanged, this, &QgsLayoutNodesItem::updateBoundingRect );
 }
 
 void QgsLayoutNodesItem::draw( QgsRenderContext &context, const QStyleOptionGraphicsItem *style )
@@ -311,18 +323,24 @@ void QgsLayoutNodesItem::updateSceneRect()
   const QRectF br = mPolygon.boundingRect();
 
   const QPointF topLeft = mapToScene( br.topLeft() );
+  //will trigger updateBoundingRect if necessary
   attemptSetSceneRect( QRectF( topLeft.x(), topLeft.y(), br.width(), br.height() ) );
 
   // update polygon position
   mPolygon.translate( -br.topLeft().x(), -br.topLeft().y() );
+}
+
+void QgsLayoutNodesItem::updateBoundingRect()
+{
+  QRectF br = rect();
+  br.adjust( -mMaxSymbolBleed, -mMaxSymbolBleed, mMaxSymbolBleed, mMaxSymbolBleed );
+  mCurrentRectangle = br;
 
   // update
   prepareGeometryChange();
   update();
 }
 
-
-
 bool QgsLayoutNodesItem::writePropertiesToElement( QDomElement &elem, QDomDocument &doc, const QgsReadWriteContext &context ) const
 {
   // style

src/core/layout/qgslayoutitemnodeitem.h

@@ -107,6 +107,13 @@ class CORE_EXPORT QgsLayoutNodesItem: public QgsLayoutItem
      */
     void deselectNode() { mSelectedNode = -1; }
 
+    // Depending on the symbol style, the bounding rectangle can be larger than the shape
+    QRectF boundingRect() const override;
+
+    // Reimplement estimatedFrameBleed, since frames on shapes are drawn using symbology
+    // rather than the item's pen
+    double estimatedFrameBleed() const override;
+
   protected:
 
     /**
@@ -132,6 +139,9 @@ class CORE_EXPORT QgsLayoutNodesItem: public QgsLayoutItem
     //! Shape's nodes.
     QPolygonF mPolygon;
 
+    //! Max symbol bleed
+    double mMaxSymbolBleed = 0.0;
+
     //! Method called in addNode.
     virtual bool _addNode( const int nodeIndex, QPointF newNode, const double radius ) = 0;
 
@@ -159,6 +169,10 @@ class CORE_EXPORT QgsLayoutNodesItem: public QgsLayoutItem
     //! Update the current scene rectangle for this item.
     void updateSceneRect();
 
+  private slots:
+
+    void updateBoundingRect();
+
   private:
 
     void init();
@@ -171,6 +185,9 @@ class CORE_EXPORT QgsLayoutNodesItem: public QgsLayoutItem
      * the painting. */
     bool mDrawNodes = false;
 
+    //! Current bounding rectangle of shape
+    QRectF mCurrentRectangle;
+
     //! Draw nodes
     void drawNodes( QgsRenderContext &context, const QStyleOptionGraphicsItem *itemStyle = nullptr ) const;
     void drawSelectedNode( QgsRenderContext &context, const QStyleOptionGraphicsItem *itemStyle = nullptr ) const;

src/core/layout/qgslayoutitempolygon.cpp

@@ -73,6 +73,19 @@ void QgsLayoutItemPolygon::createDefaultPolygonStyleSymbol()
 
   mPolygonStyleSymbol.reset( QgsFillSymbol::createSimple( properties ) );
 
+  refreshSymbol();
+}
+
+void QgsLayoutItemPolygon::refreshSymbol()
+{
+  if ( layout() )
+  {
+    QgsRenderContext rc = QgsLayoutUtils::createRenderContextForLayout( layout(), nullptr, layout()->context().dpi() );
+    mMaxSymbolBleed = ( 25.4 / layout()->context().dpi() ) * QgsSymbolLayerUtils::estimateMaxSymbolBleed( mPolygonStyleSymbol.get(), rc );
+  }
+
+  updateSceneRect();
+
   emit frameChanged();
 }
 
@@ -108,8 +121,7 @@ void QgsLayoutItemPolygon::_readXmlStyle( const QDomElement &elmt, const QgsRead
 void QgsLayoutItemPolygon::setSymbol( QgsFillSymbol *symbol )
 {
   mPolygonStyleSymbol.reset( static_cast<QgsFillSymbol *>( symbol->clone() ) );
-  update();
-  emit frameChanged();
+  refreshSymbol();
 }
 
 void QgsLayoutItemPolygon::_writeXmlStyle( QDomDocument &doc, QDomElement &elmt, const QgsReadWriteContext &context ) const

src/core/layout/qgslayoutitempolygon.h

@@ -81,6 +81,12 @@ class CORE_EXPORT QgsLayoutItemPolygon: public QgsLayoutNodesItem
     std::unique_ptr<QgsFillSymbol> mPolygonStyleSymbol;
     //! Create a default symbol.
     void createDefaultPolygonStyleSymbol();
+
+    /**
+     * Should be called after the shape's symbol is changed. Redraws the shape and recalculates
+     * its selection bounds.
+    */
+    void refreshSymbol();
 };
 
 #endif // QGSLAYOUTITEMPOLYGON_H

src/core/layout/qgslayoutitempolyline.cpp

@@ -18,7 +18,9 @@
 #include "qgslayoutitemregistry.h"
 #include "qgssymbollayerutils.h"
 #include "qgssymbol.h"
+#include "qgslayout.h"
 #include "qgsmapsettings.h"
+#include "qgslayoututils.h"
 #include <limits>
 
 QgsLayoutItemPolyline::QgsLayoutItemPolyline( QgsLayout *layout )
@@ -96,6 +98,18 @@ void QgsLayoutItemPolyline::createDefaultPolylineStyleSymbol()
   properties.insert( QStringLiteral( "capstyle" ), QStringLiteral( "square" ) );
 
   mPolylineStyleSymbol.reset( QgsLineSymbol::createSimple( properties ) );
+  refreshSymbol();
+}
+
+void QgsLayoutItemPolyline::refreshSymbol()
+{
+  if ( layout() )
+  {
+    QgsRenderContext rc = QgsLayoutUtils::createRenderContextForLayout( layout(), nullptr, layout()->context().dpi() );
+    mMaxSymbolBleed = ( 25.4 / layout()->context().dpi() ) * QgsSymbolLayerUtils::estimateMaxSymbolBleed( mPolylineStyleSymbol.get(), rc );
+  }
+
+  updateSceneRect();
 
   emit frameChanged();
 }
@@ -127,8 +141,7 @@ void QgsLayoutItemPolyline::_readXmlStyle( const QDomElement &elmt, const QgsRea
 void QgsLayoutItemPolyline::setSymbol( QgsLineSymbol *symbol )
 {
   mPolylineStyleSymbol.reset( static_cast<QgsLineSymbol *>( symbol->clone() ) );
-  update();
-  emit frameChanged();
+  refreshSymbol();
 }
 
 void QgsLayoutItemPolyline::_writeXmlStyle( QDomDocument &doc, QDomElement &elmt, const QgsReadWriteContext &context ) const

src/core/layout/qgslayoutitempolyline.h

@@ -82,6 +82,12 @@ class CORE_EXPORT QgsLayoutItemPolyline: public QgsLayoutNodesItem
 
     //! Create a default symbol.
     void createDefaultPolylineStyleSymbol();
+
+    /**
+     * Should be called after the shape's symbol is changed. Redraws the shape and recalculates
+     * its selection bounds.
+    */
+    void refreshSymbol();
 };
 
 #endif // QGSLAYOUTITEMPOLYLINE_H

src/core/layout/qgslayoutitemshape.cpp

@@ -129,6 +129,11 @@ QRectF QgsLayoutItemShape::boundingRect() const
   return mCurrentRectangle;
 }
 
+double QgsLayoutItemShape::estimatedFrameBleed() const
+{
+  return mMaxSymbolBleed;
+}
+
 void QgsLayoutItemShape::draw( QgsRenderContext &context, const QStyleOptionGraphicsItem * )
 {
   QPainter *painter = context.painter();

src/core/layout/qgslayoutitemshape.h

@@ -95,6 +95,10 @@ class CORE_EXPORT QgsLayoutItemShape : public QgsLayoutItem
     // Depending on the symbol style, the bounding rectangle can be larger than the shape
     QRectF boundingRect() const override;
 
+    // Reimplement estimatedFrameBleed, since frames on shapes are drawn using symbology
+    // rather than the item's pen
+    double estimatedFrameBleed() const override;
+
   protected:
 
     void draw( QgsRenderContext &context, const QStyleOptionGraphicsItem *itemStyle = nullptr ) override;

tests/src/core/testqgslayoutshapes.cpp

@@ -48,6 +48,7 @@ class TestQgsLayoutShapes : public QObject
     void roundedRectangle(); //test if rounded rectangle shape is functioning
     void symbol(); //test is styling shapes via symbol is working
     void readWriteXml();
+    void bounds();
 
   private:
 
@@ -253,5 +254,36 @@ void TestQgsLayoutShapes::readWriteXml()
   QCOMPARE( copy->symbol()->symbolLayer( 0 )->strokeColor().name(), QStringLiteral( "#ffff00" ) );
 }
 
+void TestQgsLayoutShapes::bounds()
+{
+  QgsProject p;
+  QgsLayout l( &p );
+  QgsLayoutItemShape *shape = new QgsLayoutItemShape( &l );
+  shape->attemptMove( QgsLayoutPoint( 20, 20 ) );
+  shape->attemptResize( QgsLayoutSize( 150, 100 ) );
+
+  QgsSimpleFillSymbolLayer *simpleFill = new QgsSimpleFillSymbolLayer();
+  QgsFillSymbol *fillSymbol = new QgsFillSymbol();
+  fillSymbol->changeSymbolLayer( 0, simpleFill );
+  simpleFill->setColor( Qt::green );
+  simpleFill->setStrokeColor( Qt::yellow );
+  simpleFill->setStrokeWidth( 6 );
+  shape->setSymbol( fillSymbol );
+
+  // scene bounding rect should include symbol outline
+  QRectF bounds = shape->sceneBoundingRect();
+  QCOMPARE( bounds.left(), 17.0 );
+  QCOMPARE( bounds.right(), 173.0 );
+  QCOMPARE( bounds.top(), 17.0 );
+  QCOMPARE( bounds.bottom(), 123.0 );
+
+  // rectWithFrame should include symbol outline too
+  bounds = shape->rectWithFrame();
+  QCOMPARE( bounds.left(), -3.0 );
+  QCOMPARE( bounds.right(), 153.0 );
+  QCOMPARE( bounds.top(), -3.0 );
+  QCOMPARE( bounds.bottom(), 103.0 );
+}
+
 QGSTEST_MAIN( TestQgsLayoutShapes )
 #include "testqgslayoutshapes.moc"

tests/src/python/test_qgslayoutpolygon.py

@@ -279,6 +279,41 @@ def testReadWriteXml(self):
         self.assertEqual(shape2.symbol().symbolLayer(0).color().name(), '#008000')
         self.assertEqual(shape2.symbol().symbolLayer(0).strokeColor().name(), '#ff0000')
 
+    def testBounds(self):
+        pr = QgsProject()
+        l = QgsLayout(pr)
+
+        p = QPolygonF()
+        p.append(QPointF(50.0, 30.0))
+        p.append(QPointF(100.0, 10.0))
+        p.append(QPointF(200.0, 100.0))
+        shape = QgsLayoutItemPolygon(p, l)
+
+        props = {}
+        props["color"] = "green"
+        props["style"] = "solid"
+        props["style_border"] = "solid"
+        props["color_border"] = "red"
+        props["width_border"] = "6.0"
+        props["joinstyle"] = "miter"
+
+        style = QgsFillSymbol.createSimple(props)
+        shape.setSymbol(style)
+
+        # scene bounding rect should include symbol outline
+        bounds = shape.sceneBoundingRect()
+        self.assertEqual(bounds.left(), 47.0)
+        self.assertEqual(bounds.right(), 203.0)
+        self.assertEqual(bounds.top(), 7.0)
+        self.assertEqual(bounds.bottom(), 103.0)
+
+        # rectWithFrame should include symbol outline too
+        bounds = shape.rectWithFrame()
+        self.assertEqual(bounds.left(), -3.0)
+        self.assertEqual(bounds.right(), 153.0)
+        self.assertEqual(bounds.top(), -3.0)
+        self.assertEqual(bounds.bottom(), 93.0)
+
 
 if __name__ == '__main__':
     unittest.main()

tests/src/python/test_qgslayoutpolyline.py

@@ -274,6 +274,38 @@ def testReadWriteXml(self):
         self.assertEqual(shape2.nodes(), shape.nodes())
         self.assertEqual(shape2.symbol().symbolLayer(0).color().name(), '#ff0000')
 
+    def testBounds(self):
+        pr = QgsProject()
+        l = QgsLayout(pr)
+
+        p = QPolygonF()
+        p.append(QPointF(50.0, 30.0))
+        p.append(QPointF(100.0, 10.0))
+        p.append(QPointF(200.0, 100.0))
+        shape = QgsLayoutItemPolyline(p, l)
+
+        props = {}
+        props["color"] = "255,0,0,255"
+        props["width"] = "6.0"
+        props["capstyle"] = "square"
+
+        style = QgsLineSymbol.createSimple(props)
+        shape.setSymbol(style)
+
+        # scene bounding rect should include symbol outline
+        bounds = shape.sceneBoundingRect()
+        self.assertEqual(bounds.left(), 47.0)
+        self.assertEqual(bounds.right(), 203.0)
+        self.assertEqual(bounds.top(), 7.0)
+        self.assertEqual(bounds.bottom(), 103.0)
+
+        # rectWithFrame should include symbol outline too
+        bounds = shape.rectWithFrame()
+        self.assertEqual(bounds.left(), -3.0)
+        self.assertEqual(bounds.right(), 153.0)
+        self.assertEqual(bounds.top(), -3.0)
+        self.assertEqual(bounds.bottom(), 93.0)
+
 
 if __name__ == '__main__':
     unittest.main()