Skip to content

Commit e453116

Browse files
committed
[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.
1 parent 6d93411 commit e453116

File tree

4 files changed

+114
-44
lines changed

4 files changed

+114
-44
lines changed

python/core/layout/qgslayoutsnapper.sip

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,11 @@ class QgsLayoutSnapper
7777
:rtype: QPointF
7878
%End
7979

80-
QPointF snapPointToGrid( QPointF point, double scaleFactor, bool &snapped /Out/ ) const;
80+
QPointF snapPointToGrid( QPointF point, double scaleFactor, bool &snappedX /Out/, bool &snappedY /Out/ ) const;
8181
%Docstring
8282
Snaps a layout coordinate ``point`` to the grid. If ``point``
83-
was snapped, ``snapped`` will be set to true.
83+
was snapped horizontally, ``snappedX`` will be set to true. If ``point``
84+
was snapped vertically, ``snappedY`` will be set to true.
8485

8586
The ``scaleFactor`` argument should be set to the transformation from
8687
scalar transform from layout coordinates to pixels, i.e. the

src/core/layout/qgslayoutsnapper.cpp

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -26,36 +26,43 @@ QPointF QgsLayoutSnapper::snapPoint( QPointF point, double scaleFactor, bool &sn
2626
{
2727
snapped = false;
2828

29-
// highest priority - grid
30-
bool snappedToGrid = false;
31-
QPointF res = snapPointToGrid( point, scaleFactor, snappedToGrid );
32-
if ( snappedToGrid )
29+
// highest priority - guides
30+
bool snappedXToGuides = false;
31+
double newX = snapPointToGuides( point.x(), QgsLayoutGuide::Vertical, scaleFactor, snappedXToGuides );
32+
if ( snappedXToGuides )
3333
{
3434
snapped = true;
35-
return res;
35+
point.setX( newX );
36+
}
37+
bool snappedYToGuides = false;
38+
double newY = snapPointToGuides( point.y(), QgsLayoutGuide::Horizontal, scaleFactor, snappedYToGuides );
39+
if ( snappedYToGuides )
40+
{
41+
snapped = true;
42+
point.setY( newY );
3643
}
3744

38-
bool snappedToHozGuides = false;
39-
double newX = snapPointToGuides( point.x(), QgsLayoutGuide::Vertical, scaleFactor, snappedToHozGuides );
40-
if ( snappedToHozGuides )
45+
bool snappedXToGrid = false;
46+
bool snappedYToGrid = false;
47+
QPointF res = snapPointToGrid( point, scaleFactor, snappedXToGrid, snappedYToGrid );
48+
if ( snappedXToGrid && !snappedXToGuides )
4149
{
4250
snapped = true;
43-
point.setX( newX );
51+
point.setX( res.x() );
4452
}
45-
bool snappedToVertGuides = false;
46-
double newY = snapPointToGuides( point.y(), QgsLayoutGuide::Horizontal, scaleFactor, snappedToVertGuides );
47-
if ( snappedToVertGuides )
53+
if ( snappedYToGrid && !snappedYToGuides )
4854
{
4955
snapped = true;
50-
point.setY( newY );
56+
point.setY( res.y() );
5157
}
5258

5359
return point;
5460
}
5561

56-
QPointF QgsLayoutSnapper::snapPointToGrid( QPointF point, double scaleFactor, bool &snapped ) const
62+
QPointF QgsLayoutSnapper::snapPointToGrid( QPointF point, double scaleFactor, bool &snappedX, bool &snappedY ) const
5763
{
58-
snapped = false;
64+
snappedX = false;
65+
snappedY = false;
5966
if ( !mLayout || !mSnapToGrid )
6067
{
6168
return point;
@@ -89,7 +96,7 @@ QPointF QgsLayoutSnapper::snapPointToGrid( QPointF point, double scaleFactor, bo
8996
}
9097
else
9198
{
92-
snapped = true;
99+
snappedX = true;
93100
}
94101
if ( fabs( ySnapped - point.y() ) > alignThreshold )
95102
{
@@ -98,7 +105,7 @@ QPointF QgsLayoutSnapper::snapPointToGrid( QPointF point, double scaleFactor, bo
98105
}
99106
else
100107
{
101-
snapped = true;
108+
snappedY = true;
102109
}
103110

104111
return QPointF( xSnapped, ySnapped );

src/core/layout/qgslayoutsnapper.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,8 @@ class CORE_EXPORT QgsLayoutSnapper
9090

9191
/**
9292
* Snaps a layout coordinate \a point to the grid. If \a point
93-
* was snapped, \a snapped will be set to true.
93+
* was snapped horizontally, \a snappedX will be set to true. If \a point
94+
* was snapped vertically, \a snappedY will be set to true.
9495
*
9596
* The \a scaleFactor argument should be set to the transformation from
9697
* scalar transform from layout coordinates to pixels, i.e. the
@@ -99,7 +100,7 @@ class CORE_EXPORT QgsLayoutSnapper
99100
* If snapToGrid() is disabled, this method will return the point
100101
* unchanged.
101102
*/
102-
QPointF snapPointToGrid( QPointF point, double scaleFactor, bool &snapped SIP_OUT ) const;
103+
QPointF snapPointToGrid( QPointF point, double scaleFactor, bool &snappedX SIP_OUT, bool &snappedY SIP_OUT ) const;
103104

104105
/**
105106
* Snaps a layout coordinate \a point to the grid. If \a point

tests/src/python/test_qgslayoutsnapper.py

Lines changed: 84 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,9 @@
2121
QgsLayoutMeasurement,
2222
QgsUnitTypes,
2323
QgsLayoutPoint,
24-
QgsLayoutItemPage)
24+
QgsLayoutItemPage,
25+
QgsLayoutGuide)
2526
from qgis.PyQt.QtCore import QPointF
26-
from qgis.PyQt.QtGui import (QPen,
27-
QColor)
2827

2928
from qgis.testing import start_app, unittest
3029

@@ -65,58 +64,112 @@ def testSnapPointToGrid(self):
6564
s.setSnapToGrid(True)
6665
s.setSnapTolerance(1)
6766

68-
point, snapped = s.snapPointToGrid(QPointF(1, 1), 1)
69-
self.assertTrue(snapped)
67+
point, snappedX, snappedY = s.snapPointToGrid(QPointF(1, 1), 1)
68+
self.assertTrue(snappedX)
69+
self.assertTrue(snappedY)
7070
self.assertEqual(point, QPointF(0, 0))
7171

72-
point, snapped = s.snapPointToGrid(QPointF(9, 1), 1)
73-
self.assertTrue(snapped)
72+
point, snappedX, snappedY = s.snapPointToGrid(QPointF(9, 1), 1)
73+
self.assertTrue(snappedX)
74+
self.assertTrue(snappedY)
7475
self.assertEqual(point, QPointF(10, 0))
7576

76-
point, snapped = s.snapPointToGrid(QPointF(1, 11), 1)
77-
self.assertTrue(snapped)
77+
point, snappedX, snappedY = s.snapPointToGrid(QPointF(1, 11), 1)
78+
self.assertTrue(snappedX)
79+
self.assertTrue(snappedY)
7880
self.assertEqual(point, QPointF(0, 10))
7981

80-
point, snapped = s.snapPointToGrid(QPointF(13, 11), 1)
81-
self.assertTrue(snapped)
82+
point, snappedX, snappedY = s.snapPointToGrid(QPointF(13, 11), 1)
83+
self.assertFalse(snappedX)
84+
self.assertTrue(snappedY)
8285
self.assertEqual(point, QPointF(13, 10))
8386

84-
point, snapped = s.snapPointToGrid(QPointF(11, 13), 1)
85-
self.assertTrue(snapped)
87+
point, snappedX, snappedY = s.snapPointToGrid(QPointF(11, 13), 1)
88+
self.assertTrue(snappedX)
89+
self.assertFalse(snappedY)
8690
self.assertEqual(point, QPointF(10, 13))
8791

88-
point, snapped = s.snapPointToGrid(QPointF(13, 23), 1)
89-
self.assertFalse(snapped)
92+
point, snappedX, snappedY = s.snapPointToGrid(QPointF(13, 23), 1)
93+
self.assertFalse(snappedX)
94+
self.assertFalse(snappedY)
9095
self.assertEqual(point, QPointF(13, 23))
9196

9297
# grid disabled
9398
s.setSnapToGrid(False)
94-
point, snapped = s.snapPointToGrid(QPointF(1, 1), 1)
95-
self.assertFalse(snapped)
99+
point, nappedX, snappedY = s.snapPointToGrid(QPointF(1, 1), 1)
100+
self.assertFalse(nappedX)
101+
self.assertFalse(snappedY)
96102
self.assertEqual(point, QPointF(1, 1))
97103
s.setSnapToGrid(True)
98104

99105
# with different pixel scale
100-
point, snapped = s.snapPointToGrid(QPointF(0.5, 0.5), 1)
101-
self.assertTrue(snapped)
106+
point, snappedX, snappedY = s.snapPointToGrid(QPointF(0.5, 0.5), 1)
107+
self.assertTrue(snappedX)
108+
self.assertTrue(snappedY)
102109
self.assertEqual(point, QPointF(0, 0))
103-
point, snapped = s.snapPointToGrid(QPointF(0.5, 0.5), 3)
104-
self.assertFalse(snapped)
110+
point, snappedX, snappedY = s.snapPointToGrid(QPointF(0.5, 0.5), 3)
111+
self.assertFalse(snappedX)
112+
self.assertFalse(snappedY)
105113
self.assertEqual(point, QPointF(0.5, 0.5))
106114

107115
# with offset grid
108116
l.gridSettings().setOffset(QgsLayoutPoint(2, 0))
109-
point, snapped = s.snapPointToGrid(QPointF(13, 23), 1)
110-
self.assertTrue(snapped)
117+
point, snappedX, snappedY = s.snapPointToGrid(QPointF(13, 23), 1)
118+
self.assertTrue(snappedX)
119+
self.assertFalse(snappedY)
111120
self.assertEqual(point, QPointF(12, 23))
112121

122+
def testSnapPointToGuides(self):
123+
p = QgsProject()
124+
l = QgsLayout(p)
125+
page = QgsLayoutItemPage(l)
126+
page.setPageSize('A4')
127+
l.pageCollection().addPage(page)
128+
s = QgsLayoutSnapper(l)
129+
guides = l.guides()
130+
131+
s.setSnapToGuides(True)
132+
s.setSnapTolerance(1)
133+
134+
# no guides
135+
point, snapped = s.snapPointToGuides(0.5, QgsLayoutGuide.Vertical, 1)
136+
self.assertFalse(snapped)
137+
138+
guides.addGuide(QgsLayoutGuide(QgsLayoutGuide.Vertical, QgsLayoutMeasurement(1)))
139+
point, snapped = s.snapPointToGuides(0.5, QgsLayoutGuide.Vertical, 1)
140+
self.assertTrue(snapped)
141+
self.assertEqual(point, 1)
142+
143+
# outside tolerance
144+
point, snapped = s.snapPointToGuides(5.5, QgsLayoutGuide.Vertical, 1)
145+
self.assertFalse(snapped)
146+
147+
# snapping off
148+
s.setSnapToGuides(False)
149+
point, snapped = s.snapPointToGuides(0.5, QgsLayoutGuide.Vertical, 1)
150+
self.assertFalse(snapped)
151+
152+
s.setSnapToGuides(True)
153+
# snap to hoz
154+
point, snapped = s.snapPointToGuides(0.5, QgsLayoutGuide.Horizontal, 1)
155+
self.assertFalse(snapped)
156+
guides.addGuide(QgsLayoutGuide(QgsLayoutGuide.Horizontal, QgsLayoutMeasurement(1)))
157+
point, snapped = s.snapPointToGuides(0.5, QgsLayoutGuide.Horizontal, 1)
158+
self.assertTrue(snapped)
159+
self.assertEqual(point, 1)
160+
161+
# with different pixel scale
162+
point, snapped = s.snapPointToGuides(0.5, QgsLayoutGuide.Horizontal, 3)
163+
self.assertFalse(snapped)
164+
113165
def testSnapPoint(self):
114166
p = QgsProject()
115167
l = QgsLayout(p)
116168
page = QgsLayoutItemPage(l)
117169
page.setPageSize('A4')
118170
l.pageCollection().addPage(page)
119171
s = QgsLayoutSnapper(l)
172+
guides = l.guides()
120173

121174
# first test snapping to grid
122175
l.gridSettings().setResolution(QgsLayoutMeasurement(5, QgsUnitTypes.LayoutMillimeters))
@@ -132,6 +185,14 @@ def testSnapPoint(self):
132185
self.assertFalse(snapped)
133186
self.assertEqual(point, QPointF(1, 1))
134187

188+
# test that guide takes precedence
189+
s.setSnapToGrid(True)
190+
s.setSnapToGuides(True)
191+
guides.addGuide(QgsLayoutGuide(QgsLayoutGuide.Horizontal, QgsLayoutMeasurement(0.5)))
192+
point, snapped = s.snapPoint(QPointF(1, 1), 1)
193+
self.assertTrue(snapped)
194+
self.assertEqual(point, QPointF(0, 0.5))
195+
135196

136197
if __name__ == '__main__':
137198
unittest.main()

0 commit comments

Comments
 (0)