search

asny / three-d

2D/3D renderer - makes it simple to draw stuff across platforms (including web)
MIT License
1.34k stars 110 forks source link

OrbitControl becomes less responsive the more you use it #405

Closed 435vic closed 1 year ago

435vic commented 1 year ago

I've been writing a custom implementation of OrbitControl so that the camera freely rotates when you let go of the mouse before stopping after a bit. For it I've been using the 'camera.rotate_around()' method (which technically is part of the three-d-asset crate, but, for visibility, I opted to post the issue here). However, after testing, it's clear that after moving around for a while it slows down considerably. You can see the effect using this demo.

After some investigation, I pinpointed the bug to the first lines of the rotate_around function (as well as the other one that keeps the up vector fixed). When calculating the vectors to the right and up of the camera, the original camera's up vector is not normalized. This makes it ever so slightly different whenever the function is called, leading to the effect seen on the gist.

Fortunately, this is a very easy fix, and involves changing dir.cross(*self.up()) to dir.cross(self.up().normalize()) in two instances. I'm making a pull request to three-d-asset with the changes in a tiny bit.

435vic commented 1 year ago

To be honest, I'm not sure if the magnitude of the up vector of the camera is used, but this solution normalizes the vector so the original magnitude is lost. If the magnitude is needed, I think it could just be calculated beforehand and then added to the vector at the end.

asny commented 1 year ago

The up vector should always be a unit vector so I'm a bit puzzled why it's necessary, but it's a nice fix anyway 👍

asny commented 1 year ago

Took a closer look, and it's probably because your up vector is not a unit vector when you call Camera::set_view, so normalized the up vector in that method instead in https://github.com/asny/three-d-asset/commit/52d9a06ecacd13033814a27d1efe7bb8a9929871 . That means we only normalize it once on input and ensure that it's always a unit vector, also when used in other places. Is it still working as expected?

435vic commented 1 year ago

Ah, that makes more sense! It works great, thanks!