o3d-webgl: Added remaining param_operation classes.

o3d-webgl: Added remaining param_operation classes.

This CL adds the Matrix4Translation, Matrix4Scale and Matrix4AxisRotation, which are used by some collada meshes.

BUG=none
TEST=Convert and load an animated collada mesh.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=54990

[Patrick Horn, 2010-08-03 02:20:28]

The collada mesh here requires a few additional param_operation classes:
http://inst.eecs.berkeley.edu/~pathorn/skinned_robot.zip

One required using a complicated o3d.Transform function which I made static.

[luchen, 2010-08-03 03:08:21]

Is running the mesh in simpleviewer the test for making sure the new params
work?

http://codereview.chromium.org/3029050/diff/1/2
File o3d/samples/o3d-webgl/param_operation.js (right):

http://codereview.chromium.org/3029050/diff/1/2#newcode374
o3d/samples/o3d-webgl/param_operation.js:374: * A Param operation that takesa an
input matrix, a float3 axis and an angle
Typo: takesa? Here and also below a few instances.

[petersont, 2010-08-03 22:15:01]

consider the comments below, otherwise, lgtm.

http://codereview.chromium.org/3029050/diff/1/2
File o3d/samples/o3d-webgl/param_operation.js (right):

http://codereview.chromium.org/3029050/diff/1/2#newcode381
o3d/samples/o3d-webgl/param_operation.js:381: this.last_output_value_ =
[[1,0,0,0],[0,1,0,0],[0,0,1,0],[0,0,0,1]];
space after comma

http://codereview.chromium.org/3029050/diff/1/2#newcode453
o3d/samples/o3d-webgl/param_operation.js:453: ret[3] = m3;
This will set ret[3] to m[3] by reference, then if you change the input, it will
change the output just in the [3] spot.  I could be wrong, but I think it might
be a good idea to sack a little efficiency and copy m3 just to be safe.

http://codereview.chromium.org/3029050/diff/1/2#newcode481
o3d/samples/o3d-webgl/param_operation.js:481: * @return {!Array<!Array<number>>}
4x4 array from translating inputMatrix
these should be {!Array.<!Array.<number>>} throughout the file.

http://codereview.chromium.org/3029050/diff/1/2#newcode498
o3d/samples/o3d-webgl/param_operation.js:498: ret[0] = m0;
Here, the same thing as above, essentially, some of the sub-arrays of the
returned matrix are brand new and some are referencing m, and I'm just not sure
that will always work.

[Patrick Horn, 2010-08-03 23:18:35]

Regarding the test, this example mesh uses all three of the new classes, so it
seems like a reasonable test. Though, if we had unit tests, it would be a lot
nicer to know that it's all working.

http://codereview.chromium.org/3029050/diff/1/2
File o3d/samples/o3d-webgl/param_operation.js (right):

http://codereview.chromium.org/3029050/diff/1/2#newcode374
o3d/samples/o3d-webgl/param_operation.js:374: * A Param operation that takesa an
input matrix, a float3 axis and an angle
On 2010/08/03 03:08:21, luchen wrote:
> Typo: takesa? Here and also below a few instances.

Done.

http://codereview.chromium.org/3029050/diff/1/2#newcode381
o3d/samples/o3d-webgl/param_operation.js:381: this.last_output_value_ =
[[1,0,0,0],[0,1,0,0],[0,0,1,0],[0,0,0,1]];
On 2010/08/03 22:15:01, petersont wrote:
> space after comma

Done here and also replaced a few I made in skin.js

http://codereview.chromium.org/3029050/diff/1/2#newcode453
o3d/samples/o3d-webgl/param_operation.js:453: ret[3] = m3;
On 2010/08/03 22:15:01, petersont wrote:
> This will set ret[3] to m[3] by reference, then if you change the input, it
will
> change the output just in the [3] spot.  I could be wrong, but I think it
might
> be a good idea to sack a little efficiency and copy m3 just to be safe.

To avoid spelling out each argument, I added .slice(0) which seems to be a
shallow copy.

http://codereview.chromium.org/3029050/diff/1/2#newcode481
o3d/samples/o3d-webgl/param_operation.js:481: * @return {!Array<!Array<number>>}
4x4 array from translating inputMatrix
On 2010/08/03 22:15:01, petersont wrote:
> these should be {!Array.<!Array.<number>>} throughout the file.

Done.

http://codereview.chromium.org/3029050/diff/1/2#newcode498
o3d/samples/o3d-webgl/param_operation.js:498: ret[0] = m0;
On 2010/08/03 22:15:01, petersont wrote:
> Here, the same thing as above, essentially, some of the sub-arrays of the
> returned matrix are brand new and some are referencing m, and I'm just not
sure
> that will always work.

Done.

[luchen, 2010-08-04 21:43:08]

LGTM.

On 2010/08/03 23:18:35, Patrick Horn wrote:
> Regarding the test, this example mesh uses all three of the new classes, so it
> seems like a reasonable test. Though, if we had unit tests, it would be a lot
> nicer to know that it's all working.
> 
> http://codereview.chromium.org/3029050/diff/1/2
> File o3d/samples/o3d-webgl/param_operation.js (right):
> 
> http://codereview.chromium.org/3029050/diff/1/2#newcode374
> o3d/samples/o3d-webgl/param_operation.js:374: * A Param operation that takesa
an
> input matrix, a float3 axis and an angle
> On 2010/08/03 03:08:21, luchen wrote:
> > Typo: takesa? Here and also below a few instances.
> 
> Done.
> 
> http://codereview.chromium.org/3029050/diff/1/2#newcode381
> o3d/samples/o3d-webgl/param_operation.js:381: this.last_output_value_ =
> [[1,0,0,0],[0,1,0,0],[0,0,1,0],[0,0,0,1]];
> On 2010/08/03 22:15:01, petersont wrote:
> > space after comma
> 
> Done here and also replaced a few I made in skin.js
> 
> http://codereview.chromium.org/3029050/diff/1/2#newcode453
> o3d/samples/o3d-webgl/param_operation.js:453: ret[3] = m3;
> On 2010/08/03 22:15:01, petersont wrote:
> > This will set ret[3] to m[3] by reference, then if you change the input, it
> will
> > change the output just in the [3] spot.  I could be wrong, but I think it
> might
> > be a good idea to sack a little efficiency and copy m3 just to be safe.
> 
> To avoid spelling out each argument, I added .slice(0) which seems to be a
> shallow copy.
> 
> http://codereview.chromium.org/3029050/diff/1/2#newcode481
> o3d/samples/o3d-webgl/param_operation.js:481: * @return
{!Array<!Array<number>>}
> 4x4 array from translating inputMatrix
> On 2010/08/03 22:15:01, petersont wrote:
> > these should be {!Array.<!Array.<number>>} throughout the file.
> 
> Done.
> 
> http://codereview.chromium.org/3029050/diff/1/2#newcode498
> o3d/samples/o3d-webgl/param_operation.js:498: ret[0] = m0;
> On 2010/08/03 22:15:01, petersont wrote:
> > Here, the same thing as above, essentially, some of the sub-arrays of the
> > returned matrix are brand new and some are referencing m, and I'm just not
> sure
> > that will always work.
> 
> Done.