Fix Orthographic camera zoomSpeed calculation

[heycharlieabbott]

Fixes #1679

Apply orthographic zoomSpeed in normalizedDelta calculation to prevent flipped/runaway zoom values

[gkjohnson]

Hey! Charlie Abbott! How's it going -

I just tested this and I agree the current behavior isn't right. I don't think we should embed the scaling factor inside the normalized delta exponent, though. The "zoomSpeed" value is allowed to be negative and should generally be a value that linearly scales the apparent "zoom" change.

I'm thinking this would be better - the "normalizedDelta" value is instead scaled by the zoomSpeed magnitude. And the zoom direction is also adjusted by the zoomSpeed sign:

const normalizedDelta = Math.pow( 0.95, Math.abs( scale * 0.05 ) ) * Math.abs( zoomSpeed );
let scaleFactor = scale * zoomSpeed > 0 ? 1 / Math.abs( normalizedDelta ) : normalizedDelta;

[heycharlieabbott]

Hey hey! Going well, hope all's well with you too!

Thanks for checking and for the feedback! I've just tested it, and unless I've missed something, it looks like setting the zoomSpeed to 0 from outside of the normalized delta exponent sets the scaleFactor to 0, then when the camera zoom is multiplied by the scaleFactor, the orthographic camera zoom is by default set to 0 and the scene becomes invisible unless the minZoom is set to a positive number.

Also, unless I've misunderstood something with the above change it looks like setting the zoomSpeed to somewhere between 0 and 1 or 0 and -1 can cause an overall increase in the zoomSpeed, which feels a bit counterintuitive to me.

If we keep the zoomSpeed in the normalized delta exponent, when set to 0 normalizedDelta will be evaluated to 1, which leads to the scaleFactor being unchanged and the orthographic camera being unable to zoom, also any number between 0 and 1 or 0 and -1 will slow the camera zoomSpeed and any number larger than 1 or -1 will increase the zoomSpeed which feels a bit more intuitive to me.

Flipping the zoom direction by zoomSpeed sign totally makes sense to me though! What do you think about the following?

	const normalizedDelta = Math.pow( 0.95, Math.abs( scale * 0.05 ) * Math.abs( zoomSpeed ) );
	let scaleFactor = scale * zoomSpeed > 0 ? 1 / Math.abs( normalizedDelta ) : normalizedDelta;

In local testing, considering a zoomSpeed of 1 as default, setting as 0.5 did appear to be about half speed, setting as 2 appeared about double speed, 0 was stopped, and negative flipped the zoom direction.

[gkjohnson]

You're right - I was mixing this "normalizedDelta" up with the scroll wheel value normalization elsewhere.

Your change looks good but it occurs to me at this can be simplified to the following so the exponent sign just implicitly handles the fraction inversion:

let scaleFactor = Math.pow( 0.95, - zoomSpeed * scale * 0.05 );

We'll also need to adjust the GlobeControls case here, as well:

3DTilesRendererJS/src/three/renderer/controls/GlobeControls.js

Lines 680 to 682 in 3dc8b0a

	const scale = this.zoomDelta;
	const normalizedDelta = Math.pow( 0.95, Math.abs( scale * 0.05 ) );
	const scaleFactor = scale > 0 ? 1 / Math.abs( normalizedDelta ) : normalizedDelta;

[heycharlieabbott]

oh nice! I've updated EnvironmentControls to the simplified version and updated GlobeControls too. Thank you!

[gkjohnson]

Great, thanks!