[layouts] Fix bounding rectangle for shapes doesn't include stroke

width after loading from xml Fixes #43748
qgis · Jul 21, 2021 · 7978634 · 7978634
1 parent 4311770
commit 7978634
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 6 deletions.
diff --git a/src/core/layout/qgslayoutitemshape.cpp b/src/core/layout/qgslayoutitemshape.cpp
@@ -37,7 +37,7 @@ QgsLayoutItemShape::QgsLayoutItemShape( QgsLayout *layout )
   properties.insert( QStringLiteral( "width_border" ), QStringLiteral( "0.3" ) );
   properties.insert( QStringLiteral( "joinstyle" ), QStringLiteral( "miter" ) );
   mShapeStyleSymbol.reset( QgsFillSymbol::createSimple( properties ) );
-  refreshSymbol();
+  refreshSymbol( false );
 
   connect( this, &QgsLayoutItemShape::sizePositionChanged, this, [ = ]
   {
@@ -117,7 +117,7 @@ void QgsLayoutItemShape::setShapeType( QgsLayoutItemShape::Shape type )
   emit clipPathChanged();
 }
 
-void QgsLayoutItemShape::refreshSymbol()
+void QgsLayoutItemShape::refreshSymbol( bool redraw )
 {
   if ( auto *lLayout = layout() )
   {
@@ -127,7 +127,9 @@ void QgsLayoutItemShape::refreshSymbol()
 
   updateBoundingRect();
 
-  update();
+  if ( redraw )
+    update();
+
   emit frameChanged();
 }
 
@@ -148,7 +150,7 @@ void QgsLayoutItemShape::setSymbol( QgsFillSymbol *symbol )
     return;
 
   mShapeStyleSymbol.reset( symbol->clone() );
-  refreshSymbol();
+  refreshSymbol( true );
 }
 
 void QgsLayoutItemShape::setCornerRadius( QgsLayoutMeasurement radius )
@@ -276,6 +278,7 @@ bool QgsLayoutItemShape::readPropertiesFromElement( const QDomElement &element,
   if ( !shapeStyleSymbolElem.isNull() )
   {
     mShapeStyleSymbol.reset( QgsSymbolLayerUtils::loadSymbol<QgsFillSymbol>( shapeStyleSymbolElem, context ) );
+    refreshSymbol( false );
   }
 
   return true;

diff --git a/src/core/layout/qgslayoutitemshape.h b/src/core/layout/qgslayoutitemshape.h
@@ -124,8 +124,11 @@ class CORE_EXPORT QgsLayoutItemShape : public QgsLayoutItem
     /**
      * Should be called after the shape's symbol is changed. Redraws the shape and recalculates
      * its selection bounds.
+     *
+     * If \a redraw is FALSE than the symbol bounds will be recalculated only, without redrawing
+     * the item.
     */
-    void refreshSymbol();
+    void refreshSymbol( bool redraw );
 
     //! Updates the bounding rect of this item
     void updateBoundingRect();

diff --git a/tests/src/python/test_qgslayoutshape.py b/tests/src/python/test_qgslayoutshape.py
@@ -19,9 +19,12 @@
     QgsLayout,
     QgsLayoutItem,
     QgsLayoutMeasurement,
-    QgsUnitTypes
+    QgsUnitTypes,
+    QgsFillSymbol,
+    QgsReadWriteContext
 )
 from qgis.PyQt.QtCore import QRectF
+from qgis.PyQt.QtXml import QDomDocument, QDomElement
 from qgis.PyQt.QtTest import QSignalSpy
 
 from test_qgslayoutitem import LayoutItemTestCase
@@ -64,6 +67,41 @@ def testClipPath(self):
         self.assertEqual(len(spy), 4)
         self.assertEqual(shape.clipPath().asWkt(), 'Polygon ((50 140, 130 140, 90 20, 50 140))')
 
+    def testBoundingRectForStrokeSizeOnRestore(self):
+        """
+        Test that item bounding rect correctly accounts for stroke size on item restore
+        """
+        pr = QgsProject()
+        l = QgsLayout(pr)
+        shape = QgsLayoutItemShape(l)
+
+        shape.setShapeType(QgsLayoutItemShape.Rectangle)
+        shape.attemptSetSceneRect(QRectF(30, 10, 100, 200))
+        self.assertEqual(shape.boundingRect(), QRectF(-0.15, -0.15, 100.3, 200.3))
+
+        # set a symbol with very wide stroke
+        s = QgsFillSymbol.createSimple({'outline_color': '#ff0000', 'outline_width': '40', 'color': '#ff5588'})
+        shape.setSymbol(s)
+        # bounding rect for item should include stroke
+        self.assertEqual(shape.boundingRect(), QRectF(-20.0, -20.0, 140.0, 240.0))
+
+        # save the shape and restore
+        doc = QDomDocument("testdoc")
+        parent_elem = doc.createElement("test")
+        doc.appendChild(parent_elem)
+        self.assertTrue(shape.writeXml(parent_elem, doc, QgsReadWriteContext()))
+
+        item_elem = parent_elem.firstChildElement("LayoutItem")
+        self.assertFalse(item_elem.isNull())
+
+        # restore
+        shape2 = QgsLayoutItemShape(l)
+
+        self.assertTrue(shape2.readXml(item_elem, doc, QgsReadWriteContext()))
+
+        # bounding rect for item should include stroke
+        self.assertEqual(shape2.boundingRect(), QRectF(-20.0, -20.0, 140.0, 240.0))
+
 
 if __name__ == '__main__':
     unittest.main()