Skip to content

TrackballControls: extends OrbitControls - API refactor #17857

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

TrackballControls: extends OrbitControls - API refactor #17857

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
149 changes: 146 additions & 3 deletions examples/js/controls/OrbitControls.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ THREE.OrbitControls = function ( object, domElement ) {
// Set to false to disable rotating
this.enableRotate = true;
this.rotateSpeed = 1.0;
this.fixedUp = true; // maintains camera.up constant

// Set to false to disable panning
this.enablePan = true;
Expand Down Expand Up @@ -131,11 +132,14 @@ THREE.OrbitControls = function ( object, domElement ) {
this.update = function () {

var offset = new THREE.Vector3();
var yUp = new THREE.Vector3( 0, 1, 0 );

// so camera.up is the orbit axis
var quat = new THREE.Quaternion().setFromUnitVectors( object.up, new THREE.Vector3( 0, 1, 0 ) );
var quatInverse = quat.clone().inverse();
var quat = new THREE.Quaternion();
var quatInverse = new THREE.Quaternion();
var axisQuat = new THREE.Quaternion();

var lastUp = new THREE.Vector3();
var lastDir = new THREE.Vector3();
var lastPosition = new THREE.Vector3();
var lastQuaternion = new THREE.Quaternion();

Expand All @@ -145,6 +149,21 @@ THREE.OrbitControls = function ( object, domElement ) {

offset.copy( position ).sub( scope.target );

if ( ! scope.fixedUp ) {

lastDir.copy( offset ).normalize();

}

if ( ! scope.object.up.equals( lastUp ) ) {

quat.setFromUnitVectors( scope.object.up, yUp );
quatInverse.copy( quat ).inverse();

lastUp.copy( scope.object.up );

}

// rotate offset to "y-axis-is-up" space
offset.applyQuaternion( quat );

Expand Down Expand Up @@ -202,6 +221,16 @@ THREE.OrbitControls = function ( object, domElement ) {

position.copy( scope.target ).add( offset );

if ( ! scope.fixedUp ) {

offset.normalize();
axisQuat.setFromUnitVectors( lastDir, offset );

// rotate camera.up along with offset
scope.object.up.applyQuaternion( axisQuat );

}

scope.object.lookAt( scope.target );

if ( scope.enableDamping === true ) {
Expand Down Expand Up @@ -1178,3 +1207,117 @@ THREE.MapControls = function ( object, domElement ) {

THREE.MapControls.prototype = Object.create( THREE.EventDispatcher.prototype );
THREE.MapControls.prototype.constructor = THREE.MapControls;

// This set of controls performs orbiting, dollying (zooming), and panning.
// Unlike OrbitControls, it rotates the "up" direction camera.up.
//
// Orbit - left mouse / touch: one-finger move
// Zoom - middle mouse, or mousewheel / touch: two-finger spread or squish
// Pan - right mouse, or left mouse + ctrl/meta/shiftKey, or arrow keys / touch: two-finger move

THREE.TrackballControls = function ( object, domElement ) {
Copy link
Collaborator

@Mugen87 Mugen87 Nov 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really think this code and the definition of MapControls should go in its own module file. The following code looks weird, IMO:

import { TrackballControls } from './jsm/controls/OrbitControls.js';

Yes, I'm aware that the JS versions would have a dependency to OrbitControls but I think this is still better than having strange imports.

Copy link
Contributor Author

@sciecode sciecode Nov 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, not sure actually. This is more of a preference / library consistency kind of deal, so I think that's up to you guys. I'll be happy to change if we decide on that course of action.

Copy link
Contributor Author

@sciecode sciecode Nov 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However, if we choose to do it. Keeping the old API available won't be an option and we will need to apply backwards compatibility.

The sensitivity and keyboard control are different from the original TrackballControls.

^ this could be a problem for the users.

Copy link
Collaborator

@Mugen87 Mugen87 Nov 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's necessary to keep the old API around. Users can checkout the repo at any release and use the respective version of TrackballControls if necessary. There are even live links to older versions:

https://rawcdn.githack.com/mrdoob/three.js/r109/examples/js/controls/TrackballControls.js

Copy link
Collaborator

@WestLangley WestLangley Nov 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Come on, @Mugen87. We tell users to always use files from the same release.

Copy link
Collaborator

@Mugen87 Mugen87 Nov 5, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I really don't understand your argumentation. We normally remove legacy/obsolete classes with very few exceptions. If we would keep all files, the repository would not be manageable anymore.

If users can't invest time in migration, it's okay to use one of the previous releases. Hence, I don't get your goal with retaining the API of TrackballControls. It's good to align it without keeping the old stuff around. Moreover, as demonstrated by my PR, API changes can be often done gracefully (meaning backwards compatible).

Copy link
Collaborator

@WestLangley WestLangley Nov 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we would keep all files, the repository would not be manageable anymore.

Come on, dude.

Copy link
Collaborator

@WestLangley WestLangley Nov 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get your goal with retaining the API of TrackballControls.

#17858 (comment)


THREE.OrbitControls.call( this, object, domElement );

this.fixedUp = false;
this.enableDamping = true;
this.screenSpacePanning = true;

};

THREE.TrackballControls.prototype = Object.create( THREE.EventDispatcher.prototype );
THREE.TrackballControls.prototype.constructor = THREE.TrackballControls;

Object.defineProperties( THREE.TrackballControls.prototype, {

noZoom: {

get: function () {

console.warn( 'THREE.TrackballControls: .noZoom has been deprecated. Use .enableZoom instead.' );
return ! this.enableZoom;

},

set: function ( value ) {

console.warn( 'THREE.TrackballControls: .noZoom has been deprecated. Use .enableZoom instead.' );
this.enableZoom = ! value;

}

},

noRotate: {

get: function () {

console.warn( 'THREE.TrackballControls: .noRotate has been deprecated. Use .enableRotate instead.' );
return ! this.enableRotate;

},

set: function ( value ) {

console.warn( 'THREE.TrackballControls: .noRotate has been deprecated. Use .enableRotate instead.' );
this.enableRotate = ! value;

}

},

noPan: {

get: function () {

console.warn( 'THREE.TrackballControls: .noPan has been deprecated. Use .enablePan instead.' );
return ! this.enablePan;

},

set: function ( value ) {

console.warn( 'THREE.TrackballControls: .noPan has been deprecated. Use .enablePan instead.' );
this.enablePan = ! value;

}

},

staticMoving: {

get: function () {

console.warn( 'THREE.TrackballControls: .staticMoving has been deprecated. Use .enableDamping instead.' );
return ! this.enableDamping;

},

set: function ( value ) {

console.warn( 'THREE.TrackballControls: .staticMoving has been deprecated. Use .enableDamping instead.' );
this.enableDamping = ! value;

}

},

dynamicDampingFactor: {

get: function () {

console.warn( 'THREE.TrackballControls: .dynamicDampingFactor has been renamed. Use .dampingFactor instead.' );
return this.dampingFactor;

},

set: function ( value ) {

console.warn( 'THREE.TrackballControls: .dynamicDampingFactor has been renamed. Use .dampingFactor instead.' );
this.dampingFactor = value;

}

}

} );
151 changes: 147 additions & 4 deletions examples/jsm/controls/OrbitControls.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ var OrbitControls = function ( object, domElement ) {
// Set to false to disable rotating
this.enableRotate = true;
this.rotateSpeed = 1.0;
this.fixedUp = true; // maintains camera.up constant

// Set to false to disable panning
this.enablePan = true;
Expand Down Expand Up @@ -141,11 +142,14 @@ var OrbitControls = function ( object, domElement ) {
this.update = function () {

var offset = new Vector3();
var yUp = new Vector3( 0, 1, 0 );

// so camera.up is the orbit axis
var quat = new Quaternion().setFromUnitVectors( object.up, new Vector3( 0, 1, 0 ) );
var quatInverse = quat.clone().inverse();
var quat = new Quaternion();
var quatInverse = new Quaternion();
var axisQuat = new Quaternion();

var lastUp = new Vector3();
var lastDir = new Vector3();
var lastPosition = new Vector3();
var lastQuaternion = new Quaternion();

Expand All @@ -155,6 +159,21 @@ var OrbitControls = function ( object, domElement ) {

offset.copy( position ).sub( scope.target );

if ( ! scope.fixedUp ) {

lastDir.copy( offset ).normalize();

}

if ( ! scope.object.up.equals( lastUp ) ) {

quat.setFromUnitVectors( scope.object.up, yUp );
quatInverse.copy( quat ).inverse();

lastUp.copy( scope.object.up );

}

// rotate offset to "y-axis-is-up" space
offset.applyQuaternion( quat );

Expand Down Expand Up @@ -212,6 +231,16 @@ var OrbitControls = function ( object, domElement ) {

position.copy( scope.target ).add( offset );

if ( ! scope.fixedUp ) {

offset.normalize();
axisQuat.setFromUnitVectors( lastDir, offset );

// rotate camera.up along with offset
scope.object.up.applyQuaternion( axisQuat );

}

scope.object.lookAt( scope.target );

if ( scope.enableDamping === true ) {
Expand Down Expand Up @@ -1189,4 +1218,118 @@ var MapControls = function ( object, domElement ) {
MapControls.prototype = Object.create( EventDispatcher.prototype );
MapControls.prototype.constructor = MapControls;

export { OrbitControls, MapControls };
// This set of controls performs orbiting, dollying (zooming), and panning.
// Unlike OrbitControls, it rotates the "up" direction camera.up.
//
// Orbit - left mouse / touch: one-finger move
// Zoom - middle mouse, or mousewheel / touch: two-finger spread or squish
// Pan - right mouse, or left mouse + ctrl/meta/shiftKey, or arrow keys / touch: two-finger move

var TrackballControls = function ( object, domElement ) {

OrbitControls.call( this, object, domElement );

this.fixedUp = false;
this.enableDamping = true;
this.screenSpacePanning = true;

};

TrackballControls.prototype = Object.create( EventDispatcher.prototype );
TrackballControls.prototype.constructor = TrackballControls;

Object.defineProperties( TrackballControls.prototype, {

noZoom: {

get: function () {

console.warn( 'THREE.TrackballControls: .noZoom has been deprecated. Use .enableZoom instead.' );
return ! this.enableZoom;

},

set: function ( value ) {

console.warn( 'THREE.TrackballControls: .noZoom has been deprecated. Use .enableZoom instead.' );
this.enableZoom = ! value;

}

},

noRotate: {

get: function () {

console.warn( 'THREE.TrackballControls: .noRotate has been deprecated. Use .enableRotate instead.' );
return ! this.enableRotate;

},

set: function ( value ) {

console.warn( 'THREE.TrackballControls: .noRotate has been deprecated. Use .enableRotate instead.' );
this.enableRotate = ! value;

}

},

noPan: {

get: function () {

console.warn( 'THREE.TrackballControls: .noPan has been deprecated. Use .enablePan instead.' );
return ! this.enablePan;

},

set: function ( value ) {

console.warn( 'THREE.TrackballControls: .noPan has been deprecated. Use .enablePan instead.' );
this.enablePan = ! value;

}

},

staticMoving: {

get: function () {

console.warn( 'THREE.TrackballControls: .staticMoving has been deprecated. Use .enableDamping instead.' );
return ! this.enableDamping;

},

set: function ( value ) {

console.warn( 'THREE.TrackballControls: .staticMoving has been deprecated. Use .enableDamping instead.' );
this.enableDamping = ! value;

}

},

dynamicDampingFactor: {

get: function () {

console.warn( 'THREE.TrackballControls: .dynamicDampingFactor has been renamed. Use .dampingFactor instead.' );
return this.dampingFactor;

},

set: function ( value ) {

console.warn( 'THREE.TrackballControls: .dynamicDampingFactor has been renamed. Use .dampingFactor instead.' );
this.dampingFactor = value;

}

}

} );

export { OrbitControls, MapControls, TrackballControls };
4 changes: 1 addition & 3 deletions examples/misc_controls_trackball.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
import Stats from './jsm/libs/stats.module.js';
import { GUI } from './jsm/libs/dat.gui.module.js';

import { TrackballControls } from './jsm/controls/TrackballControls.js';
import { TrackballControls } from './jsm/controls/OrbitControls.js';

var perspectiveCamera, orthographicCamera, controls, scene, renderer, stats;

Expand Down Expand Up @@ -142,8 +142,6 @@

renderer.setSize( window.innerWidth, window.innerHeight );

controls.handleResize();

}

function animate() {
Expand Down