Fix groups with rotated item (still glitchy when resizing with mixed rotation though)

python/core/auto_generated/layout/qgslayoutitemgroup.sip.in

@@ -73,6 +73,10 @@ Returns a list of items contained by the group.
 
     virtual ExportLayerBehavior exportLayerBehavior() const;
 
+
+    virtual QRectF rectWithFrame() const;
+
+
   protected:
     virtual void draw( QgsLayoutItemRenderContext &context );
 

src/core/layout/qgslayoutitemgroup.cpp

@@ -81,7 +81,7 @@ void QgsLayoutItemGroup::addItem( QgsLayoutItem *item )
   mItems << QPointer< QgsLayoutItem >( item );
   item->setParentGroup( this );
 
-  updateBoundingRect( item );
+  updateBoundingRect();
 }
 
 void QgsLayoutItemGroup::removeItems()
@@ -172,7 +172,7 @@ void QgsLayoutItemGroup::attemptMove( const QgsLayoutPoint &point, bool useRefer
   QgsLayoutItem::attemptMove( point, includesFrame );
   if ( !shouldBlockUndoCommands() )
     mLayout->undoStack()->endMacro();
-  resetBoundingRect();
+  updateBoundingRect();
 }
 
 void QgsLayoutItemGroup::attemptResize( const QgsLayoutSize &size, bool includesFrame )
@@ -224,7 +224,7 @@ void QgsLayoutItemGroup::attemptResize( const QgsLayoutSize &size, bool includes
   if ( !shouldBlockUndoCommands() )
     mLayout->undoStack()->endMacro();
 
-  resetBoundingRect();
+  updateBoundingRect();
 }
 
 bool QgsLayoutItemGroup::writePropertiesToElement( QDomElement &element, QDomDocument &document, const QgsReadWriteContext & ) const
@@ -269,7 +269,7 @@ void QgsLayoutItemGroup::finalizeRestoreFromXml()
     }
   }
 
-  resetBoundingRect();
+  updateBoundingRect();
 }
 
 QgsLayoutItem::ExportLayerBehavior QgsLayoutItemGroup::exportLayerBehavior() const
@@ -286,53 +286,58 @@ void QgsLayoutItemGroup::draw( QgsLayoutItemRenderContext & )
   // nothing to draw here!
 }
 
-void QgsLayoutItemGroup::resetBoundingRect()
+
+void QgsLayoutItemGroup::updateBoundingRect()
 {
-  mBoundingRectangle = QRectF();
-  for ( QgsLayoutItem *item : std::as_const( mItems ) )
+
+  if ( mItems.isEmpty() )
   {
-    updateBoundingRect( item );
+    setRect( QRectF() );
+    return;
   }
-}
 
