Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed Camera rotates slower with devtools open #95

Merged
merged 3 commits into from
Jan 19, 2023
Merged

Fixed Camera rotates slower with devtools open #95

merged 3 commits into from
Jan 19, 2023

Conversation

0-zen
Copy link
Contributor

@0-zen 0-zen commented Jan 19, 2023

Nature of the bug: The devtools augmented significantly the number of calls to onMouseMove() per second (~50 -> ~450), which led to mouse deltas to get smaller. My guess is that it made the update of the mouse position in the model throttle.

Here's what I did:

  • Edited mouse.ts to add a limit to how many calls per second (max 25/s)

  • Ran npm audit fix to update some of the dependencies

@LucasDower
Copy link
Owner

Wow, yeah you're right, onMouseMove() is being called multiple times before getMouseDelta() is read. It looks like when devtools is closed all mouse events are coalesced into one event but when it is open then each event is emitted separately.

I think a better fix would be simply to remove line this.prevMouse = this.currMouse; from onMouseMove(), that way the previous mouse position will stay as the last position a delta was calculated from.

If you wanna update the PR with the change above I'll approve and get this merged. Thanks for pointing out what was wrong, I've taken a stab at this issue a few times and it stumped me so massive thanks.

Copy link
Owner

@LucasDower LucasDower left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See prev comment.

@0-zen
Copy link
Contributor Author

0-zen commented Jan 19, 2023

No problem 👍 this repo is useful so I might continue to either tackle issues or make improvements.
Was a pain to get git to understand where I wanted to push but here you go, the changes are done :)

@LucasDower LucasDower self-requested a review January 19, 2023 21:45
@LucasDower LucasDower merged commit 4c0d856 into LucasDower:main Jan 19, 2023
@LucasDower
Copy link
Owner

LucasDower commented Jan 19, 2023

Fixes #75

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants