Altered matrix inversion code to behave correctly with single precision zero scaling #7037
Conversation
|
Thanks for the pull request @mksquires!
Reviewers, don't forget to make sure that:
I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome. 🌍 🌎 🌏 |
Source/Core/Matrix4.js
Outdated
| @@ -2308,7 +2283,31 @@ define([ | |||
| // calculate determinant | |||
| var det = src0 * dst0 + src1 * dst1 + src2 * dst2 + src3 * dst3; | |||
|
|
|||
| if (Math.abs(det) < CesiumMath.EPSILON20) { | |||
| if (Math.abs(det) < 1e-21) { | |||
There was a problem hiding this comment.
Use CesiumMath.EPSILON20. Or if it needs to be 21 add CesiumMath.EPSILON21
|
I'll let @bagnell review and merge, but this change looks fine to me |
|
I made some minor tweaks in 93cb38e. Thanks @mksquires! |
|
Hello @GatorScot! Sorry I didn't respond earlier, I just noticed your question. Thanks @emackey for responding. I'll add a few thoughts; to really check for a singular matrix the optimal route would probably involve checking the matrix's condition number or its rank (but maybe this is too slow?). The second thing is that we should consider why we elect to stop rendering when we have an ill-conditioned matrix in the first place. There may be a way to more gracefully handle this issue without crashing. |
A fix for issue #6954 . Appropriate tests have been added.