-void QgsLayoutItemGroup::updateBoundingRect( QgsLayoutItem *item )
-{
-  //update extent
-  if ( mBoundingRectangle.isEmpty() ) //we add the first item
+  //check if all child items have same rotation
+  auto itemIter = mItems.constBegin();
+
+  //start with rotation of first child
+  double rotation = ( *itemIter )->rotation();
+
+  //iterate through remaining children, checking if they have same rotation
+  for ( ++itemIter; itemIter != mItems.constEnd(); ++itemIter )
   {
-    mBoundingRectangle = QRectF( 0, 0, item->rect().width(), item->rect().height() );
-    setSceneRect( QRectF( item->pos().x(), item->pos().y(), item->rect().width(), item->rect().height() ) );
-
-    if ( !qgsDoubleNear( item->rotation(), 0.0 ) )
+    if ( !qgsDoubleNear( ( *itemIter )->rotation(), rotation ) )
     {
-      setItemRotation( item->rotation() );
+      //item has a different rotation
+      rotation = 0.0;
+      break;
     }
   }
-  else
+  setScenePos( QPointF( 0, 0 ) );
+  setItemRotation( rotation );
+
+  itemIter = mItems.constBegin();
+
+  // start with handle bounds of first child
+  QRectF groupRect = mapFromItem( ( *itemIter ), ( *itemIter )->rect() ).boundingRect();
+  QRectF groupRectWithFrame = mapFromItem( ( *itemIter ), ( *itemIter )->rectWithFrame() ).boundingRect();
+
+  //iterate through remaining children, expanding the bounds as required
+  for ( ++itemIter; itemIter != mItems.constEnd(); ++itemIter )
   {
-    if ( !qgsDoubleNear( item->rotation(), rotation() ) )
-    {
-      //items have mixed rotation, so reset rotation of group
-      mBoundingRectangle = mapRectToScene( mBoundingRectangle );
-      setItemRotation( 0 );
-      mBoundingRectangle = mBoundingRectangle.united( item->mapRectToScene( item->rect() ) );
-      setSceneRect( mBoundingRectangle );
-    }
-    else
-    {
-      //items have same rotation, so keep rotation of group
-      mBoundingRectangle = mBoundingRectangle.united( mapRectFromItem( item, item->rect() ) );
-      QPointF newPos = mapToScene( mBoundingRectangle.topLeft().x(), mBoundingRectangle.topLeft().y() );
-      mBoundingRectangle = QRectF( 0, 0, mBoundingRectangle.width(), mBoundingRectangle.height() );
-      setSceneRect( QRectF( newPos.x(), newPos.y(), mBoundingRectangle.width(), mBoundingRectangle.height() ) );
-    }
+    groupRect |= mapFromItem( ( *itemIter ), ( *itemIter )->rect() ).boundingRect();
+    groupRectWithFrame |= mapFromItem( ( *itemIter ), ( *itemIter )->rectWithFrame() ).boundingRect();
   }
+
+  mItemSize = mLayout->convertFromLayoutUnits( groupRect.size(), sizeWithUnits().units() );
+  mItemPosition = mLayout->convertFromLayoutUnits( mapToScene( groupRect.topLeft() ), positionWithUnits().units() );
+  setRect( 0, 0, groupRect.width(), groupRect.height() );
+  setPos( mapToScene( groupRect.topLeft() ) );
+
+  QPointF bleedShift = groupRectWithFrame.topLeft() - groupRect.topLeft();
+  mRectWithFrame = QRectF( bleedShift, groupRectWithFrame.size() );
 }
 
-void QgsLayoutItemGroup::setSceneRect( const QRectF &rectangle )
+QRectF QgsLayoutItemGroup::rectWithFrame() const
 {
-  mItemPosition = mLayout->convertFromLayoutUnits( rectangle.topLeft(), positionWithUnits().units() );
-  mItemSize = mLayout->convertFromLayoutUnits( rectangle.size(), sizeWithUnits().units() );
-  setScenePos( rectangle.topLeft() );
-  setRect( 0, 0, rectangle.width(), rectangle.height() );
+  return mRectWithFrame;
 }

src/core/layout/qgslayoutitemgroup.h

@@ -76,20 +76,22 @@ class CORE_EXPORT QgsLayoutItemGroup: public QgsLayoutItem
 
     void finalizeRestoreFromXml() override;
     ExportLayerBehavior exportLayerBehavior() const override;
+
+    QRectF rectWithFrame() const override;
+
   protected:
     void draw( QgsLayoutItemRenderContext &context ) override;
     bool writePropertiesToElement( QDomElement &parentElement, QDomDocument &document, const QgsReadWriteContext &context ) const override;
     bool readPropertiesFromElement( const QDomElement &itemElement, const QDomDocument &document, const QgsReadWriteContext &context ) override;
 
-  private:
+  private slots:
+    void updateBoundingRect();
 
-    void resetBoundingRect();
-    void updateBoundingRect( QgsLayoutItem *item );
-    void setSceneRect( const QRectF &rectangle );
+  private:
 
     QList< QString > mItemUuids;
     QList< QPointer< QgsLayoutItem >> mItems;
-    QRectF mBoundingRectangle;
+    QRectF mRectWithFrame;
 };
 
 #endif //QGSLAYOUTITEMGROUP_H

src/gui/layout/qgslayoutmousehandles.cpp

