Bug report #13635

different handling of invalid geometries between LTR and master version

Added by Salvatore Larosa almost 4 years ago. Updated almost 3 years ago.

Status:Closed
Priority:Severe/Regression
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:fixed/implemented
Crashes QGIS or corrupts data:No Copied to github as #:21671

Description

it is not possible to select invalid geometries in master version with the node tool.
it works fine in 2.8.3 or at the least there the user can edit the geometry to fix it.
I attached a small testcase.

I am not sure if this may be considered as a blocker although it is a regression.

invalid_geometry.zip (2.59 KB) Salvatore Larosa, 2015-10-20 12:33 AM

qgis-bug13635.qgs - strk's project file used for testing (10.2 KB) Sandro Santilli, 2016-06-22 06:44 AM


Related issues

Related to QGIS Application - Bug report #13276: node tool regressions and issues Closed 2015-08-27

History

#1 Updated by Nyall Dawson almost 4 years ago

  • Status changed from Open to Feedback

Are you referring to how the polygon disappears when you zoom, or can't you edit with the node tool? Because I just tried and while I get odd behaviour with the polygon vanishing when I zoom in (same behaviour in 2.8), I can edit the geometry with the node tool and remove the invalid rings...

#2 Updated by Salvatore Larosa almost 4 years ago

Yes, I cannot edit the geometry with the node tool, works fine in LTR on the same machine.
I get the message "could not snap to a segment on the current layer." when trying to click over the geometry.

#3 Updated by Nyall Dawson almost 4 years ago

You have to click on a node to start the edit though - I don't think clicking within a polygon to start the edit has ever been supported?

#4 Updated by Nyall Dawson almost 4 years ago

Click inside polygon to start is #3752. The snapping message is misleading though - that's a bug.

#5 Updated by Salvatore Larosa almost 4 years ago

I am clicking on nodes and I see that message (note it popups without title: Node Tool).

#6 Updated by Salvatore Larosa almost 4 years ago

I can confirm also on Windows.

@Nyall: so works as LTR for you in current master? can you select the attached feature with the Select Features tool?
In current master I cannot neither select the feature nor changing or deleting node.

#7 Updated by Nyall Dawson almost 4 years ago

  • Status changed from Feedback to Open

Ok, I can confirm that the polygon CAN'T be selected. But it's odd that the node tool works for me.

#8 Updated by Martin Dobias over 3 years ago

One inner ring has just 3 vertices (the first and the last being the same), and so OGR fails to organize that polygon.

Polygon disappears when zoomed in because non-trivial geometry intersection has to be done in OGR (using GEOS), which fails because GEOS will not construct such invalid geometry.

Select map tool again does not work because GEOS is used to determine selection (interects / contains / distance).

Node tools sometimes works for me, but sometimes it does not snap. First I need to zoom out a bit in order to "initialize" node tool, then it works. To my surprise, snapping in node tool has been rewritten to use the old QgsMapCanvasSnapper class instead of QgsSnappingUtils, which may explain the different behavior.

#9 Updated by Sandro Santilli over 3 years ago

  • Status changed from Open to Feedback
  • Assignee set to Sandro Santilli

Salvatore does the problem persist with current master (aka 2.15.0) ?

#10 Updated by Salvatore Larosa over 3 years ago

same problem here with the latest master branch. can you edit the invalid geometry?

#11 Updated by Sandro Santilli over 3 years ago

  • Status changed from Feedback to Open

I hadn't tested yet, but I seem to understand that the regression is the
"could not snap to a segment on the current layer" message, correct ?

#12 Updated by Sandro Santilli over 3 years ago

I've just tried editing an simple invalid polygon (bow-tie) and it worked in both 2.8.9 and 2.14.3.
Here's the WKT (loaded via QuickWKT): POLYGON)

Will try your shapefile next.

#13 Updated by Sandro Santilli over 3 years ago

With the attached shapefile I still DO CAN edit vertices with the node tool while I CAN NOT select the feature with the "select tool". This behavior is exactly the same for me in 2.8.9, 2.14.3 and current master (2.15.0-Master 87a560b).

Oh but I finally found a difference: moving a selected vertex behaves differently.
In 2.8.9 you can see the expected vertex move correctly; in 2.14.3 and 2.15.0-Master 87a560b you don't see the result of the vertex move as you move it, but it works fine on saving the layer. Do you confirm this, Salvatore ?

#14 Updated by Sandro Santilli over 3 years ago

I made another test with a simplified version of the invalid polygon: a polygon with unclosed ring:

010300000001000000030000000000000000000000000000000000000000000000000024400000000000000000000000000000244000000000000024400000000000000000

