Fix guide positioning when multiple pages are present

nyalldawson · nyalldawson · commit 200669aefae4 · 2017-12-18T17:39:45.000+10:00
diff --git a/src/core/layout/qgslayoutguidecollection.cpp b/src/core/layout/qgslayoutguidecollection.cpp
@@ -89,7 +89,7 @@ void QgsLayoutGuide::update()
       }
       else
       {
-        mLineItem->setLine( 0, layoutPos, mPage->rect().width(), layoutPos );
+        mLineItem->setLine( 0, layoutPos + mPage->y(), mPage->rect().width(), layoutPos + mPage->y() );
         mLineItem->setVisible( showGuide );
       }
 
@@ -102,7 +102,7 @@ void QgsLayoutGuide::update()
       }
       else
       {
-        mLineItem->setLine( layoutPos, 0, layoutPos, mPage->rect().height() );
+        mLineItem->setLine( layoutPos, mPage->y(), layoutPos, mPage->y() + mPage->rect().height() );
         mLineItem->setVisible( showGuide );
       }
 
diff --git a/src/core/layout/qgslayoutpagecollection.cpp b/src/core/layout/qgslayoutpagecollection.cpp
@@ -95,6 +95,7 @@ void QgsLayoutPageCollection::reflow()
     currentY += mLayout->convertToLayoutUnits( page->pageSize() ).height() + spaceBetweenPages();
     p.setY( currentY );
   }
+  mLayout->guides().update();
   mLayout->updateBounds();
   emit changed();
 }
@@ -193,7 +194,8 @@ int QgsLayoutPageCollection::predictPageNumberForPoint( QPointF point ) const
 
 QgsLayoutItemPage *QgsLayoutPageCollection::pageAtPoint( QPointF point ) const
 {
-  Q_FOREACH ( QGraphicsItem *item, mLayout->items( point ) )
+  const QList< QGraphicsItem * > items = mLayout->items( point );
+  for ( QGraphicsItem *item : items )
   {
     if ( item->type() == QgsLayoutItemRegistry::LayoutPage )
     {
diff --git a/tests/src/python/test_qgslayoutguides.py b/tests/src/python/test_qgslayoutguides.py
@@ -65,6 +65,11 @@ def testUpdateGuide(self):
         p = QgsProject()
         l = QgsLayout(p)
         l.initializeDefaults() # add a page
+        # add a second page
+        page2 = QgsLayoutItemPage(l)
+        page2.setPageSize('A5')
+        l.pageCollection().addPage(page2)
+
         g = QgsLayoutGuide(Qt.Horizontal, QgsLayoutMeasurement(5, QgsUnitTypes.LayoutCentimeters), l.pageCollection().page(0))
         g.setLayout(l)
         g.update()
@@ -85,6 +90,19 @@ def testUpdateGuide(self):
         self.assertEqual(g.item().line().y2(), 15)
         self.assertEqual(g.layoutPosition(), 15)
 
+        # guide on page2
+        g1 = QgsLayoutGuide(Qt.Horizontal, QgsLayoutMeasurement(5, QgsUnitTypes.LayoutCentimeters), l.pageCollection().page(1))
+        g1.setLayout(l)
+        g1.update()
+        g1.setPosition(QgsLayoutMeasurement(15, QgsUnitTypes.LayoutMillimeters))
+        g1.update()
+        self.assertTrue(g1.item().isVisible())
+        self.assertEqual(g1.item().line().x1(), 0)
+        self.assertEqual(g1.item().line().y1(), 235)
+        self.assertEqual(g1.item().line().x2(), 148)
+        self.assertEqual(g1.item().line().y2(), 235)
+        self.assertEqual(g1.layoutPosition(), 235)
+
         # vertical guide
         g2 = QgsLayoutGuide(Qt.Vertical, QgsLayoutMeasurement(5, QgsUnitTypes.LayoutCentimeters), l.pageCollection().page(0))
         g2.setLayout(l)
@@ -109,6 +127,17 @@ def testUpdateGuide(self):
         g.update()
         self.assertFalse(g.item().isVisible())
 
+        # guide on page2
+        g3 = QgsLayoutGuide(Qt.Vertical, QgsLayoutMeasurement(5, QgsUnitTypes.LayoutCentimeters), l.pageCollection().page(1))
+        g3.setLayout(l)
+        g3.update()
+        self.assertTrue(g3.item().isVisible())
+        self.assertEqual(g3.item().line().x1(), 50)
+        self.assertEqual(g3.item().line().y1(), 220)
+        self.assertEqual(g3.item().line().x2(), 50)
+        self.assertEqual(g3.item().line().y2(), 430)
+        self.assertEqual(g3.layoutPosition(), 50)
+
     def testCollection(self):
         p = QgsProject()
         l = QgsLayout(p)

Original file line number	Diff line number	Diff line change
`@@ -89,7 +89,7 @@ void QgsLayoutGuide::update()`
`89`	`89`	`}`
`90`	`90`	`else`
`91`	`91`	`{`
`92`		`- mLineItem->setLine( 0, layoutPos, mPage->rect().width(), layoutPos );`
	`92`	`+ mLineItem->setLine( 0, layoutPos + mPage->y(), mPage->rect().width(), layoutPos + mPage->y() );`
`93`	`93`	`mLineItem->setVisible( showGuide );`
`94`	`94`	`}`
`95`	`95`
`@@ -102,7 +102,7 @@ void QgsLayoutGuide::update()`
`102`	`102`	`}`
`103`	`103`	`else`
`104`	`104`	`{`
`105`		`- mLineItem->setLine( layoutPos, 0, layoutPos, mPage->rect().height() );`
	`105`	`+ mLineItem->setLine( layoutPos, mPage->y(), layoutPos, mPage->y() + mPage->rect().height() );`
`106`	`106`	`mLineItem->setVisible( showGuide );`
`107`	`107`	`}`
`108`	`108`
Original file line number	Diff line number	Diff line change
`@@ -95,6 +95,7 @@ void QgsLayoutPageCollection::reflow()`
`95`	`95`	`currentY += mLayout->convertToLayoutUnits( page->pageSize() ).height() + spaceBetweenPages();`
`96`	`96`	`p.setY( currentY );`
`97`	`97`	`}`
	`98`	`+ mLayout->guides().update();`
`98`	`99`	`mLayout->updateBounds();`
`99`	`100`	`emit changed();`
`100`	`101`	`}`
`@@ -193,7 +194,8 @@ int QgsLayoutPageCollection::predictPageNumberForPoint( QPointF point ) const`
`193`	`194`
`194`	`195`	`QgsLayoutItemPage *QgsLayoutPageCollection::pageAtPoint( QPointF point ) const`
`195`	`196`	`{`
`196`		`- Q_FOREACH ( QGraphicsItem *item, mLayout->items( point ) )`
	`197`	`+ const QList< QGraphicsItem * > items = mLayout->items( point );`
	`198`	`+ for ( QGraphicsItem *item : items )`
`197`	`199`	`{`
`198`	`200`	`if ( item->type() == QgsLayoutItemRegistry::LayoutPage )`
`199`	`201`	`{`