From e453116101cac385973e3d9f3700b41f542f8368 Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Thu, 27 Jul 2017 12:42:31 +1000 Subject: [PATCH] [needs-docs] Refine snapping logic for layouts Previously grids would always take precedence when both a grid and guide were within tolerance of a point. Now, guides will always take precedence - since they have been manually set by users we make the assumption that they have been explicitly placed at highly desirable snapping locations, and should be selected over the general grid. Additionally, grid snapping was previously only done if BOTH x and y could be snapped to the grid. We now snap to the nearest grid line for x/y separately. This means if a point is close to a vertical grid line but not a horizontal one it will still snap to that nearby vertical grid line. --- python/core/layout/qgslayoutsnapper.sip | 5 +- src/core/layout/qgslayoutsnapper.cpp | 45 +++++---- src/core/layout/qgslayoutsnapper.h | 5 +- tests/src/python/test_qgslayoutsnapper.py | 107 +++++++++++++++++----- 4 files changed, 116 insertions(+), 46 deletions(-) diff --git a/python/core/layout/qgslayoutsnapper.sip b/python/core/layout/qgslayoutsnapper.sip index 9546f207e94..f73913889b3 100644 --- a/python/core/layout/qgslayoutsnapper.sip +++ b/python/core/layout/qgslayoutsnapper.sip @@ -77,10 +77,11 @@ class QgsLayoutSnapper :rtype: QPointF %End - QPointF snapPointToGrid( QPointF point, double scaleFactor, bool &snapped /Out/ ) const; + QPointF snapPointToGrid( QPointF point, double scaleFactor, bool &snappedX /Out/, bool &snappedY /Out/ ) const; %Docstring Snaps a layout coordinate ``point`` to the grid. If ``point`` - was snapped, ``snapped`` will be set to true. + was snapped horizontally, ``snappedX`` will be set to true. If ``point`` + was snapped vertically, ``snappedY`` will be set to true. The ``scaleFactor`` argument should be set to the transformation from scalar transform from layout coordinates to pixels, i.e. the diff --git a/src/core/layout/qgslayoutsnapper.cpp b/src/core/layout/qgslayoutsnapper.cpp index b15f88635fe..f7e6da14d84 100644 --- a/src/core/layout/qgslayoutsnapper.cpp +++ b/src/core/layout/qgslayoutsnapper.cpp @@ -26,36 +26,43 @@ QPointF QgsLayoutSnapper::snapPoint( QPointF point, double scaleFactor, bool &sn { snapped = false; - // highest priority - grid - bool snappedToGrid = false; - QPointF res = snapPointToGrid( point, scaleFactor, snappedToGrid ); - if ( snappedToGrid ) - { - snapped = true; - return res; - } - - bool snappedToHozGuides = false; - double newX = snapPointToGuides( point.x(), QgsLayoutGuide::Vertical, scaleFactor, snappedToHozGuides ); - if ( snappedToHozGuides ) + // highest priority - guides + bool snappedXToGuides = false; + double newX = snapPointToGuides( point.x(), QgsLayoutGuide::Vertical, scaleFactor, snappedXToGuides ); + if ( snappedXToGuides ) { snapped = true; point.setX( newX ); } - bool snappedToVertGuides = false; - double newY = snapPointToGuides( point.y(), QgsLayoutGuide::Horizontal, scaleFactor, snappedToVertGuides ); - if ( snappedToVertGuides ) + bool snappedYToGuides = false; + double newY = snapPointToGuides( point.y(), QgsLayoutGuide::Horizontal, scaleFactor, snappedYToGuides ); + if ( snappedYToGuides ) { snapped = true; point.setY( newY ); } + bool snappedXToGrid = false; + bool snappedYToGrid = false; + QPointF res = snapPointToGrid( point, scaleFactor, snappedXToGrid, snappedYToGrid ); + if ( snappedXToGrid && !snappedXToGuides ) + { + snapped = true; + point.setX( res.x() ); + } + if ( snappedYToGrid && !snappedYToGuides ) + { + snapped = true; + point.setY( res.y() ); + } + return point; } -QPointF QgsLayoutSnapper::snapPointToGrid( QPointF point, double scaleFactor, bool &snapped ) const +QPointF QgsLayoutSnapper::snapPointToGrid( QPointF point, double scaleFactor, bool &snappedX, bool &snappedY ) const { - snapped = false; + snappedX = false; + snappedY = false; if ( !mLayout || !mSnapToGrid ) { return point; @@ -89,7 +96,7 @@ QPointF QgsLayoutSnapper::snapPointToGrid( QPointF point, double scaleFactor, bo } else { - snapped = true; + snappedX = true; } if ( fabs( ySnapped - point.y() ) > alignThreshold ) { @@ -98,7 +105,7 @@ QPointF QgsLayoutSnapper::snapPointToGrid( QPointF point, double scaleFactor, bo } else { - snapped = true; + snappedY = true; } return QPointF( xSnapped, ySnapped ); diff --git a/src/core/layout/qgslayoutsnapper.h b/src/core/layout/qgslayoutsnapper.h index 6b3518de6d3..b280a941439 100644 --- a/src/core/layout/qgslayoutsnapper.h +++ b/src/core/layout/qgslayoutsnapper.h @@ -90,7 +90,8 @@ class CORE_EXPORT QgsLayoutSnapper /** * Snaps a layout coordinate \a point to the grid. If \a point - * was snapped, \a snapped will be set to true. + * was snapped horizontally, \a snappedX will be set to true. If \a point + * was snapped vertically, \a snappedY will be set to true. * * The \a scaleFactor argument should be set to the transformation from * scalar transform from layout coordinates to pixels, i.e. the @@ -99,7 +100,7 @@ class CORE_EXPORT QgsLayoutSnapper * If snapToGrid() is disabled, this method will return the point * unchanged. */ - QPointF snapPointToGrid( QPointF point, double scaleFactor, bool &snapped SIP_OUT ) const; + QPointF snapPointToGrid( QPointF point, double scaleFactor, bool &snappedX SIP_OUT, bool &snappedY SIP_OUT ) const; /** * Snaps a layout coordinate \a point to the grid. If \a point diff --git a/tests/src/python/test_qgslayoutsnapper.py b/tests/src/python/test_qgslayoutsnapper.py index e5ebeacfde6..d70b9d528bf 100644 --- a/tests/src/python/test_qgslayoutsnapper.py +++ b/tests/src/python/test_qgslayoutsnapper.py @@ -21,10 +21,9 @@ from qgis.core import (QgsProject, QgsLayoutMeasurement, QgsUnitTypes, QgsLayoutPoint, - QgsLayoutItemPage) + QgsLayoutItemPage, + QgsLayoutGuide) from qgis.PyQt.QtCore import QPointF -from qgis.PyQt.QtGui import (QPen, - QColor) from qgis.testing import start_app, unittest @@ -65,51 +64,104 @@ class TestQgsLayoutSnapper(unittest.TestCase): s.setSnapToGrid(True) s.setSnapTolerance(1) - point, snapped = s.snapPointToGrid(QPointF(1, 1), 1) - self.assertTrue(snapped) + point, snappedX, snappedY = s.snapPointToGrid(QPointF(1, 1), 1) + self.assertTrue(snappedX) + self.assertTrue(snappedY) self.assertEqual(point, QPointF(0, 0)) - point, snapped = s.snapPointToGrid(QPointF(9, 1), 1) - self.assertTrue(snapped) + point, snappedX, snappedY = s.snapPointToGrid(QPointF(9, 1), 1) + self.assertTrue(snappedX) + self.assertTrue(snappedY) self.assertEqual(point, QPointF(10, 0)) - point, snapped = s.snapPointToGrid(QPointF(1, 11), 1) - self.assertTrue(snapped) + point, snappedX, snappedY = s.snapPointToGrid(QPointF(1, 11), 1) + self.assertTrue(snappedX) + self.assertTrue(snappedY) self.assertEqual(point, QPointF(0, 10)) - point, snapped = s.snapPointToGrid(QPointF(13, 11), 1) - self.assertTrue(snapped) + point, snappedX, snappedY = s.snapPointToGrid(QPointF(13, 11), 1) + self.assertFalse(snappedX) + self.assertTrue(snappedY) self.assertEqual(point, QPointF(13, 10)) - point, snapped = s.snapPointToGrid(QPointF(11, 13), 1) - self.assertTrue(snapped) + point, snappedX, snappedY = s.snapPointToGrid(QPointF(11, 13), 1) + self.assertTrue(snappedX) + self.assertFalse(snappedY) self.assertEqual(point, QPointF(10, 13)) - point, snapped = s.snapPointToGrid(QPointF(13, 23), 1) - self.assertFalse(snapped) + point, snappedX, snappedY = s.snapPointToGrid(QPointF(13, 23), 1) + self.assertFalse(snappedX) + self.assertFalse(snappedY) self.assertEqual(point, QPointF(13, 23)) # grid disabled s.setSnapToGrid(False) - point, snapped = s.snapPointToGrid(QPointF(1, 1), 1) - self.assertFalse(snapped) + point, nappedX, snappedY = s.snapPointToGrid(QPointF(1, 1), 1) + self.assertFalse(nappedX) + self.assertFalse(snappedY) self.assertEqual(point, QPointF(1, 1)) s.setSnapToGrid(True) # with different pixel scale - point, snapped = s.snapPointToGrid(QPointF(0.5, 0.5), 1) - self.assertTrue(snapped) + point, snappedX, snappedY = s.snapPointToGrid(QPointF(0.5, 0.5), 1) + self.assertTrue(snappedX) + self.assertTrue(snappedY) self.assertEqual(point, QPointF(0, 0)) - point, snapped = s.snapPointToGrid(QPointF(0.5, 0.5), 3) - self.assertFalse(snapped) + point, snappedX, snappedY = s.snapPointToGrid(QPointF(0.5, 0.5), 3) + self.assertFalse(snappedX) + self.assertFalse(snappedY) self.assertEqual(point, QPointF(0.5, 0.5)) # with offset grid l.gridSettings().setOffset(QgsLayoutPoint(2, 0)) - point, snapped = s.snapPointToGrid(QPointF(13, 23), 1) - self.assertTrue(snapped) + point, snappedX, snappedY = s.snapPointToGrid(QPointF(13, 23), 1) + self.assertTrue(snappedX) + self.assertFalse(snappedY) self.assertEqual(point, QPointF(12, 23)) + def testSnapPointToGuides(self): + p = QgsProject() + l = QgsLayout(p) + page = QgsLayoutItemPage(l) + page.setPageSize('A4') + l.pageCollection().addPage(page) + s = QgsLayoutSnapper(l) + guides = l.guides() + + s.setSnapToGuides(True) + s.setSnapTolerance(1) + + # no guides + point, snapped = s.snapPointToGuides(0.5, QgsLayoutGuide.Vertical, 1) + self.assertFalse(snapped) + + guides.addGuide(QgsLayoutGuide(QgsLayoutGuide.Vertical, QgsLayoutMeasurement(1))) + point, snapped = s.snapPointToGuides(0.5, QgsLayoutGuide.Vertical, 1) + self.assertTrue(snapped) + self.assertEqual(point, 1) + + # outside tolerance + point, snapped = s.snapPointToGuides(5.5, QgsLayoutGuide.Vertical, 1) + self.assertFalse(snapped) + + # snapping off + s.setSnapToGuides(False) + point, snapped = s.snapPointToGuides(0.5, QgsLayoutGuide.Vertical, 1) + self.assertFalse(snapped) + + s.setSnapToGuides(True) + # snap to hoz + point, snapped = s.snapPointToGuides(0.5, QgsLayoutGuide.Horizontal, 1) + self.assertFalse(snapped) + guides.addGuide(QgsLayoutGuide(QgsLayoutGuide.Horizontal, QgsLayoutMeasurement(1))) + point, snapped = s.snapPointToGuides(0.5, QgsLayoutGuide.Horizontal, 1) + self.assertTrue(snapped) + self.assertEqual(point, 1) + + # with different pixel scale + point, snapped = s.snapPointToGuides(0.5, QgsLayoutGuide.Horizontal, 3) + self.assertFalse(snapped) + def testSnapPoint(self): p = QgsProject() l = QgsLayout(p) @@ -117,6 +169,7 @@ class TestQgsLayoutSnapper(unittest.TestCase): page.setPageSize('A4') l.pageCollection().addPage(page) s = QgsLayoutSnapper(l) + guides = l.guides() # first test snapping to grid l.gridSettings().setResolution(QgsLayoutMeasurement(5, QgsUnitTypes.LayoutMillimeters)) @@ -132,6 +185,14 @@ class TestQgsLayoutSnapper(unittest.TestCase): self.assertFalse(snapped) self.assertEqual(point, QPointF(1, 1)) + # test that guide takes precedence + s.setSnapToGrid(True) + s.setSnapToGuides(True) + guides.addGuide(QgsLayoutGuide(QgsLayoutGuide.Horizontal, QgsLayoutMeasurement(0.5))) + point, snapped = s.snapPoint(QPointF(1, 1), 1) + self.assertTrue(snapped) + self.assertEqual(point, QPointF(0, 0.5)) + if __name__ == '__main__': unittest.main()