Cleanup and improvement for fromVectors()#491
Cleanup and improvement for fromVectors()#491infusion wants to merge 1 commit intoschteppe:masterfrom
Conversation
|
Is the project still alive? |
|
|
||
| // Close to PI - antiparallel check | ||
| if (dot < -0.999999) { | ||
| this.setFromAxisAngle(Math.abs(ux) > Math.abs(uz) ? new Vec3(-uy, ux, 0) : new Vec3(0, -uz, uy), Math.PI); |
There was a problem hiding this comment.
Why provoke GC? Previous solution is GC free.
There was a problem hiding this comment.
I think the code is cleaner this way and it only handles the (hopefully) super rare edge case this way. But I mean, we could also have an initialized Vec3 somewhere floating around for such operations
There was a problem hiding this comment.
Yes, rare or not, I'm following 100% GC free principle. This allows me to set breakpoints in Vec/Quat constructors to see where allocations are happening under stress testing, so I can make sure everything is as optimized as it gets (memory-wise). But yea, everyone has different ideas (as usual).
There was a problem hiding this comment.
Yea good way of seeing it for sure. Should I change the commit to be GC free to be merged or don't you see the necessity?
Not exactly this repo, but I'm still using and updating the code for myself. I was actually wondering just some days ago how |
|
It should maybe not be called |
2 participants
Hi Stefan,
thanks for Cannon.js! As I am the author of Quaternion.js ( https://github.com/infusion/Quaternion.js ), I was interested in your implementation and figured, your
fromVectors()method could be improved quite a bit. Besides this I added a note where the calculation fornormalizeFast()comes from and removed the unused vectors used before for multiplication.Hope this is helpful.
Robert