search

mosra / magnum-integration

Integration libraries for the Magnum C++11 graphics engine
https://magnum.graphics/
Other
99 stars 44 forks source link

BulletIntegration: fix Matrix3x3 conversion along with unit tests #34

Closed xqms closed 5 years ago

xqms commented 5 years ago

Bullet matrices are row-major, Magnum matrices are column-major. The Matrix3x3 unit test did not account for this and tested the wrong behavior. The test for Matrix4x4 seems fine, haven't looked in detail, though.

This probably hasn't been noticed before because BulletIntegration::MotionState uses the Matrix3x3 conversion only in the Magnum -> Bullet direction, and this is typically only done once during btRigidBody initialization. The bullet example in magnum-examples uses identity rotation at initialization...

mosra commented 5 years ago

Hi, thanks a lot for reporting and fixing this!

Interesting (and frightening) that this hasn't been noticed yet, huh. Looking into Bullet sources at how the Matrix4 conversion is done, yeah this was definitely wrong. @Squareys were you aware of this?

Merged your commit in 67059e523b1f61ce0ce6f117380736b64bde7de8 (without the debug prints) and then, in 13356bf68e78ebef2e4b3ebfa375d10c641ce9ee, to be really sure, updated the 3x3 test to now also convert the matrix to a quaternion and check that it indeed produces a correct value.

xqms commented 5 years ago

Ah, sorry, the debug stuff should not have been in there, that slipped through. Thanks for checking and merging :-)