Fix moving/resizing grouped items

python/core/layout/qgslayoutitem.sip

@@ -150,18 +150,28 @@ class QgsLayoutItem : QgsLayoutObject, QGraphicsRectItem, QgsLayoutUndoObjectInt
  :rtype: bool
 %End
 
-
     bool isGroupMember() const;
 %Docstring
+ Returns true if the item is part of a QgsLayoutItemGroup group.
+.. seealso:: parentGroup()
+.. seealso:: setParentGroup()
  :rtype: bool
 %End
 
     QgsLayoutItemGroup *parentGroup() const;
 %Docstring
+ Returns the item's parent group, if the item is part of a QgsLayoutItemGroup group.
+.. seealso:: isGroupMember()
+.. seealso:: setParentGroup()
  :rtype: QgsLayoutItemGroup
 %End
 
     void setParentGroup( QgsLayoutItemGroup *group );
+%Docstring
+ Sets the item's parent ``group``.
+.. seealso:: isGroupMember()
+.. seealso:: parentGroup()
+%End
 
     virtual void paint( QPainter *painter, const QStyleOptionGraphicsItem *itemStyle, QWidget *pWidget );
 

python/core/layout/qgslayoutitemgroup.sip

@@ -50,8 +50,16 @@ class QgsLayoutItemGroup: QgsLayoutItem
 
     virtual void setVisibility( const bool visible );
 
-  protected:
 
+    virtual void attemptMove( const QgsLayoutPoint &point );
+
+    virtual void attemptResize( const QgsLayoutSize &size );
+
+
+    virtual void paint( QPainter *painter, const QStyleOptionGraphicsItem *itemStyle, QWidget *pWidget );
+
+
+  protected:
     virtual void draw( QgsRenderContext &context, const QStyleOptionGraphicsItem *itemStyle = 0 );
 
 

src/core/layout/qgslayoutitem.h

@@ -170,11 +170,25 @@ class CORE_EXPORT QgsLayoutItem : public QgsLayoutObject, public QGraphicsRectIt
      */
     bool isLocked() const { return mIsLocked; }
 
-
+    /**
+     * Returns true if the item is part of a QgsLayoutItemGroup group.
+     * \see parentGroup()
+     * \see setParentGroup()
+     */
     bool isGroupMember() const;
 
+    /**
+     * Returns the item's parent group, if the item is part of a QgsLayoutItemGroup group.
+     * \see isGroupMember()
+     * \see setParentGroup()
+     */
     QgsLayoutItemGroup *parentGroup() const;
 
