henrikglass / erodr

An implementation of Hans Theobald Beyer's algorithm for simulated hydraulic erosion
MIT License
46 stars 5 forks source link

Error in calculations #3

Open Nomad1 opened 4 months ago

Nomad1 commented 4 months ago

That's funny as hell, but the whole awesomeness of this algorithm relies on a floating point error. Take a look at this part of the code:

p.vel = sqrt(p.vel*p.vel + h_diff*params->p_gravity);
p.water *= (1 - params->p_evaporation);

h_diff is mostly negative and for small enough p.vel this results in NaN value for velocity. And there the magic starts: handling this NaN gracefully will ruin the algorithm since the particle will stop rolling and we need many more iterations or other parameters to have a good-looking erosion.

I found this bug while porting the algorithm to C# and it wasn't working since Math.Min checks for NaN values and returns NaN resulting in particle stopping, while fmin returns the number and particles move up for TTL steps or hitting the boundary.

To make the code work the same way I had to add the check if the value under the root is less than zero and if so, set the velocity to FLT_MAX. It's obviously wrong, although matches the original code.

Ported code: https://github.com/Nomad1/ErodrSharp

henrikglass commented 3 months ago

It's actually awesome that you ported my code to C#. Really cool! 😄

I see what you mean. That's really quirky lol. Although the implementation is just a straight up copy of Hans's paper so I'm blaming him (jk) ¯\_(ツ)_/¯

I bet you could do some complex number magic to get the particle do do something more reasonable when p.vel*p.vel + h_diff*params->p_gravity becomes less than zero, but I'm not smart enough to figure out exactly what on the spot.

Anyways, thanks for the headsup. I'm not actively maintaining this project at the moment, but I might as well get that stick out of my ass and take a look at this along with a few other probable bugs soon, since people actually seem to be using this code, which, tbh, is not something I expected.