New snapping modes: Centroid and middle of a segment (midpoint) #33622

lbartoletti · 2020-01-06T06:30:31Z

Description

This PR is part of my proposal to improve snapping modes in QGIS by getting closer to the experience of CAD tools.

It proposes two new modes. Snapping on the center of a geometry (centroid) and the middle of a segment.

To allow the selection of these new modes, I changed the interface of the combo box allowing to select several modes at the same time.

I took the opportunity to display the snap on an area that was present in previous versions but not selectable.

On the code side, QgsSnappingConfig::SnappingType become flags. It also changes the project file. I added a control in QgsSnappingConfig::readProject

The order of preference for snapping is as follows:

Vertex, Intersection
Middle
Centroid
Edge
Area

Sponsored by: Qwat group / Ville de Lausanne ( @ponceta @dsavary ) / Oslandia and some spare time

Checklist

Commit messages are descriptive and explain the rationale for changes
Commits which fix bugs include Fixes #11111 at the bottom of the commit message
I have read the QGIS Coding Standards and this PR complies with them
New unit tests have been added for core changes
I have run the scripts/prepare-commit.sh script before each commit
I have evaluated whether it is appropriate for this PR to be backported, backport requests are left as label or comment

nyalldawson · 2020-01-06T06:51:39Z

Very nice -- that's a welcome addition!

nirvn · 2020-01-06T06:54:38Z

Nice. How does the advanced snapping mode's layers list widget looks like now?

nirvn · 2020-01-06T08:40:35Z

@lbartoletti , if I wasn't clear, here's what I meant to refer to:

lbartoletti · 2020-01-06T08:49:13Z

Nice. How does the advanced snapping mode's layers list widget looks like now?

I only change texts in hte combobox to "No snapping" or "Snapping active"

nirvn · 2020-01-06T08:50:28Z

@lbartoletti , could you replace "snapping active" with a list of active snapping types?

lbartoletti · 2020-01-06T09:31:04Z

@lbartoletti , could you replace "snapping active" with a list of active snapping types?

@nirvn It was my first intentention, but I removed it becaus I thought that was a too long text when there is more than two For ex modes, "vertex, segment, middle, centroid". But I agree that it would be better than "active snapping". Do you have any ideas?

nirvn · 2020-01-06T09:33:53Z

@lbartoletti , what about clipping the string list to two items if more than two are selected? For e.g., "vertex, segment, etc."

src/core/qgssnappingconfig.h

lbartoletti · 2020-01-06T10:47:25Z

@lbartoletti , what about clipping the string list to two items if more than two are selected? For e.g., "vertex, segment, etc."

Done. Thanks.

3nids

Such a nice addition, love it :)
I am mainly commenting on the API and project.
I would let @wonder-sk look at the core snapping part.

I have some minor requests.

src/core/qgssnappingconfig.h

src/app/qgsoptions.cpp

src/app/qgssnappinglayertreemodel.cpp

src/core/qgssnappingconfig.h

Saijin-Naib · 2020-01-06T16:57:03Z

This is such an incredible quality-of-life improvement.

Thank you, so, so much!

python/core/auto_generated/qgssnappingconfig.sip.in

python/core/auto_generated/qgspointlocator.sip.in

nyalldawson · 2020-01-06T22:49:07Z

python/core/auto_generated/qgspointlocator.sip.in

@@ -238,6 +256,35 @@ This method is either blocking or non blocking according to ``relaxed`` paramete
 .. versionadded:: 3.6
 %End

+    MatchList centroidsInRect( const QgsRectangle &rect, QgsPointLocator::MatchFilter *filter = 0 );


Suggested change

MatchList centroidsInRect( const QgsRectangle &rect, QgsPointLocator::MatchFilter *filter = 0 );

MatchList centroidsInRectangle( const QgsRectangle &rectangle, QgsPointLocator::MatchFilter *filter = 0 );

(also with the other newly added methods)

I agree with you. But was to match with older methods, and I think it's better to be consistent with the old ways, isn't it? Maybe a note fore QGIS 4?

I'd say better do it now in this case, less refactoring is not bad :)

Yes, but it's an API break?

src/app/qgsoptions.cpp

src/core/qgspointlocator.cpp

nyalldawson · 2020-01-06T22:54:39Z

src/core/qgspointlocator.cpp

+      QgsGeometry *geom = mLocator->mGeoms.value( id );
+      QgsPointXY pt;
+      int afterVertex;
+      double sqrDist = geom->closestSegmentWithContext( mSrcPoint, pt, afterVertex, nullptr, POINT_LOC_EPSILON );


Hmm -- the closest segment won't necessarily equate to the closest segment's midpoint, will it? (Think a close but very long segment vs a slightly more distant but very short segment)

I don't think so, All my manuals tests worked well (tm)
But I'm interested in exploring it further if it comes to be confirmed.