Pass the above to QuickWKT plugin (for example), then try to edit.
I get the same (bad? or just unexpected ?) experience in 2.8.9, 2.14.3 and 2.16 when moving the lower-left vertex
(the polygon collapses to a line).
I get a different experience when moving the lower-right vertex
(2.8 keeps showing a triangle, 2.14 and 2.15 show an unfilled contour)

#15 Updated by Sandro Santilli over 3 years ago

Actually, 2.14 behaves the same as 2.8 with the simplified polygon, so the difference is only in master (2.15.0dev)

#16 Updated by Sandro Santilli over 3 years ago

In no case I got the "could not snap to a segment on the current layer", while I can never select the polygon by clicking in it (as the ring is not closed so GEOS object cannot be constructed).

I guess we could fix this by ensuring rings are closed, but I'm not sure we want this to happen silently to the user.
Wasn't there a "make valid" plugin ?

#17 Updated by Sandro Santilli over 3 years ago

Code to force-close ring on converting geometries to GEOS is ready:
https://github.com/qgis/QGIS/pull/3212

Fix the difference in node tool behavior AND selectability of the simplified invalid polygon (triangle polygon with 3 vertices only). The Shapefile geometry is still not selectable with that.

#18 Updated by Sandro Santilli over 3 years ago

  • Status changed from Open to Feedback

Here's the path that takes from renderer to exception:

#6 0x00007ffff42c8024 in QgsGeos::asGeos (geom=0x364c590, precision=0) at /usr/src/qgis/qgis-master/src/core/geometry/qgsgeos.cpp:1119
#7 0x00007ffff42bfc27 in QgsGeos::cacheGeos (this=0x7fffd269f590) at /usr/src/qgis/qgis-master/src/core/geometry/qgsgeos.cpp:178
#8 0x00007ffff42bf89f in QgsGeos::QgsGeos (this=0x7fffd269f590, geometry=0x364c590, precision=0)
at /usr/src/qgis/qgis-master/src/core/geometry/qgsgeos.cpp:141
#9 0x00007ffff4293a34 in QgsGeometry::intersects (this=0x3ea9098, geometry=0x7fffc800e840)
at /usr/src/qgis/qgis-master/src/core/geometry/qgsgeometry.cpp:830
#10 0x00007ffff429391c in QgsGeometry::intersects (this=0x3ea9098, r=...) at /usr/src/qgis/qgis-master/src/core/geometry/qgsgeometry.cpp:818
#11 0x00007ffff3f4e546 in QgsVectorLayerFeatureIterator::fetchNextChangedGeomFeature (this=0x7fffc800cd80, f=...)
at /usr/src/qgis/qgis-master/src/core/qgsvectorlayerfeatureiterator.cpp:389
#12 0x00007ffff3f4d4a2 in QgsVectorLayerFeatureIterator::fetchFeature (this=0x7fffc800cd80, f=...)
at /usr/src/qgis/qgis-master/src/core/qgsvectorlayerfeatureiterator.cpp:226
#13 0x00007ffff3b819df in QgsAbstractFeatureIterator::nextFeature (this=0x7fffc800cd80, f=...)
at /usr/src/qgis/qgis-master/src/core/qgsfeatureiterator.cpp:73
#14 0x00007ffff39def3e in QgsFeatureIterator::nextFeature (this=0x7fffd269f990, f=...)
at /usr/src/qgis/qgis-master/src/core/qgsfeatureiterator.h:279
#15 0x00007ffff3f76cfe in QgsVectorLayerRenderer::drawRendererV2 (this=0x1c65830, fit=...)
at /usr/src/qgis/qgis-master/src/core/qgsvectorlayerrenderer.cpp:299

#19 Updated by Sandro Santilli over 3 years ago

Interesting enough, 2.14.3 receives a 4-vertices exterior ring when QgsGeos object is constructed, so it must be "closed" on constructing the QgsCurvePolygonV2 object. Master (2.15.0) instead still sees only 3 vertices at that point.

#20 Updated by Sandro Santilli over 3 years ago

Here's where 2.14 closes the polygon ring:

#0  QgsLineStringV2::close (this=0x7fff2800ce40) at /usr/src/qgis/qgis-2.14/src/core/geometry/qgslinestringv2.cpp:884
#1  0x00007ffff4828379 in QgsPolygonV2::setExteriorRing (this=0x7fff2800cca0, ring=0x7fff2800ce40)
    at /usr/src/qgis/qgis-2.14/src/core/geometry/qgspolygonv2.cpp:224
