Bug report #13099

[regression] nodetool cannot remove parts and rings or full geometries by removing all vertices

Added by Jürgen Fischer over 8 years ago. Updated over 8 years ago.

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

Related to QGIS Application - Bug report #13258: delete part cannot make feature having NULL geometry anymore Closed 2015-08-24
Related to QGIS Application - Bug report #13276: node tool regressions and issues Closed 2015-08-27
Related to QGIS Application - Bug report #13661: Node tool should not allow to delete nodes and leave just... Closed 2015-10-23

Associated revisions

Revision fe5085e8
Added by Nyall Dawson over 8 years ago

Fix up missing geometry tests (refs #13099)

Revision acd8f26a
Added by Nyall Dawson over 8 years ago

Fix removing whole ring by progressively deleting nodes (fix #13099)

Revision 8640c130
Added by Marco Hugentobler over 8 years ago

Merge pull request #2298 from nyalldawson/fix_13099

Fix #13099 - removal of geometries using node tool

History

#1 Updated by Nyall Dawson over 8 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 8 years ago

  • Priority changed from Normal to Severe/Regression
  • Category set to Digitising

#3 Updated by Marco Hugentobler over 8 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 over 8 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 over 8 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 over 8 years ago

  • Status changed from Open to Closed

Also available in: Atom PDF