Skip to content

fix viewer rotating on any mouse movement #48

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

izeng0975
Copy link

on firefox, regardless of whether mouse left click is pressed, the viewer would follow the mouse's movement

on firefox, regardless of whether mouse left click is pressed, the viewer would follow the mouse's movement
@willeastcott willeastcott requested review from kpal81xd and Copilot July 23, 2025 09:34
@willeastcott willeastcott added the bug Something isn't working label Jul 23, 2025
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a bug where the 3D viewer was incorrectly rotating on any mouse movement in Firefox, regardless of whether the left mouse button was pressed. The fix ensures that rotation only occurs when the mouse button is actually being held down.

  • Fixed mouse rotation logic to check for mouse button state before applying rotation
  • Added proper mouse button state validation in the rotation calculation

@@ -169,7 +169,7 @@ class AppController {
// desktop rotate
v.set(0, 0, 0);
const mouseRotate = new Vec3(mouse[0], mouse[1], 0);
v.add(mouseRotate.mulScalar((1 - this._mouse[2]) * this.orbitSpeed * dt));
v.add(mouseRotate.mulScalar(this._mouse[0] * (1 - this._mouse[2]) * this.orbitSpeed * dt));
Copy link
Preview

Copilot AI Jul 23, 2025

Choose a reason for hiding this comment

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

The fix incorrectly multiplies by this._mouse[0] (X position) instead of checking mouse button state. This will scale rotation by the X coordinate rather than enabling/disabling it based on button press. Consider using a boolean mouse button state or the appropriate mouse button index.

Suggested change
v.add(mouseRotate.mulScalar(this._mouse[0] * (1 - this._mouse[2]) * this.orbitSpeed * dt));
if (this._mouseButtonPressed) { // Check if the mouse button is pressed
v.add(mouseRotate.mulScalar((1 - this._mouse[2]) * this.orbitSpeed * dt));
}

Copilot uses AI. Check for mistakes.

@kpal81xd
Copy link
Contributor

@izeng0975 when you say regardless if mouse left click is pressed what do you mean? Any of the mouse buttons to allow for rotation or pointer lock? Im not quite sure what you are trying to achieve here

@izeng0975
Copy link
Author

@izeng0975 when you say regardless if mouse left click is pressed what do you mean? Any of the mouse buttons to allow for rotation or pointer lock? Im not quite sure what you are trying to achieve here

sorry for the bad wording, its hard to explain with words so I made two YouTube videos to show the difference before and after. This is the bug (specifically on Firefox doesn't happen in Chrome, Safari, or Brave) https://youtu.be/9MWu299un2k usually while holding left click, you can move your mouse around and the camera will follow. However, on Firefox, even after releasing left click, the camera seems to follow the mouse cursor and movement. The fix I implemented shown here https://www.youtube.com/watch?v=xNC6qkkymJE shows the results of changing the line of code to make the behavior proper for Firefox again

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants