Bug report #7929

Exiting edit mode withouth saving triggers a "geometryChanged" event for each removed vertex

Added by Sandro Santilli over 7 years ago. Updated over 7 years ago.

Status:Closed
Priority:Normal
Assignee:Jürgen Fischer
Category:Digitising
Affected QGIS version:master Regression?:No
Operating System: Easy fix?:No
Pull Request or Patch supplied:No Resolution:
Crashes QGIS or corrupts data:No Copied to github as #:16800

Description

When exiting edit mode without saving, the "undo" operation triggers a call to "geometryChanged" for every vertex that was deleted during editing.


Related issues

Related to QGIS Application - Bug report #7900: Mass vertex removal with node tool leaks memory Closed 2013-05-25

Associated revisions

Revision 59788cb8
Added by Jürgen Fischer over 7 years ago

- sync sip binding of QgsVectorLayer
- [API] introduce signal QgsVectorLayer::beforeRollBack()
- merge consecutive geometry changes (fixes #7929)
- avoid validation in nodetool on rollback

History

#1 Updated by Sandro Santilli over 7 years ago

I verified that saving the edit does not call geometryChanged so many times

#2 Updated by Sandro Santilli over 7 years ago

Under gdb I can also see that a thread is started for each vertex !! This is serious performance loss

#3 Updated by Sandro Santilli over 7 years ago

The origin seems to be this one:

#15 0x00007fb20cfdd0a1 in QgsVectorLayerUndoCommandChangeGeometry::undo (this=0x2be0e40)
    at /usr/src/qgis/qgis/src/core/qgsvectorlayerundocommand.cpp:176
#16 0x00007fb20b6bc5b1 in QUndoCommand::undo() () from /usr/lib/x86_64-linux-gnu/libQtGui.so.4
#17 0x00007fb20b6bcebf in QUndoStack::setIndex(int) () from /usr/lib/x86_64-linux-gnu/libQtGui.so.4
#18 0x00007fb20cfcf61a in QgsVectorLayerEditBuffer::rollBack (this=0x29a4080)
    at /usr/src/qgis/qgis/src/core/qgsvectorlayereditbuffer.cpp:450
#19 0x00007fb20cfbed98 in QgsVectorLayer::rollBack (this=0x2ecae40, deleteBuffer=true)
    at /usr/src/qgis/qgis/src/core/qgsvectorlayer.cpp:2612

#4 Updated by Sandro Santilli over 7 years ago

And:

#14 0x00007fb20d35ca9b in QgsVectorLayerEditBuffer::geometryChanged (this=0x29a4080, _t1=4, _t2=...)
    at /usr/src/qgis/Quantum-GIS/b/src/core/moc_qgsvectorlayereditbuffer.cxx:164

#5 Updated by Sandro Santilli over 7 years ago

I believe the problem is the undo stack as QgsVectorLayerEditUtils only exposes a function to delete a single vertex so it makes a geometry clone for each removed vertex and rolls back one vertex at the time.

It would be much much better to expose a "mass vertexes removal" kind of interface, to make it an atomic operation.

Seems to be a work for Martin Dobias :)

#6 Updated by Jürgen Fischer over 7 years ago

Sandro Santilli wrote:

I believe the problem is the undo stack as QgsVectorLayerEditUtils only exposes a function to delete a single vertex so it makes a geometry clone for each removed vertex and rolls back one vertex at the time.

Um, isn't that already in place? The undo stack should contain only one entry for "Delete vertices" for each time you hit "delete". Multiple deletes shouldn't be accumulated as they should still be separately undoable.

#7 Updated by Sandro Santilli over 7 years ago

Evidently not, "delete vertices" is not even supported by the utility class used to keep track of undo stack (AFAIU):
https://github.com/qgis/Quantum-GIS/blob/master/src/core/qgsvectorlayereditutils.h

#8 Updated by Sandro Santilli over 7 years ago

Note that the nodeTool contains an hack to avoid multiple "geometryChanged" events by simply disconnecting the hook, deleting each vertex in turn and re-connecting the event, but the method invoked for deleting the single vertex is the one that seems to be filling up the undo stack, with as many geometry copies as vertexes selected. Which explains the fast memory growth for a big geometry (>45k vertexes).

#9 Updated by Jürgen Fischer over 7 years ago

  • Assignee set to Jürgen Fischer

#10 Updated by Jürgen Fischer over 7 years ago

  • Status changed from Open to Closed

#11 Updated by Sandro Santilli over 7 years ago

Great job, the memory issue is also fixed by this!

#12 Updated by Sandro Santilli over 7 years ago

Only, it may be better to comment out the "geometry changes merged" debug line as getting tens of thousand of lines when mass-removing vertices kills CPU ;)

Also available in: Atom PDF