Bug report #13952
QGIS node tool causes snapping another feature randomly
Status: | Closed | ||
---|---|---|---|
Priority: | Low | ||
Assignee: | Sandro Santilli | ||
Category: | Digitising | ||
Affected QGIS version: | 2.14.3 | Regression?: | No |
Operating System: | Easy fix?: | No | |
Pull Request or Patch supplied: | No | Resolution: | |
Crashes QGIS or corrupts data: | No | Copied to github as #: | 21967 |
Description
Hi,
very blocking issue here with 2.12.1 (BTW could someone add it to target versions in redmine?).
with snapping options and topological editing enabled, moving some endpoints of polylines snaps the target line, but also will snapp vertex from ANOTHER unrelated feature to it.
Hard to explain, see video here
I have a complex editing project, created with 2.10 at start.
This bug prevents me from doing the edits I need... blocker then. It deserves backports if reproduced and solved.
Associated revisions
Only insert segment snap points in the layer they belong
Fixes #13952
Only insert segment snap points in the layer they belong
Fixes #13952
History
#1 Updated by Regis Haubourg almost 9 years ago
Tried to test on master, and add a crash when changing default projection (was otf 3857 and layer in 2154). So ... I could not test
Tested on 2.8.4, no issue there.
#2 Updated by Jakub Kosik almost 9 years ago
It is same issue I've noticed in earlier versions (look at issue #10957 dig3.png).
More, it happens only if I have more than one legend entry to same dataset.
#3 Updated by Giovanni Manghi almost 9 years ago
- Target version changed from Future Release - High Priority to Version 2.14
- Affected QGIS version changed from 2.12.0 to 2.12.1
#4 Updated by Martin Dobias almost 9 years ago
- Status changed from Open to Feedback
Hi Regis, would it be possible to share the project, cut down to bare minimum of layers (maybe 1-2) when it is still possible to reproduce the bug? Feel free to send directly if the data cannot be shared publicly.
#5 Updated by Daan Goedkoop over 8 years ago
Recently I have come across something similar. It might be the same bug. While digitising, with snapping but without topological editing, the pink snapping "+" would suddenly appear outside of any feature and not at any existing vertex. When clicking it, the created vertex would (after finishing digitising) end up snapped to some seemingly random vertex way outside view. Interestingly, the node tool did not seem to be affected in this case.
Unfortunately I forgot to save that particular version of the shapefile and now I cannot reproduce the problem anymore.
#6 Updated by Daan Goedkoop over 8 years ago
Just now I noticed this issue again. It is not reproducible for me. For what it is worth: the problem remained after saving any edits to the shapefile. However, when I save the project, close QGis and open the project again, the problem disappeared. Does that mean that this problem is a different one, than the one described in this bug report?
#7 Updated by Giovanni Manghi over 8 years ago
This issue seems serious, but I haven't found a way to replicate. Could the reporter attach a sample project with data? Without it is difficult to at least replicate. Thanks.
#8 Updated by Sylvain Beorchia over 8 years ago
Hi. I've got the same issue, and i'm able to reproduce it every time. I will provide a Qgis project with a small postgres DB.
#9 Updated by Paolo Cavallini over 8 years ago
Which snapping settings are you using? Which CRS?
#10 Updated by Sylvain Beorchia over 8 years ago
The problem occurs only when "Activate topologic edition" is enable in snapping options. CRS is 2154.
#11 Updated by Sylvain Beorchia over 8 years ago
- File bug_qgis_13952.zip added
Here is a ZIP containing 2 shapefiles and a Qgis project.
Open the qgis project, and search for the branch id=1212. Edit the layer and try to connect the top vertex of the branch(1212) to the closest arc. You have to enable snapping, and "Enable topological editing".
#12 Updated by Sylvain Beorchia over 8 years ago
- File Sans_titre.png added
See the image joined to see the bug.
#13 Updated by Regis Haubourg over 8 years ago
Sylvain Beorchia wrote:
Here is a ZIP containing 2 shapefiles and a Qgis project.
Open the qgis project, and search for the branch id=1212. Edit the layer and try to connect the top vertex of the branch(1212) to the closest arc. You have to enable snapping, and "Enable topological editing".
Good catch Sylvain! I can reproduce it here at first try.
I never managed to reproduced the bug outside of my complex postgis project and share a sample project.
This is pretty serious indeed.
#14 Updated by Saber Razmjooei over 8 years ago
- Status changed from Feedback to Open
#15 Updated by Saber Razmjooei over 8 years ago
- Status changed from Open to Feedback
- Affected QGIS version changed from 2.12.1 to 2.14.3
If you change the snapping setting to Vertex only, it works fine.
There was another bug related to the precision of snapping tolerance for the node tool, which I think could be related to this issue. Will try to find and link it here.
#16 Updated by Giovanni Manghi over 8 years ago
- Status changed from Feedback to Open
the error as described in #13952-11
is still valid on the latest master.
#17 Updated by Giovanni Manghi over 8 years ago
#18 Updated by Sandro Santilli over 8 years ago
- Assignee set to Sandro Santilli
#19 Updated by Sandro Santilli over 8 years ago
Confirmed as of 2.14.3 -- Snap to Segment Only is enough to reproduce (#note-11)
#20 Updated by Sandro Santilli over 8 years ago
2.8.9 is not affected, 2.12.1 supposedly was (as for original submission)
#21 Updated by Sandro Santilli over 8 years ago
2.10.1 is NOT affected either, so new range is good:2.10.1 bad:2.12.1
#22 Updated by Sandro Santilli over 8 years ago
- Status changed from Open to In Progress
I've tested 2.12.0 to be also bad, so bisect range is 2.10.1..2.12.0 -- bisecting started
#23 Updated by Sandro Santilli over 8 years ago
9c2d70186f054b71f1b792d13133f3856c855bf3 is the first bad commit commit 9c2d70186f054b71f1b792d13133f3856c855bf3 Author: Marco Hugentobler <[email protected]> Date: Wed Sep 16 05:19:26 2015 +0200 Node tool without click-click mode :040000 040000 eb8ad3fa7fc599731bbcb89067d32a44eef25dbd 1f3b3ce634bb47ed00837ca35a7278de83496099 M src
#24 Updated by Sandro Santilli over 8 years ago
Marco, could you take a look at this ? There's been lots of changes but no testcase:
commit 9c2d70186f054b71f1b792d13133f3856c855bf3 Author: Marco Hugentobler <[email protected]> Date: Wed Sep 16 05:19:26 2015 +0200 Node tool without click-click mode src/app/nodetool/qgsmaptoolnodetool.cpp | 500 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------- src/app/nodetool/qgsmaptoolnodetool.h | 49 ++++++---- src/app/nodetool/qgsnodeeditor.cpp | 23 ++--- src/app/nodetool/qgsnodeeditor.h | 1 - src/app/nodetool/qgsselectedfeature.cpp | 53 ++--------- src/app/nodetool/qgsselectedfeature.h | 3 - 6 files changed, 391 insertions(+), 238 deletions(-)
#25 Updated by Sandro Santilli over 8 years ago
- % Done changed from 0 to 40
A QGIS version with the offending commit reverted is available on https://github.com/qgis/QGIS/pull/3248.
I confirm that reverting the commit fixes the random snap.
#26 Updated by Sandro Santilli over 8 years ago
- % Done changed from 40 to 50
Adding debugging prints I came across this computation in QgsLinestringV2::closestSegment that does look wrong:
src/core/geometry/qgslinestringv2.cpp: 775: (closestSegment) [1ms] XXX ST_Distance('POINT(771938 6.95593e+06)'::geometry, 'LINESTRING(771946 6.95593e+06,771904 6.95595e+06)'::geometry); -- 0.0220404
That's coming from this snippet:
testDist = QgsGeometryUtils::sqrDistToLine( pt.x(), pt.y(), prevX, prevY, currentX, currentY, segmentPtX, segmentPtY, epsilon ); if ( testDist < sqrDist ) { QgsDebugMsg( QString("XXX ST_Distance('POINT(%1 %2)'::geometry, 'LINESTRING(%3 %4,%5 %6)'::geometry); -- %7 ") .arg(pt.x()) .arg(pt.y()) .arg(prevX) .arg(prevY) .arg(currentX) .arg(currentY) .arg(testDist) );
The debug suggests that QgsGeometryUtils::sqrDistToLine() is broken, because PostGIS gives, for the ST_Distance call, a value of
3.43946864321886, which squared becomes
11.8299445476857856634618596996.
Qgis instead gets
0.0220404which is way below sqrSnappingTolerance:6.40146, explaining the problem.
Next step would be adding a testcase for QgsGeometryUtils::sqrDistToLine().
#27 Updated by Sandro Santilli over 8 years ago
A focused testcase, pushed with f0e0ba5bb0f844e9ee28eb9572340f31ae13c6ee, does not give the same bogus result. I'm still debugging how could it be possible to obtain the bogus result (some state implied?)
#28 Updated by Sandro Santilli over 8 years ago
Printing more digits gives a matching result between PostGIS and QGIS:
ST_Distance('POINT'::geometry, 'LINESTRING'::geometry); -- 0.143113367679915
( it's actually 3.378 which squared gives 0.1431 )
But I found that the LINESTRING part is the FID of the geometry being moved, not FID=11, so this makes me think that the distanc used for FID=11 (the one which should not move) is just a leftover value for the variable computed for FID=27) -- or something along those lines.
#29 Updated by Sandro Santilli over 8 years ago
New finding: the distance to the fid is correct but the layer is different. We have 2 layers, both contain a fid=11.
The one bogusly snapped is from layer "arc", and is too distance. The fid=11 from the layer being moved (branch) is instead close (as we actually move the vertex right on top of it). The code gets confused as for which vector layer to snap geometries from.
Setting snapping options to "currentLayer" rather than "All Visible Layer" fixes the issue.
Adding another layer with the same geometry as fid=11 from "branch" adds another arbitrary FID to the list of those snapped in any visible layer.
Basically, if I'm seeing this right, all geometries with a given FID value are snapped from any visible layer, rather than FIDs being isolated for each layer.
#30 Updated by Sandro Santilli over 8 years ago
- % Done changed from 50 to 80
Pull request ready with a fix: https://github.com/qgis/QGIS/pull/3251
Can be fetched from https://github.com/strk/QGIS/tree/snap-to-proper-layer
With the fix, behavior is back to the 2.8.9 one, which seems sane enough (maybe not 100% expected, but safe).
The only thing missing would now be an automated testcase to save the many hours spent on this ...
#31 Updated by Sandro Santilli over 8 years ago
- Status changed from In Progress to Closed
Fixed in changeset 647ab4e947edec24b8babe51e188d95270275ea4.
#32 Updated by Jürgen Fischer over 7 years ago
- Description updated (diff)
- Priority changed from Severe/Regression to Low