OrbitControls: Support Ctrl-drag as right-drag alternative.

[rkirsling]

Resolves #13970.

Notes:

1. As mentioned in the corresponding issue, whether this is appropriate default behavior or should simply be configurable by users is your call.

2. Since this change requires introducing support not only for Ctrl (and perhaps Meta/Alt/Shift) but also for associating multiple triggers with a single action, I believe it's simplest to use lambda predicates (rather than, say, arrays of objects, which we'd need a function call to filter through anyway).

3. Admittedly ORBIT(event) looks a bit odd due to the enum casing, but I feel like this is preferable to juggling a possibly-undefined function isOrbit alongside ORBIT.

[WestLangley]

Without passing judgement on the advisability of this feature, I would consider a different implementation:

function onMouseDown( event ) {

	if ( scope.enabled === false ) return;

	event.preventDefault();

	switch ( scope.mouseMaping( event ) ) {

		case scope.mouseButtons.ORBIT:
	. . .

Then

this.mouseButtons = { ORBIT: 1001, ZOOM: 1002, PAN: 1003 }; // arbitrary values

this.mouseMaping = function ( event ) {

	if ( event.button === THREE.MOUSE.LEFT && ! event.ctrlKey ) return scope.mouseButtons.ORBIT;

	if ( event.button === THREE.MOUSE.MIDDLE ) return scope.mouseButtons.ZOOM;

	if ( event.button === THREE.MOUSE.RIGHT || ( event.button === THREE.MOUSE.LEFT && event.ctrlKey ) ) return scope.mouseButtons.PAN;

	return null;

}

I would expect there is even a better way...

[rkirsling]

Sure, another implementation I'd been considering is similar to that. It preserves the switch but calls a new scope.getStateForMouseDown( event ) and switches on the new STATE enum value.

Reasoning:

• If we're concerned about backwards-compatibility, then we can't change the type of mouseButtons in a destructive way, and so we need the new function to use the buttons and not return them.

• If we preserve the switch, we're going to have to walk two three-tiered conditionals (three ifs and three cases) in a row to determine whether a mousedown event is a PAN action, but switching on the new STATE lessens redundancy and allows for a bit of simplification in onMouseDown.

I'll update the PR to go with that approach—hope it's more satisfying to you. 😄

[rkirsling]

Whoops, forgot to comment after updating, but this PR should be ready for another review whenever. 😅

[WestLangley]

I like this feature. Let's see if we can get some feedback from others.

Sorry, I'm still not sure about the implementation details, though...

[rkirsling]

@WestLangley Thanks! Is there anything I can/should be doing to elicit such feedback?

[WestLangley]

Maybe @moroine has some suggestions...

[moroine]

Nice feature, about the implementation I would go with bit-mask but I might be too complicated to use for beginners.

Having something like

THREE.MODIFIER_KEYS = {  DEFAULT: 0, CTRL: 1, ALT: 2, SHIFT: 4, META: 8 };

this.mouseButtons = { 
  ORBIT: {
     [THREE.MODIFIER_KEYS.DEFAULT]: THREE.MOUSE.LEFT, // default one
     [THREE.MODIFIER_KEYS.CTRL]: 0 // No binding ==> disabled
  },
  ZOOM: THREE.MOUSE.MIDDLE,
  PAN: {
    [THREE.MODIFIER_KEYS.DEFAULT]: THREE.MOUSE.RIGHT,
    [THREE.MODIFIER_KEYS.CTRL]: THREE.MOUSE.LEFT, 
  }
};

Then we could use bit-mask to combine modifiers.

We could ensure the backward compatibility, but I'm not even convinced myself 😞

About the feature itself, I've face a limitation. When you start rotating and press CTRL it doesn't switch to Drag mode until the next mouseup event.

[rkirsling]

Assuming we allow users to configure this (instead of just silently equating Ctrl-drag to right-drag internally, which I suppose is another valid possibility?), I agree that a bitmask approach seems like an overly expert-oriented API. 😓

Regarding the limitation you described, I would personally consider this as expected behavior. One major precedent for this is Google Earth, in which Shift-drag changes the rotation (but only at mousedown).

[moroine]

On the other hand your method getStateForMouseDown can be "public option" used to configure the behavior.

This would replace mouseButtons, we could let mouseButtons in order to do easy configuration and let the ability to replace getStateForMouseDown for more advanced configurations.

[rkirsling]

I feel like we've got a sort of quiet consensus here—could we consider this ready?

[WestLangley]

I propose new nomenclature that is more general, but still allows users to swap buttons:

this.mouseButtons = { LEFT: THREE.MOUSE.LEFT, MIDDLE: THREE.MOUSE.MIDDLE, RIGHT: THREE.MOUSE.RIGHT };

And then, make this minimal change to onMouseDown():

function onMouseDown( event ) {

	if ( scope.enabled === false ) return;

	event.preventDefault();

	switch ( event.button ) {

		case scope.mouseButtons.LEFT:

			if ( event.ctrlKey || event.metaKey ) {

				if ( scope.enablePan === false ) return;

				handleMouseDownPan( event );

				state = STATE.PAN;

			} else {

				if ( scope.enableRotate === false ) return;

				handleMouseDownRotate( event );

				state = STATE.ROTATE;

			}

			break;

		case scope.mouseButtons.MIDDLE:

			if ( scope.enableZoom === false ) return;

			handleMouseDownDolly( event );

			state = STATE.DOLLY;

			break;

		case scope.mouseButtons.RIGHT:

			if ( scope.enablePan === false ) return;

			handleMouseDownPan( event );

			state = STATE.PAN;

			break;

	}

	if ( state !== STATE.NONE ) {

		document.addEventListener( 'mousemove', onMouseMove, false );
		document.addEventListener( 'mouseup', onMouseUp, false );

		scope.dispatchEvent( startEvent );

	}

}

I know this nomenclature change is not backwards-compatible, but the current nomenclature is too restrictive for the feature you wish to add.

I think this is the cleanest way to implement your feature.

Note, so as to parallel the Google maps API, I added support for both event.ctrlKey and event.metaKey.

[rkirsling]

Works for me! 😄

[WestLangley]

@mrdoob I think this is worthy of being merged.

[mrdoob]

Thanks!