src/core/qgspointlocator.h

src/core/qgssnappingutils.cpp

NathanW2 · 2020-01-06T23:44:49Z

src/core/qgspointlocator.cpp

wonder-sk

I am wondering what is the meaning of snapping to area - is it actually useful to have it as an option? Do I understand correctly that it would snap to any point within a polygon?

src/core/qgspointlocator.h

lbartoletti · 2020-01-16T06:46:10Z

I am wondering what is the meaning of snapping to area - is it actually useful to have it as an option? Do I understand correctly that it would snap to any point within a polygon?

Honnestyl, I discover it when I started to work on this feature... Yes it snaps to any point within a polygon.
Since it was there, I made it visible. There's no reason to remove it either.

m-kuhn · 2020-01-16T07:17:43Z

One use case I can see for snap to area is the possibility to draw a line or point on(to) a polygon and use attributes from this polygon in the default value expression trough @snapping_results.

qgis-bot · 2020-01-17T06:51:54Z

@lbartoletti
This pull request has been tagged as requiring documentation.

A documentation ticket will be opened at https://github.com/qgis/QGIS-Documentation when this PR is merged.

Please update the description (not the comments) with helpful description and screenshot to help the work from documentors.
Also, any commit having [needs-doc] or [Needs Documentation] in will see its message pushed to the issue, so please be as verbose as you can.

Thank you!

lbartoletti · 2020-02-07T05:40:11Z

@nyalldawson it's too late to be merged in 3.12?

nyalldawson · 2020-02-07T05:47:29Z

@lbartoletti that's not my decision to make -- you'll need to ask @jef-n

stale · 2020-02-21T06:42:23Z

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

that all unit tests are passing
that all comments by reviewers have been addressed
that there is enough information for reviewers, in particular
- link to any issues which this pull request fixes
- add a description of workflows which this pull request fixes
- add screenshots if applicable
that you have written unit tests where possible
In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
If there is no further activity on this pull request, it will be closed in a week.

lbartoletti · 2020-02-25T05:21:01Z

@3nids Denis, please, can you remove the "requested changes" since I can use scoped enum with QT 5.9?

@3nids @nyalldawson I bumped the version in my last commit but I'm not sure if it's OK if we want to merge it now?

3nids · 2020-02-25T15:52:59Z

for the parts I was involved, I'm for a merge.

lbartoletti · 2020-02-28T07:05:29Z

Thanks Denis.
Someone to push the merge button? ;)

qgis-bot · 2020-03-02T08:02:59Z

@lbartoletti
A documentation ticket has been opened at qgis/QGIS-Documentation#4950
It is your responsibility to visit this ticket and add as much detail as possible for the documentation team to correctly document this change.
Thank you!

mhugo · 2020-03-02T08:03:10Z

@lbartoletti I've pushed the button, thanks for your work !

nyalldawson reviewed Jan 6, 2020

View reviewed changes

src/core/qgssnappingconfig.h Show resolved Hide resolved

lbartoletti mentioned this pull request Jan 6, 2020

Nouveaux modes d'accrochage lbartoletti/QGIS#1

Closed

8 tasks

3nids requested changes Jan 6, 2020

View reviewed changes

nyalldawson reviewed Jan 6, 2020

View reviewed changes

github-actions bot added this to the 3.12.0 milestone Jan 10, 2020

nyalldawson added the Feature label Jan 13, 2020

lbartoletti force-pushed the snap_centroid branch from c72650e to 9877642 Compare January 15, 2020 13:25

wonder-sk reviewed Jan 15, 2020

View reviewed changes

src/core/qgspointlocator.cpp Outdated Show resolved Hide resolved

wonder-sk reviewed Jan 15, 2020

View reviewed changes

src/core/qgspointlocator.h Outdated Show resolved Hide resolved

src/core/qgspointlocator.h Outdated Show resolved Hide resolved

src/core/qgspointlocator.h Outdated Show resolved Hide resolved

lbartoletti force-pushed the snap_centroid branch from 9877642 to 336dfcc Compare January 15, 2020 19:32

Saijin-Naib mentioned this pull request Jan 16, 2020

Digitizing Tooltip Display - Snapping Layer & Snapping Method #33866

Open

lbartoletti force-pushed the snap_centroid branch from ab69591 to e53347b Compare January 17, 2020 06:11

lbartoletti requested review from 3nids and nyalldawson January 17, 2020 06:50

lbartoletti added Squash! Remember to squash this PR, instead of merging or rebasing Digitizing Related to feature digitizing map tools or functionality Needs Documentation When merging a labeled PR, an issue will be created in the Doc repo. labels Jan 17, 2020

lbartoletti force-pushed the snap_centroid branch from 7506787 to 272565f Compare January 23, 2020 17:05

lbartoletti closed this Jan 24, 2020

