Improve OrbitControls use case #12727
Conversation
|
... and mouse controls have been swapped. https://rawgit.com/clement-igonet/three.js/orbitControls_example/examples/misc_controls_orbit.html |
examples/misc_controls_orbit.html
Outdated
| 'textures/land_ocean_ice_cloud_2048.jpg', | ||
| function( tex ) { | ||
| material.map = tex; | ||
| material.needsUpdate = true; |
There was a problem hiding this comment.
FYI. This coding pattern causes two shaders to be compiled -- one with a map and one without.
|
It no longer... orbits? 🤔 |
|
Good question.
In fact, I've swapped mouse button. So, finally unswapped...
And about panning, what's the move really expected?
I mean, pan move is relative to the camera axis? or parallel to something relative to the scene origin?
Indeed I do not understand the benefit of the existing pan move implementation. It moves the camera target relatively to the camera, and then we lost the connection with a plan relative to scene axes...
I would suggest to clarify the expected behavior and associated relevant use case (if currently any) before reworking on it.
|
|
Ok, I'm going to unswap mouse buttons if disturbing. |
|
See if #12742 addresses your concerns. |
|
Not really...
|
|
Target axes added to the scene. |
| v.setFromMatrixColumn( objectMatrix, 1 ); // get Y column of objectMatrix | ||
| v.setFromMatrixColumn( objectMatrix, 0 ); // get X column of objectMatrix | ||
|
|
||
| v.applyAxisAngle( new THREE.Vector3( 0, 0, 1 ), Math.PI / 2 ); |
There was a problem hiding this comment.
This only works in your application because you have changed camera.up.
Also, avoid calling new unnecessarily. You are instantiating a Vector3 that must be garbage-collected.
Yes, we can improve OrbitControls panning, but this is not the way to do it.
There was a problem hiding this comment.
Ok. Copy.
My inspiration comes from here: https://stackoverflow.com/questions/10747510/how-to-rotate-a-three-js-vector3-around-an-axis/10747728#10747728
You commented but did not say anything about unnecessary "new" in this place.
At least, do you agree with the behaviour of the camera ?
|
camera.up removed. |
|
|
||
| v.applyAxisAngle( new THREE.Vector3( 0, 0, 1 ), Math.PI / 2 ); | ||
|
|
||
| v.applyAxisAngle( new THREE.Vector3( 0, 1, 0 ), Math.PI / 2 ); |
There was a problem hiding this comment.
This logic is not correct if camera.up is changed.
Also, we need to avoid instantiating a new Vector3 here. This method can be called many times per second.
Also, this PR will change the behavior for current users. It would be better to have the panning method as an option.
Comparing to previous example: