grass icon indicating copy to clipboard operation
grass copied to clipboard

wxGUI/vdigit: clean zero-length vector map boundaries before start editing

Open tmszi opened this issue 5 years ago • 6 comments

Fix bug reported in the issue #488.

tmszi avatar Aug 17 '20 09:08 tmszi

I didn't actually test this, so this is not a full review yet, but I would be concerned here that the extensive topology checks which would be called every time you start editing would take a lot of time. Ideally we could figure out (somehow, I don't know how at this point) something is wrong and then try v.build -e.

Yes of course. I have seen how the check of zero-length lines is solved in the C function Vect_topo_check() and I use this functionality in my _topoZeroLengthCheck() method.

It would be good to find out why the zero-length boundaries actually crash the GUI.

wxWidgets StrokeLines() function complain:

Traceback (most recent call last):
  File "/usr/lib64/grass79/gui/wxpython/mapwin/buffered.py",
line 593, in OnPaint

self.pdcVector.DrawToDCClipped(gcdc, rgn)
wx._core
.
wxAssertionError
:
C++ assertion "n > 1" failed at /var/tmp/portage/x11-libs/wx
GTK-3.0.4-r302/work/wxWidgets-3.0.4/src/common/graphcmn.cpp(
742) in StrokeLines():

Zero length line/boundary has only one point (start == end) and therefore it is raise C++ assertion "n > 1 (n -> number of points) error.

wxGUI doesn't always crash. Map rendering during editing is incorrect, due to wxWidgets StrokeLines() function assertion error (screenshot from editing #488). Rendering start working during editing if you zoom in the region which doesn't contains zero length lines/boundaries.

This should in any case go into separate function for readability.

All right.

g.remove is not needed, g.rename with overwrite will do the job.

Thanks. I fixed it.

tmszi avatar Aug 18 '20 12:08 tmszi

@tmszi @petrasovaa can this be merged?

neteler avatar Sep 21 '20 16:09 neteler

@tmszi @petrasovaa can this be merged?

I haven't had time to review it. Did you test it? In any case, I am not sure it's a good idea to backport it, the problem it fixes is rather obscure.

petrasovaa avatar Sep 22 '20 14:09 petrasovaa

@tmszi @petrasovaa can this be merged?

I haven't had time to review it. Did you test it?

I think I did but it is too long time ago. A test dataset is available here: https://github.com/OSGeo/grass/issues/488#issuecomment-619049526

In any case, I am not sure it's a good idea to backport it, the problem it fixes is rather obscure.

ok I see.

neteler avatar Sep 22 '20 20:09 neteler

I finally have been able to test this PR. It works nicely, see also the new screenshots in issue #488.

Thanks a lot, @tmszi. Please merge if there are no further objections.

neteler avatar Nov 24 '21 13:11 neteler

#1989 fixes the crash described in #488 (merged and backported to 8.0). This PR offers an automatic check and fixing before editing, so it may be nice from user point of view, but it adds a lot of code in GUI for a very minor case and I wonder how it would perform with large vector maps.

petrasovaa avatar Nov 29 '21 03:11 petrasovaa