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

Added Vector3.applyEuler() and Vector3.applyAxisAngle #2920

Merged
merged 1 commit into from Jan 15, 2013

Conversation

WestLangley
Copy link
Collaborator

We now have all four rotation representations represented in the form of Vector3.apply*();

Caveat: I had trouble getting this to build, as I needed to declare within Vector3.js the following:

THREE.Vector3.__q1 = new THREE.Quaternion();

But due to the build order, it was forward-referencing Quaternion. Not good.., so I declared it in Quaternion.js, instead. I know we don't want to do that. This needs to be fixed, somehow.

I think the solution is to change the build order, but that can be a Catch-22...

I'd appreciate help with this one. :-)

@mrdoob
Copy link
Owner

mrdoob commented Jan 14, 2013

This is difficult. The "best" solution I can think of right now is this:

THREE.Quaternion.__vq1 = new THREE.Quaternion();

But would be great to find an alternative to this pattern eventually.

@bhouston
Copy link
Contributor

Would it be possible to use a JavaScript closure style pattern? I notice
that JavaScript closures are never used in THREE.js, although it does make
the library simplier to read because of this fact.

https://www.google.com/search?q=JavaScript+closure

@bhouston
Copy link
Contributor

Here is one place that uses pre-allocated temporaries in the current THREE.js style:

    setFromCoplanarPoints: function ( a, b, c ) {

        var normal = THREE.Plane.__v1.sub( c, b ).crossSelf(
                     THREE.Plane.__v2.sub( a, b ) ).normalize();

        // Q: should an error be thrown if normal is zero (e.g. degenerate plane)?

        this.setFromNormalAndCoplanarPoint( normal, a );

        return this;

    },

I have rewritten it using a closure:

    setFromCoplanarPoints: function() {

        var v1 = new THREE.Vector3();
        var v2 = new THREE.Vector3();

        return function ( a, b, c ) {

            var normal = v1.sub( c, b ).crossSelf( v2.sub( a, b ) ).normalize();

            // Q: should an error be thrown if normal is zero (e.g. degenerate plane)?

            this.setFromNormalAndCoplanarPoint( normal, a );

            return this;

        };
    }(),

The variables declared in the wrapping function, even though that function is executed right away and never stored, are forever accessible within the body of the internal function. This is a way of declaring function static variables that are private to that internal function -- they are not accessible from any other location. Thus it is also a safer style of coding, but it is a more complex style of JavaScript.

@mrdoob
Copy link
Owner

mrdoob commented Jan 14, 2013

I was aware of the pattern but didn't think of it for this.
I like it :D

@tapio
Copy link
Contributor

tapio commented Jan 14, 2013

Here's a jsperf for truncated versions of the functions in @bhouston's comment: http://jsperf.com/three-js-closure-vs-pre-allocated. So far looks like different browsers prefer different version. More data welcome.

@bhouston
Copy link
Contributor

@tapio Very useful. But those tests you wrote are likely be dominated by the Math.random() calls -- Random is super slow. I'd suggest a new test that preallocates an array of random numbers (10000 or more) that you then read from in the performance tests just incrementing through the array each time you need a random number.

@bhouston
Copy link
Contributor

Updated to pre-allocate random: http://jsperf.com/three-js-closure-vs-pre-allocated/2

@mrdoob
Copy link
Owner

mrdoob commented Jan 14, 2013

Even if slower, I would prefer the closure approach for sanity reasons.

However, we would still have the problem that Vector3 will depend on Quaternion and Quaternion will depend on Vector3...

applyEuler: ( function ( v, eulerOrder ) {

    var quaternion = new THREE.Quaternion();

    return function ( v, eulerOrder ) {

        return this.applyQuaternion( quaternion.setFromEuler( v, eulerOrder ) );

    }

} )(),

@bhouston
Copy link
Contributor

@mrdoob, Sorry about that. I guess I am showing my lack of JavaScript experience as my solution didn't actually fix the issue at hand. :-/ I wonder if one could actually delay the realization of the prototypes until after the fact. Sort of like.

Vector3 = function()....;

Vector3.PrototypeDefinition = function() { return {}; }

Quaternion = function() ...;
Quaternion.PrototypeDefinition = function() { return {}; }

// sometime later.
Vector3.prototype = Vector3.PrototypeDefinition();
Quaternion.prototype = Quaterion.PrototypeDefinition();

I wouldn't be surprised if there is some other type of JavaScript paradigm that I am not aware of that can handle this situation -- some other method of predefining types.

One method to avoid circular dependencies would be to put the method that involve both a simple and a more complex type on the class of the more complex type. Thus methods that involve both a Vector3 and a Quaternion would go on the Quaternion class. This would ensure that complex classes are dependent upon simpler classes but not vice versa... but this would be a large change to Three.js that is likely to be disruptive at this point.

@WestLangley
Copy link
Collaborator Author

@mrdoob Quaternion.js does not depend on Vector3.js as long as my mod is moved to Vector3.js, where it should be.

Why not just move Quaternion.js above Vector3.js in the build order and be done with it?

In other words, push whatever problem we think we still have down the road. ;-)

@bhouston
Copy link
Contributor

@WestLangley that is an easy solution!

@gero3
Copy link
Contributor

gero3 commented Jan 14, 2013

That's how I fixed most of these issues. @WestLangley @bhouston

@mrdoob
Copy link
Owner

mrdoob commented Jan 14, 2013

Why not just move Quaternion.js above Vector3.js in the build order and be done with it?

Sure! ^^

@bhouston
Copy link
Contributor

@jdolan It should be equally easy to rotate your cameras using Three.js. I'd suggest looking at some of the examples such as the three.js/examples/webgl_camera.html It shows do to do this. In the future I'd suggest looking at StackExchange for this type of help, see here: http://stackoverflow.com/questions/tagged/three.js

@bhouston bhouston mentioned this pull request Jul 3, 2019
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.

None yet

6 participants