+    /**
+     * Sets the item's parent \a group.
+     * \see isGroupMember()
+     * \see parentGroup()
+     */
     void setParentGroup( QgsLayoutItemGroup *group );
 
     /**
@@ -643,6 +657,7 @@ class CORE_EXPORT QgsLayoutItem : public QgsLayoutObject, public QGraphicsRectIt
     bool shouldBlockUndoCommands() const;
 
     friend class TestQgsLayoutItem;
+    friend class QgsLayoutItemGroup;
 };
 
 #endif //QGSLAYOUTITEM_H

src/core/layout/qgslayoutitemgroup.cpp

@@ -17,6 +17,7 @@
 #include "qgslayoutitemgroup.h"
 #include "qgslayoutitemregistry.h"
 #include "qgslayout.h"
+#include "qgslayoututils.h"
 
 QgsLayoutItemGroup::QgsLayoutItemGroup( QgsLayout *layout )
   : QgsLayoutItem( layout )
@@ -77,7 +78,7 @@ void QgsLayoutItemGroup::addItem( QgsLayoutItem *item )
   mItems << QPointer< QgsLayoutItem >( item );
   item->setParentGroup( this );
 
-  updateBoundingRect();
+  updateBoundingRect( item );
 }
 
 void QgsLayoutItemGroup::removeItems()
@@ -121,50 +122,133 @@ void QgsLayoutItemGroup::setVisibility( const bool visible )
     mLayout->undoStack()->endMacro();
 }
 
-void QgsLayoutItemGroup::draw( QgsRenderContext &, const QStyleOptionGraphicsItem * )
+void QgsLayoutItemGroup::attemptMove( const QgsLayoutPoint &point )
 {
-  // nothing to draw here!
-}
+  if ( !mLayout )
+    return;
 
-void QgsLayoutItemGroup::updateBoundingRect()
-{
-#if 0
-  mBoundingRectangle = QRectF();
+  mLayout->undoStack()->beginMacro( tr( "Moved group" ) );
 
+  QPointF scenePoint = mLayout->convertToLayoutUnits( point );
+  double deltaX = scenePoint.x() - pos().x();
+  double deltaY = scenePoint.y() - pos().y();
+
+  //also move all items within the group
   for ( QgsLayoutItem *item : qgsAsConst( mItems ) )
   {
     if ( !item )
       continue;
 
-    //update extent
-    if ( mBoundingRectangle.isEmpty() ) //we add the first item
-    {
-      mBoundingRectangle = QRectF( item->pos().x(), item->pos().y(), item->rect().width(), item->rect().height() );
+    mLayout->undoStack()->beginCommand( item, QString() );
 
-      if ( !qgsDoubleNear( item->itemRotation(), 0.0 ) )
-      {
-        setItemRotation( item->itemRotation() );
-      }
+    // need to convert delta from layout units -> item units
+    QgsLayoutPoint itemPos = item->positionWithUnits();
+    QgsLayoutPoint deltaPos = mLayout->convertFromLayoutUnits( QPointF( deltaX, deltaY ), itemPos.units() );
+    itemPos.setX( itemPos.x() + deltaPos.x() );
+    itemPos.setY( itemPos.y() + deltaPos.y() );
+    item->attemptMove( itemPos );
+
+    mLayout->undoStack()->endCommand();
+  }
+  //lastly move group item itself
+  QgsLayoutItem::attemptMove( point );
+  mLayout->undoStack()->endMacro();
+  resetBoundingRect();
+}
+
+void QgsLayoutItemGroup::attemptResize( const QgsLayoutSize &size )
+{
+  if ( !mLayout )
+    return;
+
+  mLayout->undoStack()->beginMacro( tr( "Resized group" ) );
+
+  QRectF oldRect = rect();
+  QSizeF newSizeLayoutUnits = mLayout->convertToLayoutUnits( size );
+  QRectF newRect;
+  newRect.setSize( newSizeLayoutUnits );
+
+  //also resize all items within the group
+  for ( QgsLayoutItem *item : qgsAsConst( mItems ) )
+  {
+    if ( !item )
+      continue;
+
+    QRectF itemRect = mapRectFromItem( item, item->rect() );
+    QgsLayoutUtils::relativeResizeRect( itemRect, oldRect, newRect );
+
+    itemRect = itemRect.normalized();
+    QPointF newPos = mapToScene( itemRect.topLeft() );
+
+    // translate new position to current item units
+    QgsLayoutPoint itemPos = mLayout->convertFromLayoutUnits( newPos, item->positionWithUnits().units() );
+    item->attemptMove( itemPos );
+
+    QgsLayoutSize itemSize = mLayout->convertFromLayoutUnits( itemRect.size(), item->sizeWithUnits().units() );
+    item->attemptResize( itemSize );
+  }
+  QgsLayoutItem::attemptResize( size );
+  mLayout->undoStack()->endMacro();
+
+  resetBoundingRect();
+}
+
+void QgsLayoutItemGroup::paint( QPainter *, const QStyleOptionGraphicsItem *, QWidget * )
+{
+}
+
+void QgsLayoutItemGroup::draw( QgsRenderContext &, const QStyleOptionGraphicsItem * )
+{
+  // nothing to draw here!
+}
+
+void QgsLayoutItemGroup::resetBoundingRect()
+{
+  mBoundingRectangle = QRectF();
+  for ( QgsLayoutItem *item : qgsAsConst( mItems ) )
+  {
+    updateBoundingRect( item );
+  }
+}
+
+void QgsLayoutItemGroup::updateBoundingRect( QgsLayoutItem *item )
+{
+  //update extent
+  if ( mBoundingRectangle.isEmpty() ) //we add the first item
+  {
+    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->itemRotation(), 0.0 ) )
+    {
+      setItemRotation( item->itemRotation() );
+    }
+  }
+  else
+  {
+    if ( !qgsDoubleNear( item->itemRotation(), itemRotation() ) )
+    {
+      //items have mixed rotation, so reset rotation of group
+      mBoundingRectangle = mapRectToScene( mBoundingRectangle );
+      setItemRotation( 0 );
+      mBoundingRectangle = mBoundingRectangle.united( item->mapRectToScene( item->rect() ) );
+      setSceneRect( mBoundingRectangle );
     }
     else
     {
-      if ( !qgsDoubleNear( item->itemRotation(), itemRotation() ) )
-      {
-        //items have mixed rotation, so reset rotation of group
-        mBoundingRectangle = mapRectToScene( mBoundingRectangle );
-        setItemRotation( 0 );
-        mBoundingRectangle = mBoundingRectangle.united( item->mapRectToScene( item->rect() ) );
-      }
-      else
-      {
-        //items have same rotation, so keep rotation of group
-        mBoundingRectangle = mBoundingRectangle.united( mapRectFromItem( item, item->rect() ) );
-        mBoundingRectangle = QRectF( 0, 0, mBoundingRectangle.width(), mBoundingRectangle.height() );
-      }
+      //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() ) );
     }
   }
-
-  //call method of superclass to avoid repositioning of items
-  QgsLayoutItem::setSceneRect( mBoundingRectangle );
-#endif
+}
+
+void QgsLayoutItemGroup::setSceneRect( const QRectF &rectangle )
+{
+  mItemPosition = mLayout->convertFromLayoutUnits( rectangle.topLeft(), positionWithUnits().units() );
+  mItemSize = mLayout->convertFromLayoutUnits( rectangle.size(), sizeWithUnits().units() );
+  setScenePos( rectangle.topLeft() );
+  setRect( 0, 0, rectangle.width(), rectangle.height() );
 }

src/core/layout/qgslayoutitemgroup.h

@@ -56,15 +56,23 @@ class CORE_EXPORT QgsLayoutItemGroup: public QgsLayoutItem
     QList<QgsLayoutItem *> items() const;
 
     //overridden to also hide grouped items
-    virtual void setVisibility( const bool visible ) override;
+    void setVisibility( const bool visible ) override;
+
+    //overridden to move child items
+    void attemptMove( const QgsLayoutPoint &point ) override;
+    void attemptResize( const QgsLayoutSize &size ) override;
+
+    void paint( QPainter *painter, const QStyleOptionGraphicsItem *itemStyle, QWidget *pWidget ) override;
 
   protected:
-
     void draw( QgsRenderContext &context, const QStyleOptionGraphicsItem *itemStyle = nullptr ) override;
 
   private:
 
-    void updateBoundingRect();
+    void resetBoundingRect();
+    void updateBoundingRect( QgsLayoutItem *item );
+    void setSceneRect( const QRectF &rectangle );
+
 
     QList< QPointer< QgsLayoutItem >> mItems;
     QRectF mBoundingRectangle;

src/core/layout/qgslayoutsnapper.cpp

@@ -328,16 +328,13 @@ double QgsLayoutSnapper::snapPointsToItems( const QList<double> &points, Qt::Ori
   for ( QGraphicsItem *item : itemList )
   {
     QgsLayoutItem *currentItem = dynamic_cast< QgsLayoutItem *>( item );
-    if ( ignoreItems.contains( currentItem ) )
+    if ( !currentItem || ignoreItems.contains( currentItem ) )
       continue;
+    if ( currentItem->type() == QgsLayoutItemRegistry::LayoutGroup )
+      continue; // don't snap to group bounds, instead we snap to group item bounds
+    if ( !currentItem->isVisible() )
+      continue; // don't snap to invisible items
 
-    //don't snap to selected items, since they're the ones that will be snapping to something else
-    //also ignore group members - only snap to bounds of group itself
-    //also ignore hidden items
-    if ( !currentItem /* TODO || currentItem->selected() || currentItem->isGroupMember() */ || !currentItem->isVisible() )
-    {
-      continue;
-    }
     QRectF itemRect;
     if ( dynamic_cast<const QgsLayoutItemPage *>( currentItem ) )
     {

src/gui/layout/qgslayoutmousehandles.cpp

@@ -132,19 +132,26 @@ void QgsLayoutMouseHandles::drawSelectedItemBounds( QPainter *painter )
   painter->setBrush( Qt::NoBrush );
 
   QList< QgsLayoutItem * > itemsToDraw;
-  for ( QgsLayoutItem *item : selectedItems )
-  {
-    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
-      itemsToDraw.append( static_cast< QgsLayoutItemGroup * >( item )->items() );
-    }
-    else
-    {
-      itemsToDraw << item;
-    }
 
-  }
+  std::function< void( const QList< QgsLayoutItem * > items ) > collectItems;
+
+  collectItems = [&itemsToDraw, &collectItems]( const QList< QgsLayoutItem * > items )
+  {
+    for ( QgsLayoutItem *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
+        collectItems( static_cast< QgsLayoutItemGroup * >( item )->items() );
+      }
+      else
+      {
+        itemsToDraw << item;
+      }
+    }
+  };
+  collectItems( selectedItems );
+
 
   for ( QgsLayoutItem *item : qgsAsConst( itemsToDraw ) )
   {
@@ -614,9 +621,9 @@ void QgsLayoutMouseHandles::mouseReleaseEvent( QGraphicsSceneMouseEvent *event )
     const QList<QgsLayoutItem *> selectedItems = mLayout->selectedLayoutItems( false );
     for ( QgsLayoutItem *item : selectedItems )
     {
-      if ( item->isLocked() || ( item->flags() & QGraphicsItem::ItemIsSelectable ) == 0 )
+      if ( item->isLocked() || ( item->flags() & QGraphicsItem::ItemIsSelectable ) == 0 || item->isGroupMember() )
       {
-        //don't move locked items
+        //don't move locked items, or grouped items (group takes care of that)
         continue;
       }
 

tests/src/core/testqgslayoutitemgroup.cpp

@@ -46,6 +46,9 @@ class TestQgsLayoutItemGroup : public QObject
     void createGroup(); //test grouping items
     void ungroup(); //test ungrouping items
     void deleteGroup(); //test deleting group works
+    void groupVisibility();
+    void moveGroup();
+    void resizeGroup();
     void undoRedo(); //test that group/ungroup undo/redo commands don't crash
 
   private:
@@ -272,6 +275,118 @@ void TestQgsLayoutItemGroup::deleteGroup()
   QVERIFY( items.empty() );
 }
 
+void TestQgsLayoutItemGroup::groupVisibility()
+{
+  QgsProject proj;
+  QgsLayout l( &proj );
+
+  QgsLayoutItemRectangularShape *item = new QgsLayoutItemRectangularShape( &l );
+  l.addLayoutItem( item );
+  QgsLayoutItemRectangularShape *item2 = new QgsLayoutItemRectangularShape( &l );
+  l.addLayoutItem( item2 );
+
+  //group items
+  QList<QgsLayoutItem *> groupItems;
+  groupItems << item << item2;
+  QgsLayoutItemGroup *group = l.groupItems( groupItems );
+
+  QVERIFY( item->isVisible() );
+  QVERIFY( item2->isVisible() );
+  QVERIFY( group->isVisible() );
+  group->setVisibility( false );
+  QVERIFY( !item->isVisible() );
+  QVERIFY( !item2->isVisible() );
+  QVERIFY( !group->isVisible() );
+  group->setVisibility( true );
+  QVERIFY( item->isVisible() );
+  QVERIFY( item2->isVisible() );
+  QVERIFY( group->isVisible() );
+}
+
+void TestQgsLayoutItemGroup::moveGroup()
+{
+  QgsProject proj;
+  QgsLayout l( &proj );
+
+  QgsLayoutItemRectangularShape *item = new QgsLayoutItemRectangularShape( &l );
+  l.addLayoutItem( item );
+  item->attemptMove( QgsLayoutPoint( 0.05, 0.09, QgsUnitTypes::LayoutMeters ) );
+
+  QgsLayoutItemRectangularShape *item2 = new QgsLayoutItemRectangularShape( &l );
+  l.addLayoutItem( item2 );
+  item2->attemptMove( QgsLayoutPoint( 2, 3, QgsUnitTypes::LayoutInches ) );
+
+  //group items
+  QList<QgsLayoutItem *> groupItems;
+  groupItems << item << item2;
+  QgsLayoutItemGroup *group = l.groupItems( groupItems );
+  l.addLayoutItem( group );
+
+  QCOMPARE( group->positionWithUnits().x(), 50.8 );
+  QCOMPARE( group->positionWithUnits().y(), 76.2 );
+  QCOMPARE( group->positionWithUnits().units(), QgsUnitTypes::LayoutMillimeters );
+
+  group->attemptMove( QgsLayoutPoint( 20.8, 36.2, QgsUnitTypes::LayoutMillimeters ) );
+  QCOMPARE( group->positionWithUnits().x(), 20.8 );
+  QCOMPARE( group->positionWithUnits().y(), 36.2 );
+  QCOMPARE( group->positionWithUnits().units(), QgsUnitTypes::LayoutMillimeters );
+  QCOMPARE( item->positionWithUnits().x(), 0.02 );
+  QCOMPARE( item->positionWithUnits().y(), 0.05 );
+  QCOMPARE( item->positionWithUnits().units(), QgsUnitTypes::LayoutMeters );
+  QGSCOMPARENEAR( item2->positionWithUnits().x(), 0.818898, 0.0001 );
+  QGSCOMPARENEAR( item2->positionWithUnits().y(), 1.425197, 0.0001 );
+  QCOMPARE( item2->positionWithUnits().units(), QgsUnitTypes::LayoutInches );
+}
+
+void TestQgsLayoutItemGroup::resizeGroup()
+{
+  QgsProject proj;
+  QgsLayout l( &proj );
+
+  QgsLayoutItemRectangularShape *item = new QgsLayoutItemRectangularShape( &l );
+  l.addLayoutItem( item );
+  item->attemptMove( QgsLayoutPoint( 0.05, 0.09, QgsUnitTypes::LayoutMeters ) );
+  item->attemptResize( QgsLayoutSize( 0.1, 0.15, QgsUnitTypes::LayoutMeters ) );
+
+  QgsLayoutItemRectangularShape *item2 = new QgsLayoutItemRectangularShape( &l );
+  l.addLayoutItem( item2 );
+  item2->attemptMove( QgsLayoutPoint( 2, 3, QgsUnitTypes::LayoutInches ) );
+  item2->attemptResize( QgsLayoutSize( 4, 6, QgsUnitTypes::LayoutInches ) );
+
+  //group items
+  QList<QgsLayoutItem *> groupItems;
+  groupItems << item << item2;
+  QgsLayoutItemGroup *group = l.groupItems( groupItems );
+  l.addLayoutItem( group );
+
+  QCOMPARE( group->positionWithUnits().x(), 50.0 );
+  QCOMPARE( group->positionWithUnits().y(), 76.2 );
+  QCOMPARE( group->positionWithUnits().units(), QgsUnitTypes::LayoutMillimeters );
+  QCOMPARE( group->sizeWithUnits().width(), 102.4 );
+  QCOMPARE( group->sizeWithUnits().height(),  163.8 );
+  QCOMPARE( group->sizeWithUnits().units(), QgsUnitTypes::LayoutMillimeters );
+
+  group->attemptResize( QgsLayoutSize( 50.8, 76.2, QgsUnitTypes::LayoutMillimeters ) );
+  QCOMPARE( group->positionWithUnits().x(), 50.0 );
+  QCOMPARE( group->positionWithUnits().y(), 76.2 );
+  QCOMPARE( group->positionWithUnits().units(), QgsUnitTypes::LayoutMillimeters );
+  QCOMPARE( group->sizeWithUnits().width(), 50.8 );
+  QCOMPARE( group->sizeWithUnits().height(),  76.2 );
+  QCOMPARE( group->sizeWithUnits().units(), QgsUnitTypes::LayoutMillimeters );
+  QCOMPARE( item->positionWithUnits().x(), 0.05 );
+  QGSCOMPARENEAR( item->positionWithUnits().y(), 0.0826198, 0.00001 );
+  QCOMPARE( item->positionWithUnits().units(), QgsUnitTypes::LayoutMeters );
+  QGSCOMPARENEAR( item->sizeWithUnits().width(), 0.0496094, 0.0001 );
+  QGSCOMPARENEAR( item->sizeWithUnits().height(), 0.069780, 0.0001 );
+  QCOMPARE( item->sizeWithUnits().units(), QgsUnitTypes::LayoutMeters );
+  QGSCOMPARENEAR( item2->positionWithUnits().x(), 1.984129, 0.0001 );
+  QGSCOMPARENEAR( item2->positionWithUnits().y(), 3.000000, 0.0001 );
+  QCOMPARE( item2->positionWithUnits().units(), QgsUnitTypes::LayoutInches );
+  QGSCOMPARENEAR( item2->sizeWithUnits().width(), 1.98438, 0.0001 );
+  QGSCOMPARENEAR( item2->sizeWithUnits().height(),  2.791209, 0.0001 );
+  QCOMPARE( item2->sizeWithUnits().units(), QgsUnitTypes::LayoutInches );
+}
+
 Q_DECLARE_METATYPE( QgsLayoutItemGroup * )
 Q_DECLARE_METATYPE( QgsComposerPolygon * )
 Q_DECLARE_METATYPE( QgsLayoutItem * )