Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
qgib opened this issue May 28, 2013 · 12 comments
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter! Digitizing Related to feature digitizing map tools or functionality

Comments

@qgib
Copy link
Contributor

qgib commented May 28, 2013

Author Name: Sandro Santilli (@strk)
Original Redmine Issue: 7929
Affected QGIS version: master
Redmine category:digitising
Assignee: Jürgen Fischer


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


Related issue(s): #16772 (relates)
Redmine related issue(s): 7900


@qgib
Copy link
Contributor Author

qgib commented May 28, 2013

Author Name: Sandro Santilli (@strk)


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

@qgib
Copy link
Contributor Author

qgib commented May 28, 2013

Author Name: Sandro Santilli (@strk)


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

@qgib
Copy link
Contributor Author

qgib commented May 28, 2013

Author Name: Sandro Santilli (@strk)


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

@qgib
Copy link
Contributor Author

qgib commented May 28, 2013

Author Name: Sandro Santilli (@strk)


And:

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

@qgib
Copy link
Contributor Author

qgib commented May 29, 2013

Author Name: Sandro Santilli (@strk)


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 :)

@qgib
Copy link
Contributor Author

qgib commented May 29, 2013

Author Name: Jürgen Fischer (@jef-n)


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.

@qgib
Copy link
Contributor Author

qgib commented May 29, 2013

Author Name: Sandro Santilli (@strk)


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

@qgib
Copy link
Contributor Author

qgib commented May 29, 2013

Author Name: Sandro Santilli (@strk)


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).

@qgib
Copy link
Contributor Author

qgib commented May 29, 2013

Author Name: Jürgen Fischer (@jef-n)


  • assigned_to_id was configured as Jürgen Fischer

@qgib
Copy link
Contributor Author

qgib commented May 29, 2013

Author Name: Jürgen Fischer (@jef-n)


Fixed in changeset "59788cb8fcbe86a71397c3046981d00f06c40b00".


  • status_id was changed from Open to Closed

@qgib
Copy link
Contributor Author

qgib commented May 29, 2013

Author Name: Sandro Santilli (@strk)


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

@qgib
Copy link
Contributor Author

qgib commented May 29, 2013

Author Name: Sandro Santilli (@strk)


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 ;)

@qgib qgib added Bug Either a bug report, or a bug fix. Let's hope for the latter! Digitizing Related to feature digitizing map tools or functionality labels May 24, 2019
@qgib qgib closed this as completed May 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter! Digitizing Related to feature digitizing map tools or functionality
Projects
None yet
Development

No branches or pull requests

1 participant