OrbitControls: customizable touch binds #16803
Conversation
What are the values you are using in your app? |
First time I modified, I disabled dollypan and set:
Second time:
|
|
Your suggestion has merits, but you can't pinch-to-zoom with one finger, and with three fingers it is -- well -- a bit odd. The code relies on two touches for zooming. I am reluctant to support adding a feature unless it just works, otherwise we get complaints, but as I said, your suggestion is not without merit.
I am on the fence... |
I completely agree. It is a bit awkward. The most useful case for this change is definitely being able to remap rotate to two fingers and allow one finger for custom events, to be honest. Let's see how others feel about it, but I'm not strongly attached to this idea as well. |
TBH, I don't think we need an API change for this rather unusual use case. The current touch interaction seems ideal to me and I would not encourage users to change with it a respective interface. |
I understand why you say that, but I don't think it's that unusual at all. Some applications might want to have a pan option as a single touch event. I'm not saying my approach in the PR is the one we should go for. Perhaps, rethinking a more customizable api would be the correct thing to do. I think other approaches are only unusual because we might've gotten used to this fixed scheme, not necessarily because there's no need for other interfaces. I understand that So I can see how adapting or rethinking the API could be an advantage in this scenario. |
I think @sciecode's use cases are perfectly reasonable -- it is just that the PR proposes an overly-general solution that allows options that do not make sense. |
|
As I have said previously, I am reluctant to support any further changes to However, on balance, I think this PR provides a valuable feature. I would request that the change also be applied to |
|
@Mugen87 @WestLangley thoughts on extending this approach by creating independent zoom and pan functionalities and also displaying error for zoom/dollypan on single touch? default api would look something like this:
It still doesn't solve this problem, but given that documentation is clear about it and that we provide a descriptive error on incorrect use. I don't see this being a big problem. On the plus side, it would allow for much better control over touch behaviors. |
|
@sciecode I prefer your current simple proposal. I wouldn't worry about parameter validation. |
|
I don't really see the use of having two classes that fulfill almost the same purpose, when we can adjust one to do both things ( and more ). That will require us to implement A different approach to this PR, would be to only allow certain touch actions based on the number of fingers. If an invalid configuration is used, we simply treat it as |
|
@sciecode I think this looks reasonable if you could have Can you specify some example mouse and touch settings we could try? Don't forget that some users just want to be able to swap left and right mouse buttons. |
On this newly proposed scheme, changing from disabling either one or two finger events can be achieved this way: I made sure to not modify mouse / keyboard controls API in anyway. I'm just altering touch events in both PRs suggestions. |
We have to fix that, too. It does not appear you are handling With the following changes to your proposed code, In // LEFT, MIDDLE, RIGHT will be deprecated in favor of specifying "actions"
export var MOUSE = { LEFT: 0, MIDDLE: 1, RIGHT: 2, ROTATE: 0, DOLLY: 1, PAN: 2 };In // Mouse button actions
this.mouseButtons = { LEFT: THREE.MOUSE.ROTATE, MIDDLE: THREE.MOUSE.DOLLY, RIGHT: THREE.MOUSE.PAN };
//
function onMouseDown( event ) {
if ( scope.enabled === false ) return;
// Prevent the browser from scrolling.
event.preventDefault();
// Manually set the focus since calling preventDefault above
// prevents the browser from setting it automatically.
scope.domElement.focus ? scope.domElement.focus() : window.focus();
switch ( event.button ) {
case 0:
switch ( scope.mouseButtons.LEFT ) {
case THREE.MOUSE.ROTATE:
if ( event.ctrlKey || event.metaKey || event.shiftKey ) {
if ( scope.enablePan === false ) return;
handleMouseDownPan( event );
state = STATE.PAN;
} else {
if ( scope.enableRotate === false ) return;
handleMouseDownRotate( event );
state = STATE.ROTATE;
}
break;
case THREE.MOUSE.PAN:
if ( scope.enablePan === false ) return;
handleMouseDownPan( event );
state = STATE.PAN;
break;
default:
state = STATE.NONE;
}
break;
case 1:
switch ( scope.mouseButtons.MIDDLE ) {
case THREE.MOUSE.DOLLY:
if ( scope.enableZoom === false ) return;
handleMouseDownDolly( event );
state = STATE.DOLLY;
break;
default:
state = STATE.NONE;
}
break;
case 2:
switch ( scope.mouseButtons.RIGHT ) {
case THREE.MOUSE.ROTATE:
if ( scope.enableRotate === false ) return;
handleMouseDownRotate( event );
state = STATE.ROTATE;
break;
case THREE.MOUSE.PAN:
if ( scope.enablePan === false ) return;
handleMouseDownPan( event );
state = STATE.PAN;
break;
default:
state = STATE.NONE;
}
break;
}
if ( state !== STATE.NONE ) {
document.addEventListener( 'mousemove', onMouseMove, false );
document.addEventListener( 'mouseup', onMouseUp, false );
scope.dispatchEvent( startEvent );
}
} |
|
I really like this, gonna setup a proper PR with these changes. I was gonna suggest that we changed mouse constants to match touch pattern 👍 |
|
If we carry this to its natural completion, controls.mouseButtons.LEFT = THREE.MOUSE.PAN;
controls.mouseButtons.RIGHT = THREE.MOUSE.ROTATE;
controls.touches.ONE = THREE.TOUCH.PAN;
controls.touches.TWO = THREE.TOUCH.DOLLY_ROTATE; |
|
You will have to handle Please do your best to make sure the behavior is not changed. This is a refactoring only. Consider defining |
3 participants
It's becoming fairly common for me to need to modify the hardcoded number of touches for rotate and dollypan. I would imagine others could benefit from having control over that as well.
The only problem with this approach would be when users incorrectly pick a single touch for the dollypan. Perhaps a warning would suffice?