Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

enable snapping in georeferenceer #60156

Merged
merged 2 commits into from
Jan 16, 2025
Merged

enable snapping in georeferenceer #60156

merged 2 commits into from
Jan 16, 2025

Conversation

3nids
Copy link
Member

@3nids 3nids commented Jan 15, 2025

This enables snapping and advanced dock widget in the georeferencer.

georef-snapping.mov

Also fixes a regression from #58755 where snapping in QgsMapToolCapture was not working anymore in a scenario without an active layer.
Test added for this.

@3nids 3nids added the Feature label Jan 15, 2025
@github-actions github-actions bot added this to the 3.42.0 milestone Jan 15, 2025
@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 15, 2025 •
edited
Loading

🪟 Windows builds

Download Windows builds of this PR for testing.
Debug symbols for this build are available here.
(Built from commit 8170764)

🪟 Windows Qt6 builds

Download Windows Qt6 builds of this PR for testing.
(Built from commit 8170764)

@pigreco pigreco mentioned this pull request Jan 15, 2025
Closed
@albertograva albertograva mentioned this pull request Jan 15, 2025
Open
@agiudiceandrea
Copy link
Contributor

agiudiceandrea commented Jan 15, 2025 •
edited
Loading

Great! This PR fixes #52306.

lbartoletti
lbartoletti approved these changes Jan 15, 2025
View reviewed changes
Copy link
Member

@lbartoletti lbartoletti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK for me, except one comment.

This PR could be splited (one for the fix and one to enable snapping in georef), but go ahead.

src/app/georeferencer/qgsgeorefmainwindow.cpp
QgsSnappingConfig snappingConfig;
snappingConfig.setMode( Qgis::SnappingMode::AllLayers );
snappingConfig.setTypeFlag( Qgis::SnappingType::Vertex );
snappingConfig.setTolerance( 10 );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't you use the default snaping tolerance?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't find any default snapping tolerance!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be "default-snapping-tolerance" available in Options->Map Tools->Digitizing->Snapping

@agiudiceandrea
Copy link
Contributor

@3nids, since this PR also fixes a regression from #58755, which was merged in 3.40, shouldn't that part be backported to release-3_40 branch?

@3nids
Copy link
Member Author

3nids commented Jan 16, 2025

@agiudiceandrea a better fix has been pushed by @benwirf in #60166, we'll move the test there so it can be backported.
So only the feature will remain here.

@pigreco
Copy link
Contributor

pigreco commented Jan 16, 2025

I downloaded the Build window and I see that the vertex snap is always active, it would be useful to be able to activate and deactivate it as well as change the type of snap.
always thanks

@3nids
Copy link
Member Author

3nids commented Jan 16, 2025

@pigreco here you go

georef-snapp-2.mov

@albertograva
Copy link

thanx a lot @3nids !!!

@3nids 3nids requested a review from lbartoletti January 16, 2025 10:50
@3nids 3nids added the Needs Documentation When merging a labeled PR, an issue will be created in the Doc repo. label Jan 16, 2025
@qgis-bot
Copy link
Collaborator

@3nids
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!

@3nids
Copy link
Member Author

3nids commented Jan 16, 2025

@lbartoletti do you want to re-check with the last commit?

@3nids 3nids merged commit 1feaf51 into qgis:master Jan 16, 2025
35 checks passed
@3nids 3nids deleted the georef-snapping branch January 16, 2025 14:33
@qgis-bot
Copy link
Collaborator

@3nids
A documentation ticket has been opened at qgis/QGIS-Documentation#9564
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!

@qgis-bot qgis-bot mentioned this pull request Jan 16, 2025
Open
@3nids 3nids mentioned this pull request Jan 17, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lbartoletti lbartoletti lbartoletti approved these changes

Assignees
No one assigned
Labels
Feature Needs Documentation When merging a labeled PR, an issue will be created in the Doc repo.
Projects
None yet
Milestone
3.42.0
Development

Successfully merging this pull request may close these issues.
6 participants
@3nids @agiudiceandrea @pigreco @albertograva @qgis-bot @lbartoletti