yomotsu / camera-controls

A camera control for three.js, similar to THREE.OrbitControls yet supports smooth transitions and more features.
https://yomotsu.github.io/camera-controls/
MIT License
2k stars 255 forks source link

setOrbitPoint breaks setLookAt #303

Open jbeard4 opened 2 years ago

jbeard4 commented 2 years ago

Describe the bug

Setting setOrbitPoint seems to change the behavior of setLookAt. I would expect setLookAt to be idempotent, which is to say, when given identical parameters, it should move the camera to the same position and point it at the same target. Calling setOrbitPoint in between calls to setLookAt seems to break this assumption, as subsequent calls to setLookAt with the same parameters move the camera to a new position/target.

What is the best way to think about the relationship between setOrbitPoint and setLookAt? Should the code integrating camera-controls keep track of the last set orbit point, and use this when calculating the values to pass into setLookAt? E.g. use the last value passed to setOrbitPoint as an offset to setLookAt?

Thank you for your time and attention to this.

To Reproduce

Steps to reproduce the behavior:

  1. Go to https://yomotsu.github.io/camera-controls/examples/basic.html
  2. In developer console, execute cameraControls.setLookAt(3,5,2,3,0,-3)
  3. Execute cameraControls.setLookAt(3,5,2,3,0,-3) again, and observe that the camera positions doesn't change.
  4. Execute cameraControls.setOrbitPoint( 0, 0, 0 );
  5. Execute cameraControls.setLookAt(3,5,2,3,0,-3) again, and observe that the camera positions now changes.
  6. Repeat steps 4-5

Code

No response

Live example

No response

Expected behavior

Thank you for your time and attention to this.

Screenshots or Video

Screen Capture_select-area_20220907224516 mp4

Device

Desktop

OS

Linux

Browser

Firefox

yomotsu commented 2 years ago

Thanks for your detailed report, it's really helpful, and reproduced the problem. I will take a look later... when I can have some time.

yomotsu commented 2 years ago

@jbeard4

Seems, setLookAt does not respect FocalOffset which is set by setOrbitPoint. setLookAt only changes the target(center) position and the orbit(camera) coords.

below is what setLookAt() is doing: https://github.com/yomotsu/camera-controls/blob/6556f6e1632b18c6b6b9d0e81cb7e5f05d257279/src/CameraControls.ts#L1793-L1794

but i think setLookAt should reset FocalOffset as well.

In fact the steps below work. (Can you try please just in case?)

  1. Go to https://yomotsu.github.io/camera-controls/examples/basic.html
  2. cameraControls.setLookAt(3,5,2,3,0,-3);
  3. cameraControls.setLookAt(3,5,2,3,0,-3);
  4. cameraControls.setOrbitPoint( 0, 0, 0 );
  5. cameraControls.setFocalOffset( 0, 0, 0); cameraControls.setLookAt(3,5,2,3,0,-3);

The above should be work around so far, and I will add cameraControls.setFocalOffset( 0, 0, 0) in setLookAt. Sounds good?

yomotsu commented 2 years ago

Here is what I tried but unfortunately, FocalOffset cannot be calculated during setLookAt() transition.

    /**
     * Make an orbit with given points.
     * @param positionX
     * @param positionY
     * @param positionZ
     * @param targetX
     * @param targetY
     * @param targetZ
     * @param enableTransition
     * @category Methods
     */
    setLookAt(
        positionX: number, positionY: number, positionZ: number,
        targetX: number, targetY: number, targetZ: number,
        enableTransition: boolean = false,
    ): Promise<void> {

        this._camera.updateMatrix();

        _xColumn.setFromMatrixColumn( this._camera.matrix, 0 );
        _yColumn.setFromMatrixColumn( this._camera.matrix, 1 );
        _zColumn.setFromMatrixColumn( this._camera.matrix, 2 );

        _xColumn.multiplyScalar(   this._focalOffset.x );
        _yColumn.multiplyScalar( - this._focalOffset.y );
        _zColumn.multiplyScalar(   this._focalOffset.z );

        const offset = _v3C.copy( _xColumn ).add( _yColumn ).add( _zColumn );
        const target = _v3A.set( targetX, targetY, targetZ );
        const position = _v3B.set( positionX, positionY, positionZ );

        this._targetEnd.copy( target ).sub( offset );
        // this._targetEnd.copy( target );
        this._sphericalEnd.setFromVector3( position.sub( target ).applyQuaternion( this._yAxisUpSpace ) );
        this.normalizeRotations();

        this._needsUpdate = true;

        if ( ! enableTransition ) {

            this._target.copy( this._targetEnd );
            this._spherical.copy( this._sphericalEnd );

        }

        const resolveImmediately = ! enableTransition ||
            approxEquals( this._target.x, this._targetEnd.x, this.restThreshold ) &&
            approxEquals( this._target.y, this._targetEnd.y, this.restThreshold ) &&
            approxEquals( this._target.z, this._targetEnd.z, this.restThreshold ) &&
            approxEquals( this._spherical.theta, this._sphericalEnd.theta, this.restThreshold ) &&
            approxEquals( this._spherical.phi, this._sphericalEnd.phi, this.restThreshold ) &&
            approxEquals( this._spherical.radius, this._sphericalEnd.radius, this.restThreshold );
        return this._createOnRestPromise( resolveImmediately );

    }

Like, you need to run the bellow twice at least.

https://user-images.githubusercontent.com/212837/190843027-cb7a6757-87ea-47da-b98a-b669fb0fc34d.mp4

Thus, the FocalOffset ( or the OrbitPoint) can't be reserved after setLookAt(), and has to be reset in setLookAt(). ~So, I will add a solution: https://github.com/yomotsu/camera-controls/issues/303#issuecomment-1249553359~

timothyallan commented 2 years ago

I've been running into this issue as well, trying to keep the same orbit point while using setTarget to look at different objects, then re-setting the orbit point. Was wondering why the camera would slowly go crazy, but I watched the focalOffset thanks to this issue, and found it would slowly keep adjusting itself!