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.

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

Fix Topological editing when CRS are different #41306

Merged

Conversation

lbartoletti
Copy link
Member

@lbartoletti lbartoletti commented Feb 1, 2021

Description

The addTopologicalPoints methods attempt to add points without checking the CRS -- which makes sense. So, Topology editing does not work when layers have different CRS.

There are several problems when we use on-the-fly projection, the first is that points are not added when a layer has a different CRS. fix 1

The second one is that when we want to snap to a segment of a layer with a different CRS, it will snap to a vertex (the calculation is done with different coordinates), this is the original request of #29648

Fix 1. Add topological points on layer with a different CRS

before

topo_snap_diff_crs_before mkv

after

topo_snap_diff_crs_after mkv

Fix 2. Fix topological editing when CRS are different

before

topo_snap_segment_diff_crs_before mkv

after

topo_snap_segment_diff_crs_after mkv

TODO:

@github-actions github-actions bot added this to the 3.18.0 milestone Feb 1, 2021
@lbartoletti lbartoletti marked this pull request as ready for review February 4, 2021 19:17
@nyalldawson
Copy link
Collaborator

Is this ready for review?

@lbartoletti
Copy link
Member Author

Is this ready for review?

Yes please. I need to fix other maptools, but I prefer to do this in separated PRs.
The test fail on Travis, but don't know why -- it works on my machine (r)(tm). Do you think this can be related to the proj version (I use proj 7 on my machine)?

@nyalldawson
Copy link
Collaborator

Do you think this can be related to the proj version (I use proj 7 on my machine)?

Quite possibly - if you wait till the Github Actions PR is merged then we can re-test...

@lbartoletti
Copy link
Member Author

Do you think this can be related to the proj version (I use proj 7 on my machine)?

Quite possibly - if you wait till the Github Actions PR is merged then we can re-test...

It depends, I'll need this PR to fix the other tools. Maybe I can disable the test for now and I note to readd tests when the GH Actions PR is merged?

@nyalldawson
Copy link
Collaborator

@lbartoletti it's really not far off - likely a matter of hours

@nyalldawson
Copy link
Collaborator

@lbartoletti if you rebase against master now and force push you'll get the new github workflow

@lbartoletti lbartoletti force-pushed the fix_topological_editing_different_crs branch from 0794bcb to c6f9e23 Compare February 8, 2021 13:45
@lbartoletti
Copy link
Member Author

Seems to be OK @nyalldawson ?

@nyalldawson nyalldawson merged commit 19f925d into qgis:master Feb 11, 2021
@lbartoletti
Copy link
Member Author

Thanks @nyalldawson

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants