search

mattdesl / three-orbit-controls

orbit controls for ThreeJS
216 stars 72 forks source link

camera up vector not respected when switching cameras #9

Open bsergean opened 9 years ago

bsergean commented 9 years ago

In an application that support multiple view / camera, switching camera can be done by setting the .object member variable.

this.controls.object = otherCamera

The problem is that if this camera has an up vector different from the default value, that up vector won't be respected. However, OrbitControls has support for non standard (0, 1, 0) up vectors since some code respect it to compute its internal quaternion. (cf those 2 lines of code in the "constructor").

var quat = new THREE.Quaternion().setFromUnitVectors( object.up, new THREE.Vector3( 0, 1, 0 ) );
var quatInverse = quat.clone().inverse();

A patch request follows, which make quat and quatInverse member variables and turn the previous block of code into a method that is called on update.

mattdesl commented 9 years ago

Ah, cool. I assume this is also pushed to ThreeJS examples/? I am trying to keep this mostly in sync with the ThreeJS code. :smile:

bsergean commented 9 years ago

Of course. Not yet submitted to three.js/example but indeed it should be done. Last time I looked at r72 of three.js the OrbitController code had changed quite a lot. I'm forking three.js now to apply that fix in there as well. Sounds like another ticket to sync to the latest three.js should be independently created.

On Sun, Nov 1, 2015 at 8:07 AM, Matt DesLauriers notifications@github.com wrote:

Ah, cool. I assume this is also pushed to ThreeJS examples/? I am trying to keep this mostly in sync with the ThreeJS code. [image: :smile:]

— Reply to this email directly or view it on GitHub https://github.com/mattdesl/three-orbit-controls/issues/9#issuecomment-152840682 .

bsergean commented 9 years ago

Created this: https://github.com/mattdesl/three-orbit-controls/issues/11

bsergean commented 9 years ago

... this appear to be fixed already in the latest three.js :)

https://github.com/mrdoob/three.js/blob/master/examples/js/controls/OrbitControls.js#L203

quat and quatInverse are already updated inside of the update method. So if we update to the latest three.js code this ticket can be closed. This bug fix would only be interesting for the current release of three-orbit-controls.

mattdesl commented 9 years ago

I pushed this to 71.2.0. I think we will need the same changes in the new version?

bsergean commented 9 years ago

We shouldn't, it looks like this was fixed already upstream but I should test better. I actually tried the version 0.72 (I changed my code so that I can run on three.js 0.72), and apparently with my application which does change the up vector and changing cameras wasn't working well. I did that in a hurry so I should try again / troubleshoot this better. npm start and using the local test was working fine though.

In any case 71.2.0 works great for me.

On Mon, Nov 2, 2015 at 7:55 AM, Matt DesLauriers notifications@github.com wrote:

I pushed this to 71.2.0. I think we will need the same changes in the new version?

— Reply to this email directly or view it on GitHub https://github.com/mattdesl/three-orbit-controls/issues/9#issuecomment-153060792 .