three-stdlib icon indicating copy to clipboard operation
three-stdlib copied to clipboard

OrbitControls saveState differ from Three's

Open partynikko opened this issue 3 years ago • 6 comments

  • three version: r127
  • three-stdlib version: v1.2.4

Problem description:

There is an inconsistency regarding the OrbitControls method saveState and how it behaves. In Three, the camera.zoom is saved while saving the state of the controls, regardless if the camera is a perspective one or orthographic: https://github.com/mrdoob/three.js/blob/3d59a07b32394c1a0d57c8558775d0ba3a54d7da/examples/jsm/controls/OrbitControls.js#L123-L129

In this library this is not the case any longer (it used to be but was changed in 938045a). Now it is only saved for perspective cameras.

Due to the camera zoom of orthographic cameras no longer being saved, resetting the OrbitControls with the reset method will not properly reset the camera to previous state.

Relevant code:

https://github.com/pmndrs/three-stdlib/blob/cc079be1d2bb5f619202341a2fa53a4cb006056c/src/controls/OrbitControls.ts#L112-L116

Suggested solution:

If this was not done intentionally due to some weird behavior I would suggest to just revert that specific line, making it possible to properly save the state of orthographic cameras.

If it was done intentionally, it would be nice with a comment explaining why 👍

partynikko avatar Apr 22 '21 11:04 partynikko

I'd probably just revert it. Maybe someone thought there was no zoom on OrthoGraphic Camera?

joshuaellis avatar Apr 22 '21 11:04 joshuaellis

I'd probably just revert it. Maybe someone thought there was no zoom on OrthoGraphic Camera?

Yeah, could be that it was mixed up with the OrbitControls properties maxZoom/minZoom which only applies to orthographic cameras, and maxDistance/minDistance for perspective cameras. Both camera types do have zoom though.

partynikko avatar Apr 22 '21 11:04 partynikko

Friendly ping :) it's still impossible to reset zoom level when using OrthographicCamera

smashercosmo avatar Oct 12 '21 14:10 smashercosmo

@smashercosmo you're welcome to submit a PR

joshuaellis avatar Oct 12 '21 14:10 joshuaellis

Can I just copy three.js implementation as is?

smashercosmo avatar Oct 12 '21 18:10 smashercosmo

No, there's been changes in this repo that are not in the threejs repo.

joshuaellis avatar Oct 12 '21 18:10 joshuaellis

:tada: This issue has been resolved in version 2.23.1 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

github-actions[bot] avatar May 26 '23 21:05 github-actions[bot]