YiMingFu / recastnavigation

Automatically exported from code.google.com/p/recastnavigation
zlib License
0 stars 0 forks source link

Possible bug in simplifyContour #187

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?
1. Please see the image.

What version of the product are you using? On what operating system?
Latest Recast version, Windows 7

Please provide any additional information below.

I think there is some bug in simplifyContour function, sometimes it enters 
infinite loop in the "// Split too long edges." section.
I've did some debugging and it turns out that 'maxi' gets sometimes equal to 
'ai' or 'bi' which I think shouldn't happen.
The yellow line is where the breakpoint entered, you can see that 'maxi==0' and 
'bi==0'

in the breakpoint n=1, pn=185

Since this is splitting too long edges I think the middle edge point 'maxi' 
should not be equal to one of the original test points 'ai' 'bi'

is the "maxi = (ai + (n+1)/2) % pn;" calculation formula incorrect?
or additional checks should be used:
"if(maxi==ai || maxi==bi)maxi=-1;"

in that situation 'simplified' keeps growing and growing, loop becomes infinite

Original issue reported on code.google.com by esent...@gmail.com on 29 Nov 2011 at 9:44

Attachments:

GoogleCodeExporter commented 8 years ago
If you can rerun the test, what is 'n' in the case?

Original comment by memono...@gmail.com on 30 Nov 2011 at 11:01

GoogleCodeExporter commented 8 years ago
I have included this in my previous post
n=1 pn=185

Original comment by esent...@gmail.com on 30 Nov 2011 at 11:04

GoogleCodeExporter commented 8 years ago
Sorry, skimmed your message too quickly.

Obviously there should be test to see that n > 1. Something like this (around 
line 423):

const int n = bi < ai ? (bi+pn - ai) : (bi - ai);
if (n > 1)
{
  if (bx > ax || (bx == ax && bz > az))
    maxi = (ai + n/2) % pn;
  else
    maxi = (ai + (n+1)/2) % pn;
}

Let me know if it fixes it.

Original comment by memono...@gmail.com on 30 Nov 2011 at 11:21

GoogleCodeExporter commented 8 years ago
Yes, this code is now correct.
Will the change be commited into the source?

By the way: I'm using your library in my game engine : http://www.esenthel.com/
Thanks for making it!

Original comment by esent...@gmail.com on 3 Dec 2011 at 11:22

GoogleCodeExporter commented 8 years ago
Fixed in R320.

Original comment by memono...@gmail.com on 6 Dec 2011 at 11:38