residualvm/residualvm — pull request #1108 by chkr-private (2014-10-08 19:53:03+00:00)
Title: RFC: Fixes for the lava puzzle and generic handling of floating point rounding issues

This patch set fixes a bunch of issues seen especially in the lava puzzle:

### Boats don't collide
* since the boats are not moved via WalkActorTo but via SetActorAt, handleCollisionWith() was never called and so the LUA callback to handle the collision was not called as well
* simple bug fix: call handleCollisionWith() in Actor::setPos(), too - this fixes #877

### Boats are invisible
* running residualvm via apitrace revealed, that the boat's vertices consisted of just NaNs and so they were ignored by the OpenGL implementation
* they were computed using quats which contained just NaNs which were computed by normalizing a (0, 0, 0, 0) quat (which is basically a division by zero)
* commit b1af372 fixes that

### Generic floating point issues
* in floating point, some operations are valid (like x / 0.0f) but have an unexpected outcome (+/- inf for x!= 0 or NaN for x = 0.0f)
* these special values are propagated undetected through all calculations and may lead to strange effects (like the invisible boats)
* due to the nature of the limited accuracy of floating point operations, some calculations are critical:
* arcus cosinus/sinus if variable is outside of -1.0 ... 1.0
* e.g. the operand of arcCosine is sometimes just a little bit above 1.0:

Angle::arcCosine(dotProduct(v1, v2) / (v1.getMagnitude() * v2.getMagnitude()));

* even the mathematically valid formula like the one above may produce results which are just a little bit off due to float's accuracy and so they would cause effectively acosf() to generate NaN
* division by zero: can cause +/-inf or NaN
* probably others like sqrt() of the operand is smaller than 0.0f
* in order to track down these issues I have enabled the reporting of floating point exceptions when project is not compiled with --enable-release
* for the revealed exceptions I did the following:
* added asserts in the low-level math classes when undefined values are calculated or when the objects are created with undefined or non-meaningful values (like a line with no direction or a segment where the start and end point are equal)
* fixed this kind of usage in the higher-level objects (e.g. in Grim::Actor)

### Open items
* Is that approach helpful at all?
* To avoid unexpected trouble by the new asserts(), would it make sense to define NDEBUG for builds with --enable-release?
* Enable the reporting of floating point exceptions in all 3 games and play some more scenes in order to detect other areas where the inaccuracies of floating point operations could cause some trouble.