@@ -218,9 +218,25 @@ void QgsLayoutMouseHandles::expandItemList( const QList<QGraphicsItem *> &items,
     {
       // if a group is selected, we don't draw the bounds of the group - instead we draw the bounds of the grouped items
       const QList<QgsLayoutItem *> groupItems = static_cast< QgsLayoutItemGroup * >( item )->items();
-      collected.reserve( collected.size() + groupItems.size() );
-      for ( QgsLayoutItem *groupItem : groupItems )
-        collected.append( groupItem );
+      expandItemList( groupItems, collected );
+    }
+    else
+    {
+      collected << item;
+    }
+  }
+}
+
+
+void QgsLayoutMouseHandles::expandItemList( const QList<QgsLayoutItem *> &items, QList<QGraphicsItem *> &collected ) const
+{
+  for ( QGraphicsItem *item : items )
+  {
+    if ( item->type() == QgsLayoutItemRegistry::LayoutGroup )
+    {
+      // if a group is selected, we don't draw the bounds of the group - instead we draw the bounds of the grouped items
+      const QList<QgsLayoutItem *> groupItems = static_cast< QgsLayoutItemGroup * >( item )->items();
+      expandItemList( groupItems, collected );
     }
     else
     {

src/gui/layout/qgslayoutmousehandles.h

@@ -75,6 +75,7 @@ class GUI_EXPORT QgsLayoutMouseHandles: public QgsGraphicsViewMouseHandles
     bool itemIsGroupMember( QGraphicsItem *item ) override;
     QRectF itemRect( QGraphicsItem *item ) const override;
     void expandItemList( const QList< QGraphicsItem * > &items, QList< QGraphicsItem * > &collected ) const override;
+    void expandItemList( const QList< QgsLayoutItem * > &items, QList< QGraphicsItem * > &collected ) const;
     void moveItem( QGraphicsItem *item, double deltaX, double deltaY ) override;
     void setItemRect( QGraphicsItem *item, QRectF rect ) override;
     void showStatusMessage( const QString &message ) override;

src/gui/qgsgraphicsviewmousehandles.cpp

@@ -141,6 +141,15 @@ void QgsGraphicsViewMouseHandles::drawSelectedItemBounds( QPainter *painter )
     return;
   }
 
+  QList< QGraphicsItem * > itemsToDraw;
+  expandItemList( selectedItems, itemsToDraw );
+
+  if ( itemsToDraw.size() <= 1 )
+  {
+    // Single item selected. The items bounds are drawn by the MouseHandles itself.
+    return;
+  }
+
   //use difference mode so that they are visible regardless of item colors
   QgsScopedQPainterState painterState( painter );
   painter->setCompositionMode( QPainter::CompositionMode_Difference );
@@ -152,9 +161,6 @@ void QgsGraphicsViewMouseHandles::drawSelectedItemBounds( QPainter *painter )
   painter->setPen( selectedItemPen );
   painter->setBrush( Qt::NoBrush );
 
-  QList< QGraphicsItem * > itemsToDraw;
-  expandItemList( selectedItems, itemsToDraw );
-
   for ( QGraphicsItem *item : std::as_const( itemsToDraw ) )
   {
     //get bounds of selected item
@@ -173,24 +179,16 @@ void QgsGraphicsViewMouseHandles::drawSelectedItemBounds( QPainter *painter )
     else if ( isResizing() && !itemIsLocked( item ) )
     {
       //if currently resizing, calculate relative resize of this item
-      if ( selectedItems.size() > 1 )
-      {
-        //get item bounds in mouse handle item's coordinate system
-        QRectF thisItemRect = mapRectFromItem( item, itemRect( item ) );
-        //now, resize it relative to the current resized dimensions of the mouse handles
-        relativeResizeRect( thisItemRect, QRectF( -mResizeMoveX, -mResizeMoveY, mBeginHandleWidth, mBeginHandleHeight ), mResizeRect );
-        itemBounds = QPolygonF( thisItemRect );
-      }
-      else
-      {
-        //single item selected
-        itemBounds = rect();
-      }
+      //get item bounds in mouse handle item's coordinate system
+      QRectF thisItemRect = mapRectFromItem( item, itemRect( item ) );
+      //now, resize it relative to the current resized dimensions of the mouse handles
+      relativeResizeRect( thisItemRect, QRectF( -mResizeMoveX, -mResizeMoveY, mBeginHandleWidth, mBeginHandleHeight ), mResizeRect );
+      itemBounds = QPolygonF( thisItemRect );
     }
     else
     {
-      //not resizing or moving, so just map from scene bounds
-      itemBounds = mapRectFromItem( item, itemRect( item ) );
+      // not resizing or moving, so just map the item's bounds to the mouse handle item's coordinate system
+      itemBounds = item->mapToItem( this, itemRect( item ) );
     }
 
     // drawPolygon causes issues on windows - corners of path may be missing resulting in triangles being drawn