#2  0x00007ffff47be18b in QgsCurvePolygonV2::toPolygon (this=0x35148b0) at /usr/src/qgis/qgis-2.14/src/core/geometry/qgscurvepolygonv2.cpp:429
#3  0x00007ffff4800236 in QgsGeos::asGeos (geom=0x35148b0, precision=0) at /usr/src/qgis/qgis-2.14/src/core/geometry/qgsgeos.cpp:1075
#4  0x00007ffff47f7fd9 in QgsGeos::cacheGeos (this=0x7fff452494d0) at /usr/src/qgis/qgis-2.14/src/core/geometry/qgsgeos.cpp:175
#5  0x00007ffff47f7c51 in QgsGeos::QgsGeos (this=0x7fff452494d0, geometry=0x35148b0, precision=0)
    at /usr/src/qgis/qgis-2.14/src/core/geometry/qgsgeos.cpp:138

#21 Updated by Sandro Santilli over 3 years ago

  • Status changed from Feedback to In Progress

#22 Updated by Sandro Santilli over 3 years ago

  • % Done changed from 0 to 20

The 2.15 regression in identify/select was fixed with e92e7fe472bc0b6e040461ee4f2152a5369776ee in master.
That commit included a test which was ported to 2.14 branch with d3d2603025fa79e3c24f5b8890210f8fd5f9e5b6.
The backported test showed 2.14 having no problems with identifying polygons created with non-closed rings, and the references in previous comments of this ticket reveal that the approach in 2.14 was the same restored with the commit in master, that is: close rings while converting the geometry to GEOS.

As for the original submission of this bug ("different handling of invalid geometries between LTR and master version"),
assuming it means "between 2.8 and 2.14", I still can see a difference: 2.8 keeps showing a triangle, 2.14 and 2.15 show an unfilled contour.

#23 Updated by Sandro Santilli over 3 years ago

  • Target version set to Version 2.14
  • Affected QGIS version changed from master to 2.14.3

#24 Updated by Salvatore Larosa over 3 years ago

@strk thank you for all work on this issue.

the original bug was between 2.8.4 and 2.13 (master version). Today I have done a test between 2.8.4 and master (c81b14d) and the main issue is confirmed. With 2.8.4 I can move the vertices of the invalid geometry (it means that I can fix the invalidity manually) while I can't with master version (with the msg "could not snap to a segment on the current layer."). Substantially the node tool works in a way different between LTR and master version.

Tonight I try to compile the 2.8.9 LTR version and I will do a try with that.

#25 Updated by Sandro Santilli over 3 years ago

What do you have in Settings->Snapping_Options ?
My settings are:

Snapping mode: Current Layer
Snap to: Off
Tolerance: 0.000000 map units
Enable topological editing: unchecked
Enable snapping on intersection: unchecked

What about Settings->Options->CRS ?
Mine:

Default CRS for new projects: Don't enable 'on the fly reprojection'
CRS for new layers: Use project CRS
Default datum transformations: EMPTY

I'm attaching my qgis project file, can you reproduce with that one ?

#26 Updated by Salvatore Larosa over 3 years ago

Hi Sandro,

I have the same configuration like you (using your project I get the same behavior).

I done more tests on different machines (actually my friends have done):

  • Windows 7, QGIS dev (5bb2c7d), GEOS 3.5.0-CAPI-1.9.0:
    • cannot select, cannot edit with node tool and geometry disappears when zooming
  • Windows 7, QGIS 2.14.3, GEOS 3.5.0-CAPI-1.9.0:
    • cannot select, cannot edit with node tool and geometry disappears when zooming
  • Windows 7, QGIS 2.8.9, GEOS 3.5.0-CAPI-1.9.0:
    • cannot select, can edit with node tool and geometry disappears when zooming
  • Debian 8, QGIS 2.14.3, GEOS 3.4.2-CAPI-1.8.2 r3921:
    • cannot select, can edit with node tool and geometry does not disappear when zooming

#27 Updated by Sandro Santilli about 3 years ago

How about Debian 8 with QGIS-2.8.9 and dev/2.15/5bb2c7d ?
The test you (or your friend) made only show a regression under Windows7 between 2.8.9 and 2.14.3 ...

Do you think you can make an automated test reproducing the regression issue you're seeing ?
As I'm not sure what "can edit" and "cannot edit" mean, exactly (I can move the vertices with the node tool but not see the fille polygon nor the lines connecting those vertices as soon as I move a vertex, for example)

#28 Updated by Sandro Santilli about 3 years ago

  • Status changed from In Progress to Feedback
  • % Done changed from 20 to 50

ba1d38c2edaee24ca5e01dd1d9652de37d32ca9d adds a test for selecting the invalid polygon.
Salvatore: does the test pass on windows 7 ? As it passes on Travis (debian8) and on ubuntu 14.04...

#29 Updated by Sandro Santilli almost 3 years ago

  • Resolution set to fixed/implemented

Assuming fixed, for lack of feedback. Salvatore feel free to reopen if you still see a regression.
Or open a new one if it's not a regression.

#30 Updated by Sandro Santilli almost 3 years ago

  • Status changed from Feedback to Closed

Also available in: Atom PDF