Skip to content
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.

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

OrbitControls: zoomToCursor weird behavior after certain rotations. #27110

Open
alessiocancian opened this issue Nov 3, 2023 · 8 comments
Open
Labels

Comments

@alessiocancian
Copy link

Description

I found this using zoomToCursor enabled with screenSpacePanning set to false, after certain rotations the zoom to cursor become inconsistent and sometimes goes back to normal after rotating again.
I was able to reproduce this in the map controls example too.

Reproduction steps

  1. go to map example
  2. enable zoomToCursor
  3. change rotation to go the lowest possible in the city (there may be other ways but this make it happen consistently)
  4. try to zoom

Code

https://threejs.org/examples/#misc_controls_map

Live example

https://threejs.org/examples/#misc_controls_map

Screenshots

bug-map-1.mov

Version

r158

Device

Desktop

Browser

Chrome

OS

MacOS

@alessiocancian alessiocancian changed the title zoomToCursor weird behavior after certain rotation zoomToCursor weird behavior after certain rotations Nov 3, 2023
@gkjohnson
Copy link
Collaborator

gkjohnson commented Nov 3, 2023

This is a limitation noted in the original PR for zoomToCursor. Map controls requires that the camera target remain on the horizontal plane which is not possible when zooming into the "sky" above the plane so the camera zooms while keeping the current focus target.

If you have suggestions other approaches feel free to suggest them. Otherwise maybe it should just be noted in the docs that this is the behavior when the camera is below a certain horizon angle with screenSpacePanning disabled.

@alessiocancian
Copy link
Author

alessiocancian commented Nov 3, 2023

This is a limitation noted in the original PR for zoomToCursor. Map controls requires that the camera target remain on the horizontal plane which is not possible when zooming into the "sky" above the plane so the camera zooms while keeping the current focus target.

If you have suggestions other approaches feel free to suggest them. Otherwise maybe it should just be noted in the docs that this is the behavior when the camera is below a certain horizon angle with screenSpacePanning disabled.

Sorry didn't catch that, it is definitely worth to be noted in the docs and maybe allow to set a custom threshold.

I tried removing the if to see what happens and seems only a problem of zoom speed... I'm not sure how to implement it but maybe applying some sort of logarithmic function based on the angle may help to keep the speed consistent?
And when angle is really 0 (or in a really small range) it could be approximated to a small value just to make it work more likely to the zoom to cursor.
Otherwise the raycasted point could be used just to understand the direction to then apply a "consistent" distance from the previous target.
I don't know if any of these make sense, I'm not really familiar with three.js...

@Mugen87
Copy link
Collaborator

Mugen87 commented Nov 6, 2023

it is definitely worth to be noted in the docs

@alessiocancian Agreed! Would you be willing to improve the description of zoomToCursor in the docs and write a small comment about the mentioned restriction?

@Mugen87 Mugen87 added the Addons label Nov 6, 2023
@Mugen87 Mugen87 changed the title zoomToCursor weird behavior after certain rotations OrbitControls: zoomToCursor weird behavior after certain rotations. Nov 6, 2023
@alessiocancian
Copy link
Author

it is definitely worth to be noted in the docs

@alessiocancian Agreed! Would you be willing to improve the description of zoomToCursor in the docs and write a small comment about the mentioned restriction?

Sure, can I also add a property to set a custom threshold? In my case I found that even with a 5 degrees limit it is still usable.
I don't know how to call it, maybe zoomToCursorThreshold?
Should we add a "helper prop" to automatically set the polar angle based on the threshold?

I'm also looking at #24983 to find a fix but I find it hard to compare that to the merged implementation.

@gkjohnson
Copy link
Collaborator

In my case I found that even with a 5 degrees limit it is still usable. I don't know how to call it, maybe zoomToCursorThreshold?

How are you testing this? When I change or remove the threshold I see the example very quickly break at these steep angles especially when trying to zoom into the sky or the ground close to the camera. The camera quickly gets into a position where it's unusable.

I'm also looking at #24983 to find a fix but I find it hard to compare that to the merged implementation.

This implementation is unrelated and didn't work as one would expect a "zoom to cursor" map feature to behave. This is likely why it wasn't merged.

If time is going to be spent on this I'd prefer to see some investigation into how other map control systems behave at these types of angles if any. Or a proper solution to the problem rather than the current bandaid.

@alessiocancian
Copy link
Author

In my case I found that even with a 5 degrees limit it is still usable. I don't know how to call it, maybe zoomToCursorThreshold?

How are you testing this? When I change or remove the threshold I see the example very quickly break at these steep angles especially when trying to zoom into the sky or the ground close to the camera. The camera quickly gets into a position where it's unusable.

I'm testing it on my real scenario, when zooming into the sky that may get too fast at 5 degrees but since I hope users actually zoom to the model and there's a button to reset the camera, that's less important than the zoom behavior for me.

@gkjohnson
Copy link
Collaborator

when zooming into the sky that may get too fast at 5 degrees but since I hope users actually zoom to the model and there's a button to reset the camera, that's less important than the zoom behavior for me.

So it doesn't work for your case either, then... I can support investigating better approaches to handling zoom at these steep angles but I don't support adding a field that codifies behavior we know breaks the camera position.

If you're really happy with the steep camera behavior you can make the changes in a local copy of the OrbitControls file. But I'd prefer to see this thought about more critically instead of adding quick fixes that enable known poor functionality.

@alessiocancian
Copy link
Author

alessiocancian commented Nov 14, 2023

I ended up implementing my own OrbitControls with some different features (like zoom limited so it can't pass through the mesh).

I'm not sure if that's what breaks the zoom into the sky under low angles but I found out that in those cases the intersectPlane returns null but since OrbitControls code uses only the target parameter to get the result that isn't catched and it keeps the old value.
I don't keep the target vector updated so that doesn't cause any issue on my implementation.
When zooming I only move the camera towards the cursor point using the pointer direction normal multiplied by deltaY of the wheel event (so it takes into account wheel speed) and a constant value.

This is a simplyfied implementation (without mesh limits):

zoomRatio = 0.02;

onMouseWheel = (e: WheelEvent) => {
	e.preventDefault();

	const dollyRatio = -e.deltaY;
	this.camera.position.addScaledVector(
		this.calcDollyDirection(e),
		this.zoomRatio * dollyRatio
	);
};

// this does the same of updateMouseParameters of OrbitControls
calcDollyDirection = (e: PointerEvent | WheelEvent) => {
	const mouse = this.calcPointerRelativePosition(e);
	const dollyDirection = new Vector3()
		.set(mouse.x, mouse.y, 1)
		.unproject(this.camera)
		.sub(this.camera.position)
		.normalize();
	return dollyDirection;
}

calcPointerRelativePosition = (e: PointerEvent | WheelEvent) => {
	const rect = this.domElement.getBoundingClientRect();
	const x = e.clientX - rect.left;
	const y = e.clientY - rect.top;
	const w = rect.width;
	const h = rect.height;
	const xRay = (x / w) * 2 - 1;
	const yRay = -(y / h) * 2 + 1;
	return new Vector2(xRay, yRay);
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants