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

OrbitControls effective rotate speed changes based on screen aspect ratio #13956

Closed
2 tasks done
dustinkerstein opened this issue Apr 30, 2018 · 4 comments
Closed
2 tasks done

Comments

@dustinkerstein
Copy link

This is more of a design decision, but I figured I'd add it as a bug for discussion's sake since it was causing some issues for my implementation.

The rotate functions currently take into consideration the screen size through the use of element.clientHeight/Width:

// rotating across whole screen goes 360 degrees around
rotateLeft( 2 * Math.PI * rotateDelta.x / element.clientWidth );

// rotating up and down along whole screen attempts to go 360, but limited to 180
rotateUp( 2 * Math.PI * rotateDelta.y / element.clientHeight );

But this means that on non-square screens, the effective speed is different in the Up/Down vs. Left/Right directions. Ie. Try drawing a circle when your screen is a very wide/tall rectangle. You'll notice that you're effectively drawing an oval. To me it feels much more natural without the element.clientHeight/Width in the above equations (with the necessary tweaks to rotationSpeed). It seems like the comments are the explanation, but this results in awkward behavior for wide/tall screens. Is there some other benefit of having the screen size in this equation that I'm not seeing?

Three.js version
  • [X ] r92
Browser
  • All of them
OS
  • All of them
@WestLangley
Copy link
Collaborator

Good point.

In handleMouseMoveRotate() and handleTouchMoveRotate() we could remove the comments and try this, instead:

rotateLeft( 2 * Math.PI * rotateDelta.x / element.clientHeight );

rotateUp( 2 * Math.PI * rotateDelta.y / element.clientHeight );

@dustinkerstein
Copy link
Author

That helps with the aspect ratio, but I still find it odd to scale the speed with the screen size.

@WestLangley
Copy link
Collaborator

The factor must be unit-less.

@dustinkerstein
Copy link
Author

Ah, yeah. I just played around with it some more and that makes sense.

dustinkerstein added a commit to dustinkerstein/panolens.js that referenced this issue Apr 18, 2020
pchen66 added a commit to pchen66/panolens.js that referenced this issue Apr 20, 2020
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

No branches or pull requests

2 participants