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

customizable order for rotations & transformations #93

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mlarcher
Copy link

@mlarcher mlarcher commented Dec 4, 2012

It can come in handy to be able to force the rotation on the Y axis to occur before the one on the X axis, or to apply rotations before applying translations.
This pull request handles those cases, and won't interfere with the default behaviour.

It adds the following options :

rotateOrder: 'xyz'
Its value is a string that can be any combination of x,y and z.
Translations will be applied in that order.

transformOrder: 'trs'
Its value is a string that can be any combination of t, r and s.
t stands for translate, r for rotate and s for scale.
Transformations will be applied in that order.

…teY before rotateX, or apply rotations before applying translations)
@sokra
Copy link
Member

sokra commented Dec 4, 2012

I like the idea to make it more flexible.


  • Please use tabs instead of spaces.
  • I think the switch could be more compact. Though array indexing and the char codes for 'xyz'
  • There is a little error. I'll mark it in the commit
  • It may be good to have also an global option
  • An example (as test case)

Using different orders in steps may cause weird translations between them...

@mlarcher
Copy link
Author

mlarcher commented Dec 4, 2012

I'm sure there could be some optimizations :)
I tried to stay as close as possible to the existing code, a more efficient rewrite would require more drastic changes. Are you ok with this ?
Also, what do you mean by "It should use the transforms of the parent" ? Those arrays are taken from what was there, I just stuck them to variables in order to reuse them without having to repeat myself.
I agree the switch could me more compact too, I'll see to it as soon as I find some time for it.

@gaurav21r
Copy link

+1 IS this going to be merged?

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.

3 participants