fix: reset focal offsets on input start and setLookAt#507
fix: reset focal offsets on input start and setLookAt#507CodyJasonBennett wants to merge 2 commits intoyomotsu:devfrom
Conversation
|
Sorry for the delay... |
|
No worries. These are sweeping changes, and people can work around it if they really want to today. |
| if ( ! this._domElement ) return; | ||
| if ( ! this._enabled || this.mouseButtons.wheel === ACTION.NONE ) return; | ||
|
|
||
| this._resetFocalOffsets(); |
There was a problem hiding this comment.
What about moving this line to the beginning of _zoomInternal() and _dollyInternal() with if ( this.dollyToCursor )?
Then wheel zoom/dolly will reset focalOffsets.
eg
protected _dollyInternal = ( delta: number, x: number, y : number ): void => {
if ( this.dollyToCursor ) this._resetFocalOffsets();|
|
||
| if ( ! this._enabled ) return; | ||
|
|
||
| this._resetFocalOffsets(); |
There was a problem hiding this comment.
I think mouse drag movement does not affect focalOffset. so, can we remove this?
There was a problem hiding this comment.
The idea is to call this whenever any input starts. Otherwise, we may interrupt transitions or animations. Is there a better place to put this?
| // Applies and resets focal offsets to play nice with lookAt and zoom-to-cursor | ||
| // https://github.com/yomotsu/camera-controls/issues/491 | ||
| _resetFocalOffsets() { | ||
|
|
There was a problem hiding this comment.
Could we check the focalOffset value first?
If the value is 0, 0, 0, let's just return and do nothing.
Co-authored-by: Akihiro Oyamada <admin@yomotsu.net>
|
Thank you very much for fixing this issue. It indeed resolves #424, and I look forward to it being merged soon. |
9f1d761 to
38c26ca
Compare
|
I am experiencing the same problems that this pull request could fix? |
Fixes #303
Fixes #316
Fixes #424
Fixes #491
Applies and zeroes focal offsets into the control's spherical coordinates so features like zoom-to-cursor (e.g.
setOrbitPointand then interacting with anOrthographicCamera) andsetLookAtwork without interruption. I have not testedsetLookAtas thoroughly, but expanded the fix to it after reading #303.