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.

Sign up for GitHub

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

Cleanup and improvement for fromVectors() #491

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

Cleanup and improvement for fromVectors() #491

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 44 additions & 28 deletions src/math/Quaternion.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,28 +108,50 @@ Quaternion.prototype.toAxisAngle = function(targetAxis){
return [targetAxis,angle];
};

var sfv_t1 = new Vec3(),
sfv_t2 = new Vec3();

/**
* Set the quaternion value given two vectors. The resulting rotation will be the needed rotation to rotate u to v.
* @method setFromVectors
* @param {Vec3} u
* @param {Vec3} v
* @author Robert Eisele, https://github.com/rawify
*/
Quaternion.prototype.setFromVectors = function(u,v){
if(u.isAntiparallelTo(v)){
var t1 = sfv_t1;
var t2 = sfv_t2;

u.tangents(t1,t2);
this.setFromAxisAngle(t1,Math.PI);
// Implements https://raw.org/proof/quaternion-from-two-vectors/

var ux = u.x;
var uy = u.y;
var uz = u.z;

var vx = v.x;
var vy = v.y;
var vz = v.z;

var uLen = u.norm();
var vLen = v.norm();

if (uLen > 0) ux /= uLen, uy /= uLen, uz /= uLen;
if (vLen > 0) vx /= vLen, vy /= vLen, vz /= vLen;

var dot = ux * vx + uy * vy + uz * vz;

// Parallel check
if (dot > 0.999999) {
this.x = 0;
this.y = 0;
this.z = 0;
this.w = 1;
} else

// 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);
Copy link

Choose a reason for hiding this comment

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

Why provoke GC? Previous solution is GC free.

Copy link
Author

Choose a reason for hiding this comment

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

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

Copy link

Choose a reason for hiding this comment

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

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).

Copy link
Author

Choose a reason for hiding this comment

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

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?

} else {
var a = u.cross(v);
this.x = a.x;
this.y = a.y;
this.z = a.z;
this.w = Math.sqrt(Math.pow(u.norm(),2) * Math.pow(v.norm(),2)) + u.dot(v);
this.x = uy * vz - uz * vy;
this.y = uz * vx - ux * vz;
this.z = ux * vy - uy * vx;
this.w = 1 + dot;
this.normalize();
}
return this;
Expand All @@ -142,9 +164,6 @@ Quaternion.prototype.setFromVectors = function(u,v){
* @param {Quaternion} target Optional.
* @return {Quaternion}
*/
var Quaternion_mult_va = new Vec3();
var Quaternion_mult_vb = new Vec3();
var Quaternion_mult_vaxvb = new Vec3();
Quaternion.prototype.mult = function(q,target){
target = target || new Quaternion();

Expand Down Expand Up @@ -220,22 +239,19 @@ Quaternion.prototype.normalize = function(){
/**
* Approximation of quaternion normalization. Works best when quat is already almost-normalized.
* @method normalizeFast
* @see http://jsperf.com/fast-quaternion-normalization
* @author unphased, https://github.com/unphased
*/
Quaternion.prototype.normalizeFast = function () {
var f = (3.0-(this.x*this.x+this.y*this.y+this.z*this.z+this.w*this.w))/2.0;
if ( f === 0 ) {
this.x = 0;
this.y = 0;
this.z = 0;
this.w = 0;
} else {
this.x *= f;
this.y *= f;
this.z *= f;
this.w *= f;
}

// Makes use of two steps of a Taylor expansion of 1/sqrt(x) ~ 1 - (x - 1) / 2 = (3 - x) / 2

var f = (3 - (this.x*this.x+this.y*this.y+this.z*this.z+this.w*this.w)) / 2;

this.x *= f;
this.y *= f;
this.z *= f;
this.w *= f;

return this;
};

Expand Down