Fix items moving after altering page size or inserting/deleting pages

qgis · Dec 17, 2017 · 447a949 · 447a949
1 parent f649f1f
commit 447a949
Show file tree

Hide file tree

Showing 7 changed files with 232 additions and 1 deletion.
diff --git a/python/core/layout/qgslayoutpagecollection.sip b/python/core/layout/qgslayoutpagecollection.sip
@@ -184,6 +184,22 @@ Ownership is not transferred, and a copy of the symbol is created internally.
 Returns the symbol to use for drawing pages in the collection.
 
 .. seealso:: :py:func:`setPageStyleSymbol()`
+%End
+
+    void beginPageSizeChange();
+%Docstring
+ Should be called before changing any page item sizes, and followed by a call to
+ endPageSizeChange(). If page size changes are wrapped in these calls, then items
+ will maintain their same relative position on pages after the page sizes are updated.
+.. seealso:: :py:func:`endPageSizeChange()`
+%End
+
+    void endPageSizeChange();
+%Docstring
+ Should be called after changing any page item sizes, and preceded by a call to
+ beginPageSizeChange(). If page size changes are wrapped in these calls, then items
+ will maintain their same relative position on pages after the page sizes are updated.
+.. seealso:: :py:func:`beginPageSizeChange()`
 %End
 
     void reflow();

diff --git a/src/app/layout/qgslayoutpagepropertieswidget.cpp b/src/app/layout/qgslayoutpagepropertieswidget.cpp
@@ -138,10 +138,12 @@ void QgsLayoutPagePropertiesWidget::orientationChanged( int )
 
 void QgsLayoutPagePropertiesWidget::updatePageSize()
 {
+  mPage->layout()->pageCollection()->beginPageSizeChange();
   mPage->layout()->undoStack()->beginCommand( mPage, tr( "Change Page Size" ), 1 + mPage->layout()->pageCollection()->pageNumber( mPage ) );
   mPage->setPageSize( QgsLayoutSize( mWidthSpin->value(), mHeightSpin->value(), mSizeUnitsComboBox->unit() ) );
   mPage->layout()->undoStack()->endCommand();
   mPage->layout()->pageCollection()->reflow();
+  mPage->layout()->pageCollection()->endPageSizeChange();
 }
 
 void QgsLayoutPagePropertiesWidget::setToCustomSize()

diff --git a/src/core/composer/qgslayoutmanager.cpp b/src/core/composer/qgslayoutmanager.cpp
@@ -17,6 +17,7 @@
 #include "qgslayout.h"
 #include "qgsproject.h"
 #include "qgslogger.h"
+#include "qgslayoutundostack.h"
 
 QgsLayoutManager::QgsLayoutManager( QgsProject *project )
   : QObject( project )
@@ -193,11 +194,13 @@ bool QgsLayoutManager::readXml( const QDomElement &element, const QDomDocument &
   for ( int i = 0; i < layoutNodes.size(); ++i )
   {
     std::unique_ptr< QgsLayout > l = qgis::make_unique< QgsLayout >( mProject );
+    l->undoStack()->blockCommands( true );
     if ( !l->readXml( layoutNodes.at( i ).toElement(), doc, context ) )
     {
       result = false;
       continue;
     }
+    l->undoStack()->blockCommands( false );
     if ( addLayout( l.get() ) )
     {
       ( void )l.release(); // ownership was transferred successfully

diff --git a/src/core/layout/qgslayout.cpp b/src/core/layout/qgslayout.cpp
@@ -701,6 +701,9 @@ QList<QgsLayoutItem *> QgsLayout::ungroupItems( QgsLayoutItemGroup *group )
 void QgsLayout::refresh()
 {
   emit refreshed();
+  mPageCollection->beginPageSizeChange();
+  mPageCollection->reflow();
+  mPageCollection->endPageSizeChange();
   update();
 }
 

diff --git a/src/core/layout/qgslayoutpagecollection.cpp b/src/core/layout/qgslayoutpagecollection.cpp
@@ -54,6 +54,37 @@ void QgsLayoutPageCollection::setPageStyleSymbol( QgsFillSymbol *symbol )
 
 }
 
+void QgsLayoutPageCollection::beginPageSizeChange()
+{
+  mPreviousItemPositions.clear();
+  QList< QgsLayoutItem * > items;
+  mLayout->layoutItems( items );
+
+  for ( QgsLayoutItem *item : qgis::as_const( items ) )
+  {
+    if ( item->type() == QgsLayoutItemRegistry::LayoutPage )
+      continue;
+
+    mPreviousItemPositions.insert( item->uuid(), qMakePair( item->page(), item->pagePositionWithUnits() ) );
+  }
+}
+
+void QgsLayoutPageCollection::endPageSizeChange()
+{
+  for ( auto it = mPreviousItemPositions.constBegin(); it != mPreviousItemPositions.constEnd(); ++it )
+  {
+    if ( QgsLayoutItem *item = mLayout->itemByUuid( it.key() ) )
+    {
+      if ( !mBlockUndoCommands )
+        item->beginCommand( QString() );
+      item->attemptMove( it.value().second, true, false, it.value().first );
+      if ( !mBlockUndoCommands )
+        item->endCommand();
+    }
+  }
+  mPreviousItemPositions.clear();
+}
+
 void QgsLayoutPageCollection::reflow()
 {
   double currentY = 0;
@@ -526,11 +557,15 @@ QgsLayoutItemPage *QgsLayoutPageCollection::extendByNewPage()
 void QgsLayoutPageCollection::insertPage( QgsLayoutItemPage *page, int beforePage )
 {
   if ( !mBlockUndoCommands )
+  {
+    mLayout->undoStack()->beginMacro( tr( "Add Page" ) );
     mLayout->undoStack()->beginCommand( this, tr( "Add Page" ) );
+  }
 
   if ( beforePage < 0 )
     beforePage = 0;
 
+  beginPageSizeChange();
   if ( beforePage >= mPages.count() )
   {
     mPages.append( page );
@@ -541,8 +576,22 @@ void QgsLayoutPageCollection::insertPage( QgsLayoutItemPage *page, int beforePag
   }
   mLayout->addItem( page );
   reflow();
+
+  // bump up stored page numbers to account
+  for ( auto it = mPreviousItemPositions.begin(); it != mPreviousItemPositions.end(); ++it )
+  {
+    if ( it.value().first < beforePage )
+      continue;
+
+    it.value().first = it.value().first + 1;
+  }
+
+  endPageSizeChange();
   if ( ! mBlockUndoCommands )
+  {
     mLayout->undoStack()->endCommand();
+    mLayout->undoStack()->endMacro();
+  }
 }
 
 void QgsLayoutPageCollection::deletePage( int pageNumber )
@@ -556,10 +605,22 @@ void QgsLayoutPageCollection::deletePage( int pageNumber )
     mLayout->undoStack()->beginCommand( this, tr( "Remove Page" ) );
   }
   emit pageAboutToBeRemoved( pageNumber );
+  beginPageSizeChange();
   QgsLayoutItemPage *page = mPages.takeAt( pageNumber );
   mLayout->removeItem( page );
   page->deleteLater();
   reflow();
+
+  // bump stored page numbers to account
+  for ( auto it = mPreviousItemPositions.begin(); it != mPreviousItemPositions.end(); ++it )
+  {
+    if ( it.value().first <= pageNumber )
+      continue;
+
+    it.value().first = it.value().first - 1;
+  }
+
+  endPageSizeChange();
   if ( ! mBlockUndoCommands )
   {
     mLayout->undoStack()->endCommand();
@@ -577,10 +638,23 @@ void QgsLayoutPageCollection::deletePage( QgsLayoutItemPage *page )
     mLayout->undoStack()->beginMacro( tr( "Remove Page" ) );
     mLayout->undoStack()->beginCommand( this, tr( "Remove Page" ) );
   }
-  emit pageAboutToBeRemoved( mPages.indexOf( page ) );
+  int pageIndex = mPages.indexOf( page );
+  emit pageAboutToBeRemoved( pageIndex );
+  beginPageSizeChange();
   mPages.removeAll( page );
   page->deleteLater();
   reflow();
+
+  // bump stored page numbers to account
+  for ( auto it = mPreviousItemPositions.begin(); it != mPreviousItemPositions.end(); ++it )
+  {
+    if ( it.value().first <= pageIndex )
+      continue;
+
+    it.value().first = it.value().first - 1;
+  }
+
+  endPageSizeChange();
   if ( !mBlockUndoCommands )
   {
     mLayout->undoStack()->endCommand();

diff --git a/src/core/layout/qgslayoutpagecollection.h b/src/core/layout/qgslayoutpagecollection.h
@@ -24,6 +24,7 @@
 #include "qgslayoutitempage.h"
 #include "qgslayoutitem.h"
 #include "qgslayoutserializableobject.h"
+#include "qgslayoutpoint.h"
 #include <QObject>
 #include <memory>
 
@@ -220,6 +221,22 @@ class CORE_EXPORT QgsLayoutPageCollection : public QObject, public QgsLayoutSeri
      */
     const QgsFillSymbol *pageStyleSymbol() const { return mPageStyleSymbol.get(); }
 
+    /**
+     * Should be called before changing any page item sizes, and followed by a call to
+     * endPageSizeChange(). If page size changes are wrapped in these calls, then items
+     * will maintain their same relative position on pages after the page sizes are updated.
+     * \see endPageSizeChange()
+     */
+    void beginPageSizeChange();
+
+    /**
+     * Should be called after changing any page item sizes, and preceded by a call to
+     * beginPageSizeChange(). If page size changes are wrapped in these calls, then items
+     * will maintain their same relative position on pages after the page sizes are updated.
+     * \see beginPageSizeChange()
+     */
+    void endPageSizeChange();
+
     /**
      * Forces the page collection to reflow the arrangement of pages, e.g. to account
      * for page size/orientation change.
@@ -391,6 +408,8 @@ class CORE_EXPORT QgsLayoutPageCollection : public QObject, public QgsLayoutSeri
 
     bool mBlockUndoCommands = false;
 
+    QMap< QString, QPair< int, QgsLayoutPoint > > mPreviousItemPositions;
+
     void createDefaultPageStyleSymbol();
 
     friend class QgsLayoutPageCollectionUndoCommand;

diff --git a/tests/src/python/test_qgslayoutpagecollection.py b/tests/src/python/test_qgslayoutpagecollection.py
@@ -365,6 +365,120 @@ def testReflow(self):
         self.assertEqual(page3.pos().x(), 0)
         self.assertEqual(page3.pos().y(), 130)
 
+    def testInsertPageWithItems(self):
+        p = QgsProject()
+        l = QgsLayout(p)
+        collection = l.pageCollection()
+
+        # add a page
+        page = QgsLayoutItemPage(l)
+        page.setPageSize('A4')
+        collection.addPage(page)
+        page2 = QgsLayoutItemPage(l)
+        page2.setPageSize('A5')
+        collection.addPage(page2)
+
+        # item on pages
+        shape1 = QgsLayoutItemShape(l)
+        shape1.attemptResize(QgsLayoutSize(90, 50))
+        shape1.attemptMove(QgsLayoutPoint(90, 50), page=0)
+        l.addLayoutItem(shape1)
+
+        shape2 = QgsLayoutItemShape(l)
+        shape2.attemptResize(QgsLayoutSize(110, 50))
+        shape2.attemptMove(QgsLayoutPoint(100, 150), page=1)
+        l.addLayoutItem(shape2)
+
+        self.assertEqual(shape1.page(), 0)
+        self.assertEqual(shape2.page(), 1)
+
+        # third page, slotted in middle
+        page3 = QgsLayoutItemPage(l)
+        page3.setPageSize('A3')
+        collection.insertPage(page3, 0)
+
+        # check item position
+        self.assertEqual(shape1.page(), 1)
+        self.assertEqual(shape1.pagePositionWithUnits(), QgsLayoutPoint(90, 50))
+        self.assertEqual(shape2.page(), 2)
+        self.assertEqual(shape2.pagePositionWithUnits(), QgsLayoutPoint(100, 150))
+
+    def testDeletePageWithItems(self):
+        p = QgsProject()
+        l = QgsLayout(p)
+        collection = l.pageCollection()
+
+        # add a page
+        page = QgsLayoutItemPage(l)
+        page.setPageSize('A4')
+        collection.addPage(page)
+        page2 = QgsLayoutItemPage(l)
+        page2.setPageSize('A4')
+        collection.addPage(page2)
+        page3 = QgsLayoutItemPage(l)
+        page3.setPageSize('A4')
+        collection.addPage(page3)
+
+        # item on pages
+        shape1 = QgsLayoutItemShape(l)
+        shape1.attemptResize(QgsLayoutSize(90, 50))
+        shape1.attemptMove(QgsLayoutPoint(90, 50), page=0)
+        l.addLayoutItem(shape1)
+
+        shape2 = QgsLayoutItemShape(l)
+        shape2.attemptResize(QgsLayoutSize(110, 50))
+        shape2.attemptMove(QgsLayoutPoint(100, 150), page=2)
+        l.addLayoutItem(shape2)
+
+        self.assertEqual(shape1.page(), 0)
+        self.assertEqual(shape2.page(), 2)
+
+        collection.deletePage(1)
+
+        # check item position
+        self.assertEqual(shape1.page(), 0)
+        self.assertEqual(shape1.pagePositionWithUnits(), QgsLayoutPoint(90, 50))
+        self.assertEqual(shape2.page(), 1)
+        self.assertEqual(shape2.pagePositionWithUnits(), QgsLayoutPoint(100, 150))
+
+    def testDeletePageWithItems2(self):
+        p = QgsProject()
+        l = QgsLayout(p)
+        collection = l.pageCollection()
+
+        # add a page
+        page = QgsLayoutItemPage(l)
+        page.setPageSize('A4')
+        collection.addPage(page)
+        page2 = QgsLayoutItemPage(l)
+        page2.setPageSize('A4')
+        collection.addPage(page2)
+        page3 = QgsLayoutItemPage(l)
+        page3.setPageSize('A4')
+        collection.addPage(page3)
+
+        # item on pages
+        shape1 = QgsLayoutItemShape(l)
+        shape1.attemptResize(QgsLayoutSize(90, 50))
+        shape1.attemptMove(QgsLayoutPoint(90, 50), page=0)
+        l.addLayoutItem(shape1)
+
+        shape2 = QgsLayoutItemShape(l)
+        shape2.attemptResize(QgsLayoutSize(110, 50))
+        shape2.attemptMove(QgsLayoutPoint(100, 150), page=2)
+        l.addLayoutItem(shape2)
+
+        self.assertEqual(shape1.page(), 0)
+        self.assertEqual(shape2.page(), 2)
+
+        collection.deletePage(page2)
+
+        # check item position
+        self.assertEqual(shape1.page(), 0)
+        self.assertEqual(shape1.pagePositionWithUnits(), QgsLayoutPoint(90, 50))
+        self.assertEqual(shape2.page(), 1)
+        self.assertEqual(shape2.pagePositionWithUnits(), QgsLayoutPoint(100, 150))
+
     def testDataDefinedSize(self):
         p = QgsProject()
         l = QgsLayout(p)