From e17b32c6e20e3e9b33d0bf4453b36cf307f3b57a Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Mon, 9 Oct 2017 15:02:34 +1000 Subject: [PATCH] Fix moving/resizing grouped items --- python/core/layout/qgslayoutitem.sip | 12 +- python/core/layout/qgslayoutitemgroup.sip | 10 +- src/core/layout/qgslayoutitem.h | 17 ++- src/core/layout/qgslayoutitemgroup.cpp | 150 +++++++++++++++++----- src/core/layout/qgslayoutitemgroup.h | 14 +- src/core/layout/qgslayoutsnapper.cpp | 13 +- src/gui/layout/qgslayoutmousehandles.cpp | 35 +++-- tests/src/core/testqgslayoutitemgroup.cpp | 115 +++++++++++++++++ 8 files changed, 305 insertions(+), 61 deletions(-) diff --git a/python/core/layout/qgslayoutitem.sip b/python/core/layout/qgslayoutitem.sip index 6c5440de9cf..43ca4af9b80 100644 --- a/python/core/layout/qgslayoutitem.sip +++ b/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 ); diff --git a/python/core/layout/qgslayoutitemgroup.sip b/python/core/layout/qgslayoutitemgroup.sip index 5d6c1abd728..bfcfb739fb7 100644 --- a/python/core/layout/qgslayoutitemgroup.sip +++ b/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 ); diff --git a/src/core/layout/qgslayoutitem.h b/src/core/layout/qgslayoutitem.h index f747a1ab378..997eaa340bf 100644 --- a/src/core/layout/qgslayoutitem.h +++ b/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 diff --git a/src/core/layout/qgslayoutitemgroup.cpp b/src/core/layout/qgslayoutitemgroup.cpp index e1cc2ffbd38..92673637627 100644 --- a/src/core/layout/qgslayoutitemgroup.cpp +++ b/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() ); } diff --git a/src/core/layout/qgslayoutitemgroup.h b/src/core/layout/qgslayoutitemgroup.h index 29d6551f70e..1ded6ec7df5 100644 --- a/src/core/layout/qgslayoutitemgroup.h +++ b/src/core/layout/qgslayoutitemgroup.h @@ -56,15 +56,23 @@ class CORE_EXPORT QgsLayoutItemGroup: public QgsLayoutItem QList 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; diff --git a/src/core/layout/qgslayoutsnapper.cpp b/src/core/layout/qgslayoutsnapper.cpp index bbb775a8830..72dd9e10824 100644 --- a/src/core/layout/qgslayoutsnapper.cpp +++ b/src/core/layout/qgslayoutsnapper.cpp @@ -328,16 +328,13 @@ double QgsLayoutSnapper::snapPointsToItems( const QList &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( currentItem ) ) { diff --git a/src/gui/layout/qgslayoutmousehandles.cpp b/src/gui/layout/qgslayoutmousehandles.cpp index 2a3786baa56..b2d65d5f781 100644 --- a/src/gui/layout/qgslayoutmousehandles.cpp +++ b/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 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; } diff --git a/tests/src/core/testqgslayoutitemgroup.cpp b/tests/src/core/testqgslayoutitemgroup.cpp index 433c6bbf085..6face17a176 100644 --- a/tests/src/core/testqgslayoutitemgroup.cpp +++ b/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 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 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 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 * )