#699 closed defect (fixed)
v.buffer2 segfault: in Vect_get_isle_points()
| Reported by: | neteler | Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | 7.0.0 |
| Component: | Vector | Version: | 6.4.0 RCs |
| Keywords: | v.buffer | Cc: | rmatev |
| CPU: | All | Platform: | All |
Description
On Fri, Jul 24, 2009 at 1:17 PM, Corrado wrote:
> Yes it is reproducible with spearfish: > > v.buffer input=landcover output=lcb distance=2 --overwrite > > Buffering lines... > 100% > Buffering areas... > Segmentation fault
Same here with GRASS 6.5:
GRASS 6.5.svn (spearfish60):~ > gdb v.buffer ... (gdb) r input=landcover output=lcb distance=2 --o Starting program: /home/neteler/grass65/dist.x86_64-unknown-linux-gnu/bin/v.buffer input=landcover output=lcb distance=2 --o [Thread debugging using libthread_db enabled] WARNING: Vector map <lcb> already exists and will be overwritten Buffering lines... 100% Buffering areas... [New Thread 0x7f8b055c4720 (LWP 3392)] Program received signal SIGSEGV, Segmentation fault. 0x00007f8b051b64c8 in Vect_get_isle_points (Map=0x7fff0d60f6d0, isle=0, BPoints=0x2362de0) at area.c:118 118 G_debug(3, " n_lines = %d", Isle->n_lines);
If needed, I can provide a full backtrace.
Markus
Attachments (1)
Change History (31)
comment:1 by , 15 years ago
comment:2 by , 15 years ago
Not fixed. I got the segfault fixed but ran into the next bug in buffer_lines. No quick fix here.
comment:3 by , 15 years ago
v.buffer is broken for areas. After I fixed the segfault (not submitted), it hangs with the above example. Further on, with some simple test vectors, it throws away isles of areas, removes boundaries between adjacent areas with different categories, and buffers both boundaries and areas causing damaged topology.
IMO,
- boundaries shared between two areas should not be touched when buffering areas, because it can not be decided where the buffered boundary should be
- isles should only be modified if they consist of areas without centroid, otherwise the should be preserved as is
- v.buffer should buffer either boundaries or areas, not both at the same time
There is redundant code in Vlib/buffer2.c, functions that are present elsewhere in the vector libs. New Vect_*() functions should make use of existing functions instead of writing duplicates of existing functions. Someone has to maintain all the code...
This is bad news, I have no time right now to attempt to fix it, best would be if Rosen Matev fixes it, he knows the code best.
Markus M
comment:4 by , 15 years ago
| Cc: | added |
|---|
follow-up: 7 comment:5 by , 15 years ago
The segfault is fixed in 6.4 r38554, but the other problems remain.
Something else I noticed is that categories get lost for areas, all areas get cat = 1.
I would very much like to disable area buffering in v.buffer until it's fixed, is this ok?
Markus M
comment:6 by , 14 years ago
| Keywords: | v.buffer added |
|---|
mmetz:
Something else I noticed is that categories get lost for areas, all areas get cat = 1.
that is by design. if the buffers of two areas will different cat overlap, which cat gets to claim the new combined area? The question can not be answered so we don't ask it- cats are wiped clean.
Hamish
follow-up: 8 comment:7 by , 13 years ago
Replying to mmetz:
The segfault is fixed in 6.4 r38554, but the other problems remain.
I still get a segfault in all three version (6.4, 6.5 and trunk). In 6.4:
v.generalize railroads out=rail_simple method=douglas_reduction reduction=20 v.buffer rail_simple dist=1000 out=rail1000
results in:
Starting program: /home/mlennert/SRC/GRASS/grass-6.4.0/dist.i686-pc-linux-gnu/bin/v.buffer in=rail_simple dist=1000 out=rail1000 [Thread debugging using libthread_db enabled] warning: Lowest section in /usr/lib/libicudata.so.38 is .hash at 000000b4 [New Thread 0xb55c16d0 (LWP 31004)] Buffering lines... 5% Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 0xb55c16d0 (LWP 31004)] 0xb7ed0d00 in pg_create (Points=0x86b6558) at dgraph.c:471 471 v = si->ip[si->il[i].a[0].ip].group;
Moritz
comment:8 by , 13 years ago
| Milestone: | 6.4.0 → 7.0.0 |
|---|
Replying to mlennert:
Replying to mmetz:
The segfault is fixed in 6.4 r38554, but the other problems remain.
I still get a segfault in all three version (6.4, 6.5 and trunk). In 6.4:
> v.generalize railroads out=rail_simple method=douglas_reduction reduction=20 > v.buffer rail_simple dist=1000 out=rail1000
This is unfortunately a different bug in v.buffer, happening in pg_create in dgraph.c. The original bug reported in this ticket happened only when buffering areas, whereas in this case lines are buffered and a different bug has been detected.
I am changing Milestone because these bugs probably don't get fixed in 6.4.x.
Markus M
follow-up: 10 comment:9 by , 13 years ago
follow-up: 13 comment:10 by , 13 years ago
Replying to mmetz:
The second bug should now be fixed in r45219, r45220, r45221 (7.0, 6.5, 6.4), working on Linux 32bit and 64bit.
Correction, it works on Linux 32bit and Windows, but not always on Linux 64bit. On Linux 64 bit, v.buffer2 (and v.parallel2) create sometimes zero-size areas, the bug is somewhere in the vector libs, more specifically in buffer2.c and/or dgraph.c.
BTW, buffering works with the original v.buffer module (slightly modified) where v.buffer2 fails. Umh, and v.buffer is up to 10x faster and easier to maintain than v.buffer2...
Markus M
follow-up: 12 comment:11 by , 13 years ago
Could this be some int overflow? This is output from 6.5 on AMD64.
D3/3: Vect_get_area_isle(): area = 2 isle = 19 D3/3: -> isle = 134 D3/3: Vect_get_isle_points(): isle = 134 D3/3: n_lines = 1 D3/3: append line(0) = -976 D3/3: Vect_read_line() D3/3: V2_read_line_nat(): line = 976 D3/3: Vect__Read_line_nat: offset = 160553 D3/3: type = 4, do_cats = 0 dead = 0 D3/3: n_points = 11 D3/3: off = 160734 D3/3: line n_points = 11 D3/3: isle n_points = 11 D3/3: Vect_get_area_isle(): area = 2 isle = 20 D3/3: -> isle = 137 D3/3: Vect_get_isle_points(): isle = 137 D3/3: n_lines = 1 D3/3: append line(0) = -1002 D3/3: Vect_read_line() D3/3: V2_read_line_nat(): line = 1002 D3/3: Vect__Read_line_nat: offset = 165931 D3/3: type = 4, do_cats = 0 dead = 0 D3/3: n_points = 11 D3/3: off = 166112 D3/3: line n_points = 11 D3/3: isle n_points = 11 D3/3: Vect_get_area_isle(): area = 2 isle = 21 D3/3: -> isle = 175 D3/3: Vect_get_isle_points(): isle = 175 D3/3: n_lines = 1 D3/3: append line(0) = -1233 D3/3: Vect_read_line() D3/3: V2_read_line_nat(): line = 1233 D3/3: Vect__Read_line_nat: offset = 206334 D3/3: type = 4, do_cats = 0 dead = 0 D3/3: n_points = 11 D3/3: off = 206515 D3/3: line n_points = 11 D3/3: isle n_points = 11 D3/3: Vect_get_area_isle(): area = 2 isle = 22 ==12849== Invalid read of size 4 ==12849== at 0x4E43509: Vect_get_area_isle (area.c:288) ==12849== by 0x4E3D64D: Vect_area_buffer2 (buffer2.c:1096) ==12849== by 0x402D6E: main (main.c:478) ==12849== Address 0xdcfc978 is 0 bytes after a block of size 88 alloc'd ==12849== at 0x4C262CA: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==12849== by 0x52A2027: G__realloc (alloc.c:109) ==12849== by 0x5D2BAFE: dig_area_alloc_isle (struct_alloc.c:333) ==12849== by 0x5D27E56: dig_Rd_P_area (plus_struct.c:368) ==12849== by 0x5D25045: dig_load_plus (plus.c:312) ==12849== by 0x4E497C5: Vect_open_topo (open.c:751) ==12849== by 0x4E4A0F2: Vect__open_old (open.c:229) ==12849== by 0x40269A: main (main.c:302) ==12849== D3/3: -> isle = 0 ==12849== Invalid read of size 4 ==12849== at 0x4E43521: Vect_get_area_isle (area.c:288) ==12849== by 0x4E3D64D: Vect_area_buffer2 (buffer2.c:1096) ==12849== by 0x402D6E: main (main.c:478) ==12849== Address 0xdcfc978 is 0 bytes after a block of size 88 alloc'd ==12849== at 0x4C262CA: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==12849== by 0x52A2027: G__realloc (alloc.c:109) ==12849== by 0x5D2BAFE: dig_area_alloc_isle (struct_alloc.c:333) ==12849== by 0x5D27E56: dig_Rd_P_area (plus_struct.c:368) ==12849== by 0x5D25045: dig_load_plus (plus.c:312) ==12849== by 0x4E497C5: Vect_open_topo (open.c:751) ==12849== by 0x4E4A0F2: Vect__open_old (open.c:229) ==12849== by 0x40269A: main (main.c:302) ==12849== D3/3: Vect_get_isle_points(): isle = 0 ==12849== Use of uninitialised value of size 8 ==12849== at 0x4E43145: Vect_get_isle_points (area.c:118) ==12849== by 0x4E3D65A: Vect_area_buffer2 (buffer2.c:1097) ==12849== by 0x402D6E: main (main.c:478) ==12849== ==12849== Invalid read of size 4 ==12849== at 0x4E43145: Vect_get_isle_points (area.c:118) ==12849== by 0x4E3D65A: Vect_area_buffer2 (buffer2.c:1097) ==12849== by 0x402D6E: main (main.c:478) ==12849== Address 0x30 is not stack'd, malloc'd or (recently) free'd ==12849== ==12849== ==12849== Process terminating with default action of signal 11 (SIGSEGV) ==12849== Access not within mapped region at address 0x30 ==12849== at 0x4E43145: Vect_get_isle_points (area.c:118) ==12849== by 0x4E3D65A: Vect_area_buffer2 (buffer2.c:1097) ==12849== by 0x402D6E: main (main.c:478) ==12849== If you believe this happened as a result of a stack ==12849== overflow in your program's main thread (unlikely but ==12849== possible), you can try to increase the size of the ==12849== main thread stack using the --main-stacksize= flag. ==12849== The main thread stack size used in this run was 8388608. ==12849== ==12849== HEAP SUMMARY: ==12849== in use at exit: 7,945,280 bytes in 139,148 blocks ==12849== total heap usage: 139,617 allocs, 469 frees, 8,023,374 bytes allocated ==12849== ==12849== LEAK SUMMARY: ==12849== definitely lost: 4,373 bytes in 13 blocks ==12849== indirectly lost: 240 bytes in 10 blocks ==12849== possibly lost: 0 bytes in 0 blocks ==12849== still reachable: 7,940,667 bytes in 139,125 blocks ==12849== suppressed: 0 bytes in 0 blocks ==12849== Rerun with --leak-check=full to see details of leaked memory ==12849== ==12849== For counts of detected and suppressed errors, rerun with: -v ==12849== Use --track-origins=yes to see where uninitialised values come from ==12849== ERROR SUMMARY: 14 errors from 8 contexts (suppressed: 6 from 6) Segmentation fault
comment:12 by , 13 years ago
Replying to marisn:
Could this be some int overflow? This is output from 6.5 on AMD64.
> [...] > D3/3: Vect_get_area_isle(): area = 2 isle = 22 > [...] > D3/3: -> isle = 0 > [...] > Segmentation fault
This should be fixed as of r45178 from 1/25/2011. Just to be sure, can you confirm that the vector libs in your local copy of 6.5 are at least r45178, better 45217? Thanks!
Markus M
follow-up: 14 comment:13 by , 13 years ago
Replying to mmetz:
Replying to mmetz:
The second bug should now be fixed in r45219, r45220, r45221 (7.0, 6.5, 6.4), working on Linux 32bit and 64bit.
Correction, it works on Linux 32bit and Windows, but not always on Linux 64bit. On Linux 64 bit, v.buffer2 (and v.parallel2) create sometimes zero-size areas, the bug is somewhere in the vector libs, more specifically in buffer2.c and/or dgraph.c.
Test commands for the North Carolina dataset
v.extract input=roadsmajor@PERMANENT output=roads_multilane where="MULTILANE = 'yes'" v.buffer input=roads_multilane output=roads_multilane_buf distance=2000
results in
WARNING: Next edge was visited but it is not the first one !!! breaking
loop
WARNING: Next edge was visited but it is not the first one !!! breaking
loop
WARNING: Next edge was visited but it is not the first one !!! breaking
loop
WARNING: Vect_get_point_in_poly(): Unable to find point in polygon
ERROR: Vect_get_point_in_poly() failed
if the patch from #1231 has been applied, otherwise it cycles forever as described for v.parallel2 in #1231, which happens because Vect_get_point_in_poly() can not find a point in a zero-size polygon. This happens only on Linux 64bit, not on Linux 32bit or Windows. The same command with distance=1000 completes on 64bit, but there are also warnings like
WARNING: Next edge was visited but it is not the first one !!! breaking
loop
If this bug can not be resolved I would suggest to reactivate the original versions of v.buffer and v.parallel (the bugs described there are easy to fix within the module without touching the libs) and deactivate v.buffer2 and v.parallel2.
Markus M
follow-up: 15 comment:14 by , 13 years ago
Replying to mmetz:
If this bug can not be resolved I would suggest to reactivate the original versions of v.buffer and v.parallel (the bugs described there are easy to fix within the module without touching the libs) and deactivate v.buffer2 and v.parallel2.
I can't remember what the issues were triggering the rewrite of v.buffer, but if you think you can solve these issues and that that the original v.buffer is much faster, you definitely have my +1.
Moritz
follow-up: 16 comment:15 by , 13 years ago
Replying to mlennert:
I can't remember what the issues were triggering the rewrite of v.buffer, but if you think you can solve these issues and that that the original v.buffer is much faster, you definitely have my +1.
I think the motivation was the dirty output of v.parallel and only halfway implemented support for a column with buffer distances unique for each feature.
Unfortunately it was not as easy as I assumed, I had to modify the original vector lib functions, i.e. the disabled lib/vector/Vlib/buffer.c file in order to get clean results.
For grass 6.5, r45437 fixes (for me) the issues mentioned in this ticket and in the tickets #90, #994, #1231, #1244.
For testing purposes, I have used the vectors mentioned in these tickets with both v.buffer and v.parallel, that is vector/v.buffer and vector/v.parallel and not vector/v.buffer2 and vector/v.parallel2. All vectors were used as input with the settings given in the respective tickets and various variations of the distance and tolerance options in order to fix the original versions.
Still outstanding is the fix for the bufcol option, but that is not that difficult (seriously). I can look at it once the current change has been approved.
In order to test, you would need to get grass 6.5 r45437 and manually compile the two modules vector/v.buffer and vector/v.parallel. For comparison, the Makefiles of v.buffer2 and v.parallel2 could be modified to name the modules v.buffer2 and v.parallel2 and the modules recompiled.
Markus M
follow-up: 17 comment:16 by , 13 years ago
Replying to mmetz:
Replying to mlennert:
I can't remember what the issues were triggering the rewrite of v.buffer, but if you think you can solve these issues and that that the original v.buffer is much faster, you definitely have my +1.
I think the motivation was the dirty output of v.parallel and only halfway implemented support for a column with buffer distances unique for each feature.
Unfortunately it was not as easy as I assumed, I had to modify the original vector lib functions, i.e. the disabled lib/vector/Vlib/buffer.c file in order to get clean results.
For grass 6.5, r45437 fixes (for me) the issues mentioned in this ticket and in the tickets #90, #994, #1231, #1244.
For testing purposes, I have used the vectors mentioned in these tickets with both v.buffer and v.parallel, that is vector/v.buffer and vector/v.parallel and not vector/v.buffer2 and vector/v.parallel2. All vectors were used as input with the settings given in the respective tickets and various variations of the distance and tolerance options in order to fix the original versions.
Still outstanding is the fix for the bufcol option, but that is not that difficult (seriously). I can look at it once the current change has been approved.
In order to test, you would need to get grass 6.5 r45437 and manually compile the two modules vector/v.buffer and vector/v.parallel. For comparison, the Makefiles of v.buffer2 and v.parallel2 could be modified to name the modules v.buffer2 and v.parallel2 and the modules recompiled.
For me it works very well, much faster than v.buffer2. I was able to calculate 5km buffers of the entire, non simplified nc_spm railroads map in +/- 25 minutes. I stopped v.buffer2 after 40 minutes when it was at only 27% of breaking boundaries.
Interestingly, simplifying railroads (with the v.generalize command mentioned in comment 7) does not seem to significantly increase the speed (running now, but I have to go...).
Moritz
comment:17 by , 13 years ago
Replying to mlennert:
Replying to mmetz:
[...]
For grass 6.5, r45437 fixes (for me) the issues mentioned in this ticket and in the tickets #90, #994, #1231, #1244.
For testing purposes, I have used the vectors mentioned in these tickets with both v.buffer and v.parallel, that is vector/v.buffer and vector/v.parallel and not vector/v.buffer2 and vector/v.parallel2. All vectors were used as input with the settings given in the respective tickets and various variations of the distance and tolerance options in order to fix the original versions.
Still outstanding is the fix for the bufcol option, but that is not that difficult (seriously). I can look at it once the current change has been approved.
In order to test, you would need to get grass 6.5 r45437 and manually compile the two modules vector/v.buffer and vector/v.parallel. For comparison, the Makefiles of v.buffer2 and v.parallel2 could be modified to name the modules v.buffer2 and v.parallel2 and the modules recompiled.
For me it works very well, much faster than v.buffer2. I was able to calculate 5km buffers of the entire, non simplified nc_spm railroads map in +/- 25 minutes. I stopped v.buffer2 after 40 minutes when it was at only 27% of breaking boundaries.
Interestingly, simplifying railroads (with the v.generalize command mentioned in comment 7) does not seem to significantly increase the speed (running now, but I have to go...).
It should still be the same number of railroads, the same number of created buffers, and a similar amount of cleaning that needs to be done on the created buffers.
I managed to fix the segfaults v.buffer2 caused in Vect_get_isle_points() and pg_create(), but the test cases not properly working with v.buffer2/v.parallel2 are
- this ticket comment 13
- ticket #90
- ticket #1231
- ticket #1244
- ticket #994 does not apply to v.buffer, maybe I fixed it for v.buffer2
All these test cases are successfully completed by v.buffer/v.parallel; on confirmation by I would reactivate v.buffer/v.parallel and deactivate v.buffer2/v.parallel. For trunk that would mean that v.buffer which is actually v.buffer2 gets replaced with v.buffer, same for v.parallel.
Markus M
follow-up: 19 comment:18 by , 13 years ago
I'm super glad of the progress here. Before we go much further I think it would be a good idea to go back to the mailing list archives (and private emails) and re-read the threads discussing the specific issues which lead to the creation of v.buffer2/v.parallel2 in the first place, and revisit them to see if those issues are now addressed. Radim's vector TODO list talked about it as well.
I just want to be sure we're betting on the right horse and aren't going to run back into the same previously discovered fatal flaws that we've forgotten about.
I've been meaning to do this dig myself for some weeks as I remember bits of it, but time has been against me lately.
mmetz:
Umh, and v.buffer is up to 10x faster and easier to maintain than v.buffer2...
short of anything truly fatal in the original reasons for writing v.buffer2, that statement is rather hard to argue with.
regards, Hamish
follow-up: 20 comment:19 by , 13 years ago
Replying to hamish:
I'm super glad of the progress here. Before we go much further I think it would be a good idea to go back to the mailing list archives (and private emails) and re-read the threads discussing the specific issues which lead to the creation of v.buffer2/v.parallel2 in the first place, and revisit them to see if those issues are now addressed. Radim's vector TODO list talked about it as well.
I just want to be sure we're betting on the right horse and aren't going to run back into the same previously discovered fatal flaws that we've forgotten about.
The most serious bug I am aware of was that cleaning would not work with the buffer column option, i.e. if different lines/areas are buffered with different distances. This has been fixed in v.buffer2, but the implementation in vbuffer2 is both using lots of memory and is very slow. The new v.buffer (r45833) uses much less memory and is much faster. The entire nc_spm railroad map is now buffered in less than 2 minutes (see also comment 16). There are still some special cases where the results of v.buffer/v.parallel can be improved, but their results are IMHO far better than v.buffer2/v.parallel2.
I've been meaning to do this dig myself for some weeks as I remember bits of it, but time has been against me lately.
mmetz:
Umh, and v.buffer is up to 10x faster and easier to maintain than v.buffer2...
short of anything truly fatal in the original reasons for writing v.buffer2, that statement is rather hard to argue with.
Unfortunately, everything became a bit more complicated in order to support a buffer column with individual buffering distances and in order to handle some special cases.
Markus M
follow-up: 21 comment:20 by , 13 years ago
Replying to mmetz:
The most serious bug I am aware of was that cleaning would not work with the buffer column option, i.e. if different lines/areas are buffered with different distances. This has been fixed in v.buffer2, but the implementation in vbuffer2 is both using lots of memory and is very slow.
[snip]
Unfortunately, everything became a bit more complicated in order to support a buffer column with individual buffering distances and in order to handle some special cases.
Just to make sure I understand: so your corrected version of v.buffer still contains a bug for buffer columns ? Any idea how this bug could be solved ? Or are you pleading for having a fast v.buffer without buffer column and maybe an option v.buffer.column based on v.buffer2 which provides a buffer column option, but runs slower ?
Moritz
follow-up: 22 comment:21 by , 13 years ago
Replying to mlennert:
Replying to mmetz:
The most serious bug I am aware of was that cleaning would not work with the buffer column option, i.e. if different lines/areas are buffered with different distances. This has been fixed in v.buffer2, but the implementation in vbuffer2 is both using lots of memory and is very slow.
[snip]
Unfortunately, everything became a bit more complicated in order to support a buffer column with individual buffering distances and in order to handle some special cases.
Just to make sure I understand: so your corrected version of v.buffer still contains a bug for buffer columns ? Any idea how this bug could be solved ? Or are you pleading for having a fast v.buffer without buffer column and maybe an option v.buffer.column based on v.buffer2 which provides a buffer column option, but runs slower ?
Sorry for being unclear: the buffer column option is working just fine in v.buffer. I transferred the method of v.buffer2 to v.buffer and optimized the method.
In my tests, the results of v.buffer/v.parallel are more accurate than v.buffer2/v.parallel2 (granted that v.buffer2/v.parallel2 do not bomb out); therefore I would plead to replace v.buffer2/v.parallel2 with v.buffer/v.parallel.
Markus M
comment:22 by , 13 years ago
Replying to mmetz:
In my tests, the results of v.buffer/v.parallel are more accurate than v.buffer2/v.parallel2 (granted that v.buffer2/v.parallel2 do not bomb out); therefore I would plead to replace v.buffer2/v.parallel2 with v.buffer/v.parallel.
+1
Moritz
follow-ups: 24 25 comment:23 by , 13 years ago
Hi,
I am really thankful for the progress, and have no objection, but before these plans are enacted and issues closed & forgotten about, I would think it to be a good idea to revisit the original ML threads re. reasons that v.buffer2 was written in the first place.
http://intevation.de/rt/webrt?serial_num=2765
I think there's another report focusing on v.parallel- I would check if the "inside corner" problem which was present in both (1) and (2) versions is indeed now fixed.
I'm having trouble accessing the 2008 Summer of Code application, due I think to the recent re-vamp of the SoC website. But fwiw,
2008 Summer of Code Reimplement And Add Features to Buffer Generation Module in GRASS by Rosen Ivanov Matev, mentored by Wolf Bergenheim http://thread.gmane.org/gmane.comp.gis.grass.devel/27534/ http://code.google.com/soc/2008/osgeo/about.html
but back to the old RT bug tracker bug #2765 re v.buffer1.. I think it is important to read through all that report before (re)activating; here's one snippet:
[Apr 29 2007] example of vector shape that doesn't get buffered correctly: GRASS> v.in.ascii out=ditch_1205 form=standard -n << EOF B 4 600727.68682251 5677973.32091602 600739.16582305 5678137.49568095 600736.32863997 5678140.4618269 600730.63832718 5678056.67823368 B 2 600730.63832718 5678056.67823368 600725.02385533 5677974.01131491 C 1 1 600730.04890192 5678035.56666655 1 1205 B 2 600727.68682251 5677973.32091602 600725.02385533 5677974.01131491 EOF # and then: GRASS> v.buffer ditch_1205 out=ditch_1205_b1 type=area buffer=1 GRASS> v.buffer ditch_1205 out=ditch_1205_b4 type=area buffer=4 [...] http://bambi.otago.ac.nz/hamish/grass/bugs/bug2765.png http://bambi.otago.ac.nz/hamish/grass/bugs/bug2765_zoom.png
thanks, Hamish
by , 13 years ago
| Attachment: | buff_test.png added |
|---|
test of current v.buffer in trunk based on old RT bug report
comment:24 by , 13 years ago
Replying to hamish:
but back to the old RT bug tracker bug #2765 re v.buffer1.. I think it is important to read through all that report before (re)activating; here's one snippet:
[Apr 29 2007] example of vector shape that doesn't get buffered correctly: GRASS> v.in.ascii out=ditch_1205 form=standard -n << EOF B 4 600727.68682251 5677973.32091602 600739.16582305 5678137.49568095 600736.32863997 5678140.4618269 600730.63832718 5678056.67823368 B 2 600730.63832718 5678056.67823368 600725.02385533 5677974.01131491 C 1 1 600730.04890192 5678035.56666655 1 1205 B 2 600727.68682251 5677973.32091602 600725.02385533 5677974.01131491 EOF # and then: GRASS> v.buffer ditch_1205 out=ditch_1205_b1 type=area buffer=1 GRASS> v.buffer ditch_1205 out=ditch_1205_b4 type=area buffer=4 [...] http://bambi.otago.ac.nz/hamish/grass/bugs/bug2765.png http://bambi.otago.ac.nz/hamish/grass/bugs/bug2765_zoom.png
This specific issue seems to be solved. See attached PNG file.
Moritz
comment:25 by , 13 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
| Summary: | v.buffer segfault: in Vect_get_isle_points() → v.buffer2 segfault: in Vect_get_isle_points() |
Replying to hamish:
Hi,
I am really thankful for the progress, and have no objection, but before these plans are enacted and issues closed & forgotten about, I would think it to be a good idea to revisit the original ML threads re. reasons that v.buffer2 was written in the first place.
I tested the ascii vectors in this ticket, including the ditches_1205, all work fine, all troubles mentioned in this long thread seem to be solved.
Unfortunately, the test data polygonmap and frame are no longer available, but then it seems that the ascii vectors available in the ticket where mainly used for testing and debugging.
I think there's another report focusing on v.parallel- I would check if the "inside corner" problem which was present in both (1) and (2) versions is indeed now fixed.
Some tickets related to v.buffer/v.parallel/v.buffer2/v.parallel2 (it's not always clear which version is meant) are listed in comment 17 of this ticket, and solved for v.buffer/v.parallel, see comment 17. That includes the "inside corner" bug.
I'm having trouble accessing the 2008 Summer of Code application, due I think to the recent re-vamp of the SoC website. But fwiw,
> 2008 Summer of Code > Reimplement And Add Features to Buffer Generation Module in GRASS > by Rosen Ivanov Matev, mentored by Wolf Bergenheim > http://thread.gmane.org/gmane.comp.gis.grass.devel/27534/ > http://code.google.com/soc/2008/osgeo/about.html
The ideas and concepts mentioned in these threads are surely convincing, but it really is a pity that Rosen Matev does not responds to these tickets. I fixed what I could for v.buffer2/v.parallel2, and would have preferred to get these working, but I reached a dead end. That is why I decided to go for the original version of Radim. If Rosen Matev would fix the library functions, I could fix the modules.
but back to the old RT bug tracker bug #2765 re v.buffer1.. I think it is important to read through all that report before (re)activating; here's one snippet:
> [Apr 29 2007] > example of vector shape that doesn't get buffered correctly: > > GRASS> v.in.ascii out=ditch_1205 form=standard -n << EOF > B 4 > 600727.68682251 5677973.32091602 > 600739.16582305 5678137.49568095 > 600736.32863997 5678140.4618269 > 600730.63832718 5678056.67823368 > B 2 > 600730.63832718 5678056.67823368 > 600725.02385533 5677974.01131491 > C 1 1 > 600730.04890192 5678035.56666655 > 1 1205 > B 2 > 600727.68682251 5677973.32091602 > 600725.02385533 5677974.01131491 > EOF > > # and then: > GRASS> v.buffer ditch_1205 out=ditch_1205_b1 type=area buffer=1 > GRASS> v.buffer ditch_1205 out=ditch_1205_b4 type=area buffer=4 > > [...] > > http://bambi.otago.ac.nz/hamish/grass/bugs/bug2765.png > http://bambi.otago.ac.nz/hamish/grass/bugs/bug2765_zoom.png
Have you tested? Buffering ditch_1205 works fine for me.
I still opt for reactivating old v.buffer/v.parallel (already done for 6.5 in r45837), but keeping the code for v.buffer2/v.parallel2 in the hope that this gets fixed at some stage, or used for some new v.buffer3/v.parallel3.
I am closing this ticket because the two segfaults reported here related exclusively to v.buffer2 and not v.buffer, and I fixed these for v.buffer2. I would recommend a new ticket for further discussion of v.buffer/v.parallel vs. v.buffer2/v.parallel2.
Markus M
follow-ups: 27 28 comment:26 by , 13 years ago
However, testing with the other example (see Mon, Oct 9 2006 08:02:33) from the old RT report gives the following, seems to reveal some remaining issues.
First of all, I cannot reproduce the procedure exactly, as the v.buffer call with bufcolumn=lane_cout gives me
GRASS 7.0.svn (nc_spm_06):~ > v.buffer vbuf_fill_bug bufcol=lane_count out=vbuf_fill_bug_lanes scale=150
ATTENTION : The bufcol option may contain bugs during the cleaning step. If
you encounter problems, use the debug option or clean manually
with v.clean tool=break; v.category step=0; v.extract -d
type=area
ERREUR :The bufcol option requires a valid layer.
Looking at the table connection info:
GRASS 7.0.svn (nc_spm_06):~ > v.db.connect -p $OUTMAP Vector map <vbuf_fill_bug> is connected by: layer <1/vbuf_fill_bug> table <vbuf_fill_bug> in database </home/mlennert/GRASS/DATA/nc_spm_06/mlennert/dbf/> through driver <dbf> with key <cat>
the layer <1/vbuf_fill_bug> looks weird. This should be layer <1>, no ? (At least it is in dev6).
So, for now, I just try it without the bufcol parameter:
v.buffer input=$OUTMAP output=${OUTMAP}_buf_dyn dist=450 --o
which gives me a segfault during the "Deleting boundaries" stage. Here's the end of the debug output:
D3/3: dig__angle_next_line: line = -41, side = 1, type = 4 D3/3: node = 25 D3/3: n_lines = 3 D3/3: i = 0 line = 45 angle = -2.446704 D3/3: i = 1 line = 119 angle = -2.371544 D3/3: i = 2 line = -41 angle = 0.770048 D3/3: current position = 2 D3/3: next = 1 line = 119 angle = -2.371544 D3/3: this one D3/3: dig_node_line_angle: node = 25 line = 119 D3/3: Unclosed area: D3/3: n_lines = 0 D3/3: Build area for line = -29, side = 1 D3/3: Vect_build_line_area() line = 29, side = 1 D3/3: dig_line_get_area(): line = 29, side = 1 (left), area = 0 D3/3: dig_build_area_with_line(): first_line = 29, side = 1 D3/3: dig_node_line_angle: node = 12 line = 29 D3/3: dig__angle_next_line: line = 29, side = 2, type = 4 D3/3: node = 12 D3/3: n_lines = 1 D3/3: i = 0 line = 29 angle = 2.079045 D3/3: current position = 0 D3/3: next = 0 line = 29 angle = 2.079045 D3/3: this one D3/3: next_line = 29 D3/3: dig_node_angle_check: line = 29, type = 4 D3/3: dig_node_line_angle: node = 12 line = 29 D3/3: dig__angle_next_line: line = 29, side = 2, type = 4 D3/3: node = 12 D3/3: n_lines = 1 D3/3: i = 0 line = 29 angle = 2.079045 D3/3: current position = 0 D3/3: next = 0 line = 29 angle = 2.079045 D3/3: this one D3/3: dig_node_line_angle: node = 12 line = 29 D3/3: The line to the right has the same angle: node = 12, line = 29 D3/3: Cannot build area, a neighbour of the line 29 has the same angle at the node D3/3: n_lines = 0 D3/3: Vect_attach_isles () D3/3: Vect_select_isles_by_box() D3/3: Box(N,S,E,W,T,B): 5.853381e+06, 5.851999e+06, 1.773754e+06, 1.772819e+06, 0.000000e+00, 0.000000e+00 D3/3: dig_select_isles() D3/3: 1 isles selected D3/3: number of isles to attach = 1 D3/3: Vect_attach_isle (): isle = 1 D3/3: Vect_isle_find_area () island = 1 D3/3: Vect_select_areas_by_box() D3/3: Box(N,S,E,W,T,B): 5.850540e+06, 5.850540e+06, 1.770830e+06, 1.770830e+06, 1.797693e+308, -1.797693e+308 D3/3: dig_select_areas() D3/3: 1 areas selected D3/3: area = 20 pointer to area structure = 910d088 D3/3: 1 areas overlap island boundary point D3/3: area = 20 D3/3: area inside isolated isle D3/3: Island 1 is not in area D3/3: isle = 1 -> area outside = 0 D3/3: updated lines : 0 , updated nodes : 0 D3/3: delete line 29 D3/3: Vect_delete_line(): name = vbuf_fill_bug_buf_dyn, line = 29 D3/3: V2_delete_line_nat(), line = 29 D3/3: V1_delete_line_nat(), offset = 5086 D3/3: dig__angle_next_line: line = 29, side = 2, type = 4 D3/3: node = 12 D3/3: n_lines = 1 D3/3: i = 0 line = 29 angle = 2.079045 D3/3: current position = 0 D3/3: next = 0 line = 29 angle = 2.079045 D3/3: this one D3/3: dig__angle_next_line: line = 29, side = 1, type = 4 D3/3: node = 12 D3/3: n_lines = 1 D3/3: i = 0 line = 29 angle = 2.079045 D3/3: current position = 0 D3/3: next = 0 line = 29 angle = 2.079045 D3/3: this one D3/3: dig__angle_next_line: line = -29, side = 2, type = 4 D3/3: node = 13 D3/3: n_lines = 2 D3/3: i = 0 line = -29 angle = -0.800748 D3/3: i = 1 line = 30 angle = 2.340845 D3/3: current position = 0 D3/3: next = 1 line = 30 angle = 2.340845 D3/3: this one D3/3: dig__angle_next_line: line = -29, side = 1, type = 4 D3/3: node = 13 D3/3: n_lines = 2 D3/3: i = 0 line = -29 angle = -0.800748 D3/3: i = 1 line = 30 angle = 2.340845 D3/3: current position = 0 D3/3: next = 1 line = 30 angle = 2.340845 D3/3: this one D3/3: dig_del_line() line = 29 D3/3: dig_spidx_del_line(): line = 29 D3/3: box(x1,y1,z1,x2,y2,z2): 1773637.964964 5852700.640374 0.000000 1773675.441777 5852745.598814 0.000000 D3/3: ret = 0 D3/3: node 12 has 0 lines -> delete D3/3: dig_spidx_del_node(): node = 12 D3/3: Build area for line = 29, side = 2 D3/3: Vect_build_line_area() line = 29, side = 2 Segmentation fault
Moritz
follow-up: 29 comment:27 by , 13 years ago
Replying to mlennert:
However, testing with the other example (see Mon, Oct 9 2006 08:02:33) from the old RT report gives the following, seems to reveal some remaining issues.
First of all, I cannot reproduce the procedure exactly, as the v.buffer call with bufcolumn=lane_cout gives me
> GRASS 7.0.svn (nc_spm_06):~ > v.buffer vbuf_fill_bug bufcol=lane_count out=vbuf_fill_bug_lanes scale=150 > ATTENTION : The bufcol option may contain bugs during the cleaning step. If > you encounter problems, use the debug option or clean manually > with v.clean tool=break; v.category step=0; v.extract -d > type=area > ERREUR :The bufcol option requires a valid layer.
Note that v.buffer in grass 7 is the same like v.buffer2 in grass 6.x. The original v.buffer has been removed from grass 7 some time ago. Testing the buffer/parallel modules should take place in grass 6.5 only.
It seems v.buffer2 has issues with the test cases described in the old RT bug 2765 whereas v.buffer does not...
comment:28 by , 13 years ago
Replying to mlennert:
However, testing with the other example (see Mon, Oct 9 2006 08:02:33) from the old RT report gives the following, seems to reveal some remaining issues.
First of all, I cannot reproduce the procedure exactly, as the v.buffer call with bufcolumn=lane_cout gives me
> GRASS 7.0.svn (nc_spm_06):~ > v.buffer vbuf_fill_bug bufcol=lane_count out=vbuf_fill_bug_lanes scale=150 > ATTENTION : The bufcol option may contain bugs during the cleaning step. If > you encounter problems, use the debug option or clean manually > with v.clean tool=break; v.category step=0; v.extract -d > type=area > ERREUR :The bufcol option requires a valid layer.
Looking at the table connection info:
> GRASS 7.0.svn (nc_spm_06):~ > v.db.connect -p $OUTMAP > Vector map <vbuf_fill_bug> is connected by: > layer <1/vbuf_fill_bug> table <vbuf_fill_bug> in database </home/mlennert/GRASS/DATA/nc_spm_06/mlennert/dbf/> through driver <dbf> with key <cat>
the layer <1/vbuf_fill_bug> looks weird. This should be layer <1>, no ? (At least it is in dev6).
The syntax is <number>/<name>, nothing wrong there. When using v.buffer(2) with the bufcolumn option, layer must be specified because it defaults to all layers (-1). Without the bufcolumn option, all features of the selected type should be buffered, also those without category in one of the map's layers.
Markus M
follow-up: 30 comment:29 by , 13 years ago
Replying to mmetz:
Replying to mlennert:
However, testing with the other example (see Mon, Oct 9 2006 08:02:33) from the old RT report gives the following, seems to reveal some remaining issues.
First of all, I cannot reproduce the procedure exactly, as the v.buffer call with bufcolumn=lane_cout gives me
> GRASS 7.0.svn (nc_spm_06):~ > v.buffer vbuf_fill_bug bufcol=lane_count out=vbuf_fill_bug_lanes scale=150 > ATTENTION : The bufcol option may contain bugs during the cleaning step. If > you encounter problems, use the debug option or clean manually > with v.clean tool=break; v.category step=0; v.extract -d > type=area > ERREUR :The bufcol option requires a valid layer.Note that v.buffer in grass 7 is the same like v.buffer2 in grass 6.x. The original v.buffer has been removed from grass 7 some time ago. Testing the buffer/parallel modules should take place in grass 6.5 only.
Oops. I had forgotten that I'd done the tests in dev6 before, and just assumed that it was in grass7.
It seems v.buffer2 has issues with the test cases described in the old RT bug 2765 whereas v.buffer does not...
Just tested in dev6: I actually don't see any difference between v.buffer and v.buffer2 with the two test cases mentioned above. Both work fine.
Sorry for the noise...
And I still support Markus in replacing v.buffer2 by v.buffer. I know this will be frustrating to see a SoC project apparently disappear, but IIUC, Markus has actually integrated some of the code into v.buffer, or ?
Moritz
comment:30 by , 13 years ago
Replying to mlennert:
[snip]
It seems v.buffer2 has issues with the test cases described in the old RT bug 2765 whereas v.buffer does not...
Just tested in dev6: I actually don't see any difference between v.buffer and v.buffer2 with the two test cases mentioned above. Both work fine.
So the segfault in G7 needs to be explained. BTW, works fine for me in G7.
And I still support Markus in replacing v.buffer2 by v.buffer. I know this will be frustrating to see a SoC project apparently disappear, but IIUC, Markus has actually integrated some of the code into v.buffer, or ?
Yes, the solution for the buffer column bug has been adapted from v.buffer2 to v.buffer. And it is indeed frustrating to disable a GSoC project output. I started with v.buffer2, but I do not understand relevant parts of the code in buffer2.c, dgraph.[c|h], e_intersect.[c|h], all in lib/vector/Vlib/, so I went for the older v.buffer. Maybe someone else has more success.
Anyway, the lesson learned is that GSoC needs more thorough quality control before broken modules end up in a stable release branch.
Markus M
Hopefully fixed in r38521, r38522, r38523 (trunk, 65, and 64 respectively).