lbartoletti reopened this Jan 24, 2020

lbartoletti force-pushed the snap_centroid branch from 272565f to dd7a822 Compare January 24, 2020 14:53

lbartoletti closed this Jan 24, 2020

lbartoletti reopened this Jan 24, 2020

lbartoletti closed this Jan 27, 2020

lbartoletti reopened this Jan 27, 2020

3nids mentioned this pull request Feb 18, 2020

Snap midpoint #29697

Closed

stale bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Feb 21, 2020

lbartoletti removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Feb 21, 2020

lbartoletti modified the milestones: 3.12.0, 3.14.0 Feb 21, 2020

bump version

3d1e8b5

3nids approved these changes Feb 25, 2020

View reviewed changes

fix version

45e5722

mhugo merged commit dfa5cb6 into qgis:master Mar 2, 2020

qgis-bot mentioned this pull request Mar 2, 2020

Request in QGIS (New snapping modes: Centroid and middle of a segment (midpoint)) qgis/QGIS-Documentation#4950

Closed

6 tasks

lbartoletti deleted the snap_centroid branch March 2, 2020 08:07

timlinux added Changelog Items that are queued to appear in the visual changelog - remove after harvesting and removed Changelog Items that are queued to appear in the visual changelog - remove after harvesting labels May 22, 2020

lbartoletti mentioned this pull request May 18, 2022

Snap to midpoint of segment #23075

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New snapping modes: Centroid and middle of a segment (midpoint) #33622

New snapping modes: Centroid and middle of a segment (midpoint) #33622

lbartoletti commented Jan 6, 2020

nyalldawson commented Jan 6, 2020

nirvn commented Jan 6, 2020

nirvn commented Jan 6, 2020

lbartoletti commented Jan 6, 2020

nirvn commented Jan 6, 2020

lbartoletti commented Jan 6, 2020

nirvn commented Jan 6, 2020

lbartoletti commented Jan 6, 2020

3nids left a comment

Saijin-Naib commented Jan 6, 2020

nyalldawson Jan 6, 2020

nyalldawson Jan 6, 2020

lbartoletti Jan 14, 2020

3nids Jan 16, 2020

lbartoletti Jan 16, 2020

nyalldawson Jan 6, 2020

lbartoletti Jan 16, 2020

NathanW2 commented Jan 6, 2020

wonder-sk left a comment

lbartoletti commented Jan 16, 2020

m-kuhn commented Jan 16, 2020

qgis-bot commented Jan 17, 2020

lbartoletti commented Feb 7, 2020

nyalldawson commented Feb 7, 2020

stale bot commented Feb 21, 2020

lbartoletti commented Feb 25, 2020

3nids commented Feb 25, 2020

lbartoletti commented Feb 28, 2020

qgis-bot commented Mar 2, 2020

mhugo commented Mar 2, 2020

	MatchList centroidsInRect( const QgsRectangle &rect, QgsPointLocator::MatchFilter *filter = 0 );
	MatchList centroidsInRectangle( const QgsRectangle &rectangle, QgsPointLocator::MatchFilter *filter = 0 );

New snapping modes: Centroid and middle of a segment (midpoint) #33622

New snapping modes: Centroid and middle of a segment (midpoint) #33622

Conversation

lbartoletti commented Jan 6, 2020

Description

Checklist

nyalldawson commented Jan 6, 2020

nirvn commented Jan 6, 2020

nirvn commented Jan 6, 2020

lbartoletti commented Jan 6, 2020

nirvn commented Jan 6, 2020

lbartoletti commented Jan 6, 2020

nirvn commented Jan 6, 2020

lbartoletti commented Jan 6, 2020

3nids left a comment

Choose a reason for hiding this comment

Saijin-Naib commented Jan 6, 2020

nyalldawson Jan 6, 2020

Choose a reason for hiding this comment

nyalldawson Jan 6, 2020

Choose a reason for hiding this comment

lbartoletti Jan 14, 2020

Choose a reason for hiding this comment

3nids Jan 16, 2020

Choose a reason for hiding this comment

lbartoletti Jan 16, 2020

Choose a reason for hiding this comment

nyalldawson Jan 6, 2020

Choose a reason for hiding this comment

lbartoletti Jan 16, 2020

Choose a reason for hiding this comment

NathanW2 commented Jan 6, 2020

wonder-sk left a comment

Choose a reason for hiding this comment

lbartoletti commented Jan 16, 2020

m-kuhn commented Jan 16, 2020

qgis-bot commented Jan 17, 2020

lbartoletti commented Feb 7, 2020

nyalldawson commented Feb 7, 2020

stale bot commented Feb 21, 2020

lbartoletti commented Feb 25, 2020

3nids commented Feb 25, 2020

lbartoletti commented Feb 28, 2020

qgis-bot commented Mar 2, 2020

mhugo commented Mar 2, 2020