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

Add additional parameter to orbitControl to enable/disable inverted vertical rotation #2962

Closed
2 of 14 tasks
dekmm opened this issue May 31, 2018 · 8 comments
Closed
2 of 14 tasks
Assignees

Comments

@dekmm
Copy link
Contributor

dekmm commented May 31, 2018

Nature of issue?

  • Found a bug
  • Existing feature enhancement
  • New feature request

Most appropriate sub-area of p5.js?

  • Color
  • Core/Environment/Rendering
  • Data
  • Events
  • Image
  • IO
  • Math
  • Typography
  • Utilities
  • WebGL
  • Other (specify if possible)

Feature enhancement details:

As mentioned in PR #2956 I was thinking of adding an additional parameter to orbitControl to enable/disable inverted vertical rotation. The linked PR currently changes the inverted rotation in orbitControl, that was supposedly incorrect, to normal rotation. Knowing some users actually prefer inverted vertical rotation, adding a flag to control this would be helpful.

@dekmm dekmm changed the title Add additional parameter to orbitControl to enable/disable vertical rotation Add additional parameter to orbitControl to enable/disable inverted vertical rotation May 31, 2018
@Spongman
Copy link
Contributor

how about adding two parameters scaleX and scaleY with default value 1? making either of these negative would change the sense of the rotation, and adjusting the magnitude would change the sensitivity.

@dekmm
Copy link
Contributor Author

dekmm commented May 31, 2018

I like the approach. Would 1 be considered the maximum allowed sensitivity so it ranges from -1 to 1 or is it just an agreed upon starting point?

@kjhollen
Copy link
Member

Thanks @dekmm! @AidanNelson is one of our Google Summer of Code students for p5.js and part of his project is improving the camera API and interactions for Web GL mode. Is it ok with you if I assign this issue to him?

@Spongman
Copy link
Contributor

I don't see the need for a setting a maximum. If someone wants to make it more sensitive than normal, it should probably allow that.

@dekmm
Copy link
Contributor Author

dekmm commented May 31, 2018

@kjhollen go ahead!

@AidanNelson
Copy link
Member

AidanNelson commented Jun 1, 2018

Two questions:

  1. It looks like orbitControl() expects angleMode to be RADIANS, but doesn't explicitly set angleMode in the function. I can add this check, but am wondering if I should file it as a separate issue for organizational purposes?
  2. If the user specifies a scaleX and scaleY, does it make sense for those values to override the width-dependent scaling or modify it? I'm inclined to think that overriding those values adds more flexibility for the user (who can always pass width/2 * scaleFactor in as the scale factor). That said, I think there should be some scaling such that users can pass in values from 1-10, so maybe that scaling should just be width/2. Thoughts?

@Spongman
Copy link
Contributor

Spongman commented Jun 2, 2018

  1. you can use _fromRadians to convert from radians to angleMode, if necessary.
  2. i'd recommend coming up with some sensible default scale factor that you think makes sense, and then just multiply that by any scale factors that are passed in.

@kjhollen
Copy link
Member

closing because the fix for this has been merged into webgl-gsoc-2018!

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

No branches or pull requests

4 participants