Bug report #13099
[regression] nodetool cannot remove parts and rings or full geometries by removing all vertices
| Status: | Closed | ||
|---|---|---|---|
| Priority: | Severe/Regression | ||
| Assignee: | - | ||
| Category: | Digitising | ||
| Affected QGIS version: | 2.10.0 | Regression?: | No |
| Operating System: | Easy fix?: | No | |
| Pull Request or Patch supplied: | No | Resolution: | |
| Crashes QGIS or corrupts data: | No | Copied to github as #: | 21165 |
Description
The old geometry implementation allowed to remove vertices even if that would produce invalid geometry (c686c4f). This was meant to allow interim steps to fix already broken geometry in the nodetool (eg. remove invalid rings, that the remove ring tool doesn't find).
The commit even included a test that used to verify this worked. Unfortunately it was also changed (68fe5f5; "line 967":#68fe5f5#diff-fe3aa1328ee04f0eb00a1b1d59c0ea71L967).
Looking at the changed tests also noticed that the behavior of QgsGeometry::insertVertex() and QgsGeometry::deleteVertex() on MultiPoint geometries apparently changed as that test was substantially trimmed as well ("line 757":#68fe5f5#diff-fe3aa1328ee04f0eb00a1b1d59c0ea71L757; introduced in cb5c4e9).
Related issues
Associated revisions
Fix up missing geometry tests (refs #13099)
Fix removing whole ring by progressively deleting nodes (fix #13099)
History
#1
Updated by Nyall Dawson over 9 years ago
PR #2212 (https://github.com/qgis/QGIS/pull/2212) brings back the missing tests, but doesn't fix the underlying issues...
#2
Updated by Giovanni Manghi over 9 years ago
- Priority changed from Normal to Severe/Regression
- Category set to Digitising
#3
Updated by Marco Hugentobler about 9 years ago
It is debatable wether a vertex tool should remove not only vertices, but also features or rings. There are other tools for that (therefore the changed tests). I however see jefs argument to temporarily fix broken geometries, so I'm trying to bring that functionality back.
#4
Updated by Jürgen Fischer about 9 years ago
Marco Hugentobler wrote:
It is debatable wether a vertex tool should remove not only vertices, but also features or rings. There are other tools for that (therefore the changed tests). I however see jefs argument to temporarily fix broken geometries, so I'm trying to bring that functionality back.
BTW the other tools rely on GEOS to find the geometries/rings/parts - which usually doesn't work if geometries are broken. So the geometries must be "fixed" first in the nodetool and then removed using the other tools - hence that was added to the nodetool directly.
#5
Updated by Nyall Dawson about 9 years ago
It is debatable wether a vertex tool should remove not only vertices, but also features or rings. There are other tools for that (therefore the changed tests). I however see jefs argument to temporarily fix broken geometries, so I'm trying to bring that functionality back.
Looking at how the new geometry classes function it seems like they are all hard-coded to prevent creation of invalid geometries (and modifications like these which result in invalid geometries). But I'm not sure about this approach... wouldn't a better way be to allow creation and modifications which result in invalid geometries in QgsGeometry, but block (or fix) their creation in the map tools? The reality is that invalid geometries are prevalent throughout geospatial data, and QGIS still needs to be able to read and work with these geometries.
#6
Updated by Marco Hugentobler about 9 years ago
- Status changed from Open to Closed
Fixed in changeset 8640c1300265a81bf5122d4510e01668e38f700f.