search

Izzimach / react-three-legacy

React bindings to create and control a 3D scene using three.js
Other
1.52k stars 128 forks source link

Fixed issue with quaternion rotations being overwritten #93

Closed slindberg closed 7 years ago

slindberg commented 7 years ago

With the introduction of the rotation property in #65, setting the quaternion property on 3D objects no longer has any effect. This is because the object's rotation gets reset back to nothing on this line.

This PR changes the behavior to look first for the quaternion property, then the rotation property, and only when neither are present does it reset the rotation.

slindberg commented 7 years ago

EDIT

Also, if you're not opposed, I'd like to sneak in another commit that adds support for the matrix property. Currently I have an app that deals with matrix rotation transformations, and I'm creating quaternions from them just so I can set the quaternion property.

The matrix, quaternion, and rotation properties are all essentially mutually exclusive since they define a local transformations, but the matrix property is more generic (it can include scaling, etc.), so the added logic would check for matrix first. What do you think?

There is one more local transformation property, scale that operates independently from quaternion and rotation, but affects matrix. So if you wanted to be more thorough, I could open another PR that adds logic for handling all four local transformation properties together.

I realize now that scale (and position) is already supported, so adding support for matrix would need to account for it being mutually exclusive with quaternion, rotation, scale, and position. Best do that in another PR if you are amenable.

slindberg commented 7 years ago

Fixed by #94