WIP: OrbitControls: added support for zoom to cursor

[WestLangley]

As suggested in #16961 (comment).

I think this should be the default for MapControls. I'm not sure if it makes as much sense for OrbitControls, but it can be toggled on.

This is a WIP. Feedback welcome.

<EDIT - live link removed>

[Mugen87]

Hint: Consider to use draft PRs if your code is really WIP.

[sciecode]

The behavior looks great 👍
Just pointing out some places where a division by zero is possible and you didn't clearly state with comments.

[EliasHasle]

The desired behavior is confirmed with touchpad pinch-to-zoom on Chrome for Windows. Does it work on touch screens too?

I think I agree that disabling panning should also disable this feature. Also that zoom to (through?) cursor should be default for MapControls and not for OrbitControls.

[looeee]

The updated example looks great 👍

[Mugen87]

@WestLangley This change is great! But what about the concerns of @sciecode here #17145 (comment)?

Apart from that, I think it's okay to remove the WIP label and updating the docs/TS instead. Would be nice to have this feature in context of MapControls.

[WestLangley]

I now have a version that supports touch, too. I'll get back to this soon...

[Peque]

@WestLangley Friendly ping. 😇

I too think this would be a great default for MapControls.

[duhaime]

@Peque check out the PR, he exports MapControls and OrbitControls!

[Peque]

@duhaime Not sure what you mean... 😅 😊

[duhaime]

Haha, no worries, I just meant that this PR includes a zoom to cursor implementation for the Map controls. The OrbitControls here exports both OrbitControls and MapControls objects.

Did you see @WestLangley's MapControls demo with zoom to cursor? That might be the easiest way to see the zoom to cursor in action...

[Peque]

@duhaime Thanks. With my comment I just wanted to upvote this MR and ask @WestLangley if they were planning to work on this to get it merged. 😊

[Peque]

In particular, I wanted to upvote for MapControls to zoom to cursor by default, which I think is what most people would expect nowadays (matching OpenStreetMaps, Google Maps, Bing Maps...).

[Peque]

@WestLangley I tried your code here and found one issue, in case you are still interested in this. 😇

The problem occurs when using an OrtographicCamera and setting controls.maxPolarAngle = Math.PI / 2;. Then, if you go to a completely horizontal view (i.e.: polar angle is 90º) and zoom (out or it, it doesn't matter), it breaks. With break I mean you are unable to see the texture again.

[sciecode]

@Peque there are comments informing of this limitation in the commit:

this.maxPolarAngle = Math.PI / 3; // must be less than pi/2 when zoomToCursor is true

[Peque]

@sciecode Do you know how to work-around that limitation?

[sciecode]

@sciecode Do you know how to work-around that limitation?

From a design point of view there is no work-around, because we keep camera.up fixed and that's invaluable to many of other features, so we can't change that.

From a project stand point, you could simply make it so the "map" is not oriented facing up ( normal pointing to +Y ). Change the orientation to something else, for example... facing +Z.
This way, you avoid this limitation by working-around it and not against it.

[Peque]

@sciecode Thanks for your feedback! 😊

I have an application with the map facing +Z, but I seem to find the same issue. 😕 I do set camera.up.set(0, 0, 1); though.

[looeee]

@Peque there are comments informing of this limitation in the commit:

this.maxPolarAngle = Math.PI / 3; // must be less than pi/2 when zoomToCursor is true|

We should note this in the docs as well.

[molzer]

Thank you @WestLangley for this!

[promontis]

@WestLangley Is this something that is ready to be merged?

[JHB101]

I for one am confused. Has this been added? Is it stable now?
The issue is still open and I don't remember the recent r125 release mentioning it being added.

[Mugen87]

When a PR is open then it is not yet merged. And that means the code is not yet added to the repo.

[JHB101]

Well, I am forced to use your code by my current client because his people think its great. Sitting on your hands whilst a PR is open with just has a cryptic message at the bottom is not my idea of fun. So get snotty with me all you like, but you know you guys are starting to drop the ball on this, and then I am going to have to migrate this company back to the real thing. I would love to say I have never seen so much confusion in a code base, but unfortunately I have, so waddle on.

[molzer]

I'm using this PR's code without problems since a few months even though it hasn't been merged yet – just do it this way?

[JHB101]

@molzer

I am using it as is, and then my client reads somewhere (not here) that you can now zoom to cursor. Now he wants to know from me if I used ThreeJS at all, because he tested and his website does not zoom to cursor. He substantiates his claims by sighting some blog post that reports you must set some keyword to true. Little does he know that the keyword has already been removed again by the very people that are supposed to guard against such behavior.

The control js files in all their flavors, are not supposed to be part of the code base (its in examples), but it is treated as part of it in here, or is it part of it? I simply cannot tell anymore. It might help if the examples are moved to their own repo. Also there has been so many settings/keywords used in it that has been added and then removed again that I have lost count. These were added/removed at the whim of every person coming in here with a new suggestion/complaint. These keywords then start losing their meaning, because the word that describes the effect has been used and removed already.

My suggestion (for what it is worth) is make only one controls.js and make it part of the core. Then use settings/keywords to change it into map-/orbit-/camera-/whatever-/controls. Now using js (pseudo) pointers at the start of the file will send the request to the correct function (i.e. there will be no trackballcontrolls or orbitcontrols, but you could have either based on a setting like "Don't orbit all the way around the y axis or yAxisOrbitLimit=90"). If I remember correctly that is the difference between the two.

At the moment you are trying to mimic google maps in one file and a plane in another. This can all be one file. I saw in r120 someone made a cameracontrols.js in its own sub folder and I was hoping it was to merge all this, but here we go again with zoom to mouse and we all sit in limbo waiting to see if our next update is going to break this again.

[timrutter]

is there any chance this can be merged? It pretty basic and commonly expected behaviour.

that said I've just tried using it locally and i have issues because my camera always looks along Z. The camera's position ends up as NaN. Any ideas why this might be?

[ammanvedi]

would be great to merge !

[metalim]

Hey. So, what's preventing this from being merged? Should we just create a new PR?

[WestLangley]

Updated to the latest release... Support for touch in progress...

[WestLangley]

I have come to the conclusion that this approach is not going to work.

(1) It only works when damping is enabled. That is not acceptable in my view.

(2) Touch is not supported. (My mobile devices do not support pointer events, so I am no longer able to test the code I wrote previously.)

To prevent further delay, I suggest someone else give this a try.

[PSausM]

It's best to not use orbit controls: Use camera controls
https://github.com/yomotsu/camera-controls
It's implemented there and they have a nice "fitToScreen" function as well.

[Akbar1909]

@WestLangley I used https://raw.githack.com/westlangley/three.js/dev-zoom_to_cursor/examples/misc_controls_map.html source code. But It didn't work