Feature Discussion: gl-matrix 2.0

[toji]

So I've got a bit of free time on my hands and wanted to direct it at something I've been meaning to do for a while: Start the second iteration of the glMatrix library. This issue is to collect feedback on the proposed changes before we go crazy implementing them.

To be clear, the bump to a 2.0 library would indicate breaking of backwards compatibility, and thus several ill considered design decisions/flaws can be corrected without concern about breaking existing usage. The library in it's current form will likely stick around for a while longer and receive a few minor bug fixes, but new features would be 2.0 facing.

A few items that I feel must be part of the refactor:

• The current method of using array-like objects will stick around. The library will continue to create typed arrays by default, but can accept anything numerically indexable.
• As proposed in Proposal: always require a receiving object in every API #41, the semantics for input/output parameters should completely standardized. ALL functions would require an output object where applicable, no more implied change to the first/second param.
• Related to the previous item: Parameter names should be consistent. Names like "vec2" are confusing, since vec2 is also a "type" in gl-matrix. Preferred naming conventions for parameters are "a", "b", and "out", unless a different name would clarify the usage substantially.
• Standardize error handling. There are a couple of cases (matrix inversion, for example) that may have no valid solution. Currently the error handling is "return null and hope they notice." but that's not terribly well defined. Whatever method is used to handle errors, it needs to be well documented and consistent.

In addition there are some things that I would like to see

• I've long lamented the fact that the symantics for matrix multiplication are essentially "backwards" in gl-matrix, as noted by this Google code bug. I feel that any applicable operations should be changed so that the result is a more natural out = a * b instead of the current out = b * a. My only reservation here is that it may make porting code slightly more confusing.
• When gl-matrix was first introduced TypedArrays were fairly new. Now even browsers without WebGL support them, so we should investigate ways to utilize them better. A specific example would be creating a "clone" function that passes the structure to copy into a Float32Array constructor.

Finally, there's a couple of items that have been proposed by the community but that I'm not sold on:

• This Google code bug suggests changing to a non-namespaced pattern, so that mat4.multiply() would be mat4Multiply(). The reason given is that it would be faster (and it would be, if only a little!) but I'm not sure if I like the idea or not.
• I've heard a couple of suggestions that along with requiring an output parameter on all operations we also introduce a set of functions that explicitly alter the first param, most likely named ___Self. a.k.a: vec3.translateSelf(a, [1, 2, 3]); would alter a. This would allow us to retain some current functionality and may even be faster is some cases, but the API is pretty large as-is, and I'm a little reluctant to clutter it further.
• Stop returning the output parameter. This would (theoretically) be faster, but would eliminate the ability to nest functions.

I'm sure there's other items I can add to this list, but that's what I've got off the top of my head. More feedback is appreciated!

[ioquatix]

I haven't used glm but I have looked at it briefly.

You should be using column major multiplication which is standard for OpenGL, this means that a * b * c * vec is read from right to left, e.g. a(b(c(v)))) - e.g. apply c to v, then apply b to that result, then apply a to that result. This is natural for mathematicians and pretty much the defacto form.

Using typed arrays makes sense and hopefully allows some of the operations to be optimised.

If people have a problem with having a prefix, why don't they just write var m = mat4.multiply and call m(...).

vec3.translateSelf seems like a bad idea. If people want to optimise the code to minimise copying you could have a separate module for doing that.

I'd be inclined to put all the code in one namespace e.g. glm.mat44.multiply

[ioquatix]

Another thing you might want to consider is applying a simple multiply to multiple vertices, e.g.

glm.mat44.apply(m44, vertices)

This allows you to provide efficient implementations of matrix multiplication and avoid temporaries.

The same can be done for chaining multiple multiplications, e.g.

glm.mat44.product([mat1, mat2, mat3, mat4, mat5])

By doing this in one function we can avoid making 4 temporary buffers.

It is pretty common to see this use case e.g. when making cameras, traversing a scene graph, etc.

[jagenjo]

Great work, I think there is no need to ditch the namespace to improve performance, if anyone want it it is easy to create bindings.

Here is a list of methods I missed when using glMatrix:
vec3.toArray: to non-typed array, I need this when using stringify.
vec3.min: given two vectors returns a vector where every value is the min of both vectors
vec3.max: the same but max value
mat4.rotateVec3: as converting to mat3 and multiplying the vector but direct
quat4.toEuler transform the rotation in 3 simple rotation along the basic axis (its useful when you want to show the rotation in a user friendly and manipulable way).
quat4.fromEuler: the oposite.

And I also miss a lot functions to create matrices with a transformation, like setTranslation or setRotation instead of having to rotate an identity matrix, that annoys me .

Thanks again for you amazing effort. :)

[kpreid]

I like the idea of the ...Self methods. I would use a single-letter suffix (us GL programmers are used to it, and there's some good old precedent (to me) in, for example, Common Lisp's n... functions). For this library, I don't mind having a large API — the job of gl-matrix is to provide exactly the operation I need in the most efficient form. However, it would be nice — even for the current version — if there was a ‘quick reference card’ style documentation page which listed each function and the roles of its arguments (including left vs right multiplication) in an extremely compact layout — one line per function, or nearly so.

Please don't un-namespace the methods; if we want speed we can always explicitly assign them to local variables (which in future JS versions will likely have nice syntax as a "module import" facility), and my understanding is that local (or even closed-over to some levels) variables are sometimes significantly faster than global variables since the lookup for global variables is hairily dynamic.

Suggestion for error handling — return a vector/matrix of NaN. This has several advantages: (1) NaNs may already occur in numerical code written by the user, so if they have the sort of code with exceptional cases they may need to check anyway. (2) The user can do a chain of numerical operations and check for NaN at the end, rather than having to test for a null matrix at every point gl-matrix might return one. (3) If the user doesn't check at all and has a rarely occurring failure case they didn't think about, then they'll just see some disappearing geometry or similar occasionally, rather than possibly crashing.

[sinisterchipmunk]

Disclaimer: there are an infinite ways to read the English language. The following are all merely my opinions based on my own experiences over time, and shouldn't be taken as anything but. If anything implies an accusation, it's not meant to. Also, apologies for the length. I had one thought, and then I had a lot of them.

• I'm unconvinced but largely uncaring about the Self methods. I know right away that I likely won't use them; on the other hand, that doesn't mean they're not useful and maybe even more efficient (though I'd like to see this benchmarked). I just find the construct a bit ugly. I love the terseness of gl-matrix: mat4.multiply, vec3.add, and friends all give a massive amount of information about what they do without adding the overhead of extra typing (as compared, for an extreme example, with something like Matrix4x4F.multiplyMatrix4x4F). Adding Self to the end of any given method makes it less terse, confuses the meaning just a tiny bit (in my own mind at least), and adds at least 5 keypresses (including SHIFT) to any given invocation -- not to mention leading to tired pinkies. :( Again, not saying they won't be useful, but if it was up to me and only me, I'd probably skip them in favor of less a bulky API.
• One more note about the *Self methods. I don't like having too many ways to do any one thing; it makes the API at large more confusing and steepens the learning curve. If an operation can be performed another way, IMHO we should be scrutinizing the value the extra functions are actually adding.
• I'm with @kpreid on the namespacing 100% -- it was my first thought on the matter. Namespacing is your friend, and not namespacing is bad practice in JS, asking for collisions at one point or another. If it weren't so wordy, I'd be in favor of GLM.mat4.multiply, even. I'm not in favor of something like GLM.mat4Multiply, though. To me, something like that loses readability. I love the current namespacing. Let's keep it.
• I also love @ioquatix's suggestion about applying transformations to a set of vertices (at least I think that's what he said). Something like mat4.multiplyVec3s(vecs), where vecs is a 1D array. I would stay away from nested arrays or variable arguments though, as these are both slow. I think the idea of combining multiple operations is novel, but I don't see how it can be done efficiently without introducing the unnecessary creation of temporaries (as shown in ioquatix's own example). I think such a thing should be thoroughly benchmarked before committed to; it may be better to simply encourage reuse of cached variables even if it's a little more wordy.
• Operation order, IMHO, should be identical to what we (as GL coders) see in the shaders. mat4.multiply(mvp, p, mv) should be equivalent to the GLSL code mvp = p * m. We need consistency not only in our own APIs, but across the realm of APIs that are likely to be worked with. Same goes for column ordering, as it's crucial to be able to toss gl-matrix objects directly into a shader's matrix uniforms and have them Just Work (TM).
• I don't think there's any beautiful way to handle error conditions. The most correct would be to throw an actual Error. That'd also be the most difficult for developers to work around, having to wrap basically every potentially-offending operation in try...catch. IMHO, that's not acceptable. I'm not opposed to returning a matrix of NaN, but honestly in my own code an error condition has always been due to invalid methodology on my part. In these cases, I've been perfectly happy with them returning null, as this gets exposed in my JavaScript console immediately as a catastrophic error with a stack trace -- exactly what I've wanted in each case. I don't refute that there are edge cases, but I say let those edge cases check for null explicitly, and let the current implementation prevail. It's easier to track down the source of the error for the rest of us.
• Another note on errors. Personally, I like to see them in development as often as possible. It helps me vet the code prior to release. In production, I hook up an onError listener to intercept, report on, and possibly recover from such things. So we can throw errors or return null and let the developer decide how best to recover, if at all. If we use NaN, we take away the ability to do such things under the assumption that the developer will always know about NaN conditions prior to release.
• I'd love to see the new version of the library thoroughly test-driven, instead of tossing unit tests in as an afterthought. This not only builds our own confidence in the system, it also gives newcomers very nice examples of how to use the library, and it also sets a precedent so that each pull request can be rejected if it doesn't include a corresponding test demonstrating the need (and use cases) for the new code, especially for when we don't understand it due to complexity or language barrier. As it is, I feel bad about accepting pull requests that are untested, but I feel hypocritical rejecting them on that basis because our own testing discipline has been lacking.
• Please, let's start tagging releases. This gives us a better way to track APIs, changes and regressions over time; it allows other coders to "lock down" the version of gl-matrix that they are using; and it gives us an excuse to make changes to the master branch willy-nilly, because we can say, "Why are you linking directly to gl-matrix.js on master? We have tags for that." Master should be used for active development, and people should never link their pages directly to a git repo's master because of that -- but right now they have nowhere else to link to. (I disagree with the linking practice entirely; they should cache their own copies of gl-matrix, but that's another matter.)
• I second the cheat sheet API docs idea. Most of the names tell the developer all s/he needs to know; the full signature (name + arguments) tells the rest of the story. With more API consistency (always or never return a value, always make that value the receiver, always have a receiver as the first argument, and always do [whatever] on an error), we don't even need to document such things for each method. Just doc them once at the top of the page, and then throw up the method names and arguments. If the whole thing can be printed on a single sheet of paper, bonus.
• I like the returning of values. I'm all in favor of speed boosts, but we have to draw the line at some point: browsers should be fast at JavaScript, period, and a fundamental thing like returning a value from a function should not incur major overhead. If it does, we need to start harassing the vendors more. Even so, I could see removing the return values if benchmark shows conclusively that the performance gains are massive; but otherwise I'm of the opinion that returning the receiving object vastly simplifies the use of gl-matrix and I'd hate to see it go.
• Let's split the gl-matrix.js file into chunks. Create a lib directory, and let each file be named after its namespace (mat4.js, for example). Then aggregate them together during build. This can be added to the Rakefile very easily; then something like rake release is all you need to build, minimize, tag and push the latest version. Likewise, let's pull the bootstrapping code into its own file, and make that whole process a bit cleaner.
• Let's integrate with travis-ci. This way any time a commit gets pushed or a pull request gets merged, the test suite runs. If anything breaks, we'll know via email the moment it happens. Obviously, this goes hand-in-hand with the unit testing. We won't get this info unless we test.

[toji]

Thanks for all the great feedback! Feel free to keep it coming!

So far popular consensus seems to be settled on a few points:

• The output parameter will be the passed first. (mat4.multiply(out, a, b)) This was a surprisingly popular request!
• We'll be keeping the namespaces.
• There are several requests to provide functions that operate on an array of values. I think this is a lower priority overall, but it's a good suggestion that I think it worth pursuing once the current functionality has been refactored.
• Unless there is a MASSIVE performance gain to be had (there's probably not) returning the output parameter is a popular function and will be staying

Opinions are split on the "Self" function variants. I think we'll be leaving them out for now.

On the subject of Error handling, I think @sinisterchipmunk's analysis makes the most sense: It's usually more important to be able to track errors than to gloss over them. As such while the idea of returning an array of NaN's is somewhat appealing I think we'll probably stick with null returns instead. I do think that a "debug version" of the library is worth considering, though, that would add a lot of error checks and throw detailed messages to help track down issues. That's probably lower priority, though.

There's a lot of other suggestions that have been given, and a lot of them are very appealing. They'll be evaluated as we go. I did want to mention that specifically that I back everything @sinisterchipmunk mentioned, especially the points on test-driven development and splitting the library into multiple files during development (obviously there would be a single "production" file available for easy distribution)

[toji]

Copying suggestions from Won Chun on my Google+ post here for easy referencing:

• I strongly support having distinct 2 (mutating) and 3 (explicit destination) interfaces as opposed to if()-style overloading.
• Definitely keep the namespace.
• You have Matrix inverse, but no adjoint, which is the more useful of the two when dealing with homogeneous coordinates. Dividing by a scalar, like the determinant is a waste of time and precision, and and opportunity for numerical instability).
• Re: parameter order and such: ryg has some very clear prose on the matter: http://fgiesen.wordpress.com/2012/02/12/row-major-vs-column-major-row-vectors-vs-column-vectors/
• Global epsilon is somewhere between a hack and a lie. With floats, there are times you care about relative error (scale factors, such as matrix coefficients) or absolute error (post-transform positions).
• Bikeshedding: "equal" is usually "equals"
• I don't think quaternions should have all those branches in there to deal with the double-cover. What is a double-cover "problem" to some people is a double-cover "solution" since you can use the sign to control the interpolation direction (long v. short).
• Slerp is overkill much of the time. You should also support lerp, even though it isn't velocity-uniform. As a bonus, lerp commutes which lets you more easily compose them. I guess you could just use vec4.lerp in that case, so I guess you would just need an alias
• String representation of quat and vec4 should probably be distinct.
• Bikeshedding: Multiply -> Mul throughout. I don't think the shorthand would affect readability much.
• Array-based (e.g. transform N vec3s with this matrix) application would be awesome. Support explicit start/end bounds instead of requiring a view, since TypedArray creation is so bloody expensive.

[toji]

Starting on code now! Work on 2.0 will happen in the 2.0-dev branch until it's ready to be merged back into master.

I did vec2 first, since it's one of the simpler types and makes for a good testing ground. It'd be nice to get a bit of feedback on it before I start applying the same patterns to the rest of the library. Notable changes are:

• Unit Tests for EVERYTHING from day 1. You're welcome, @sinisterchipmunk. :)
• All vec2 functions are in their own file now. Will be combined with a build process for release
• out is always the first parameter where applicable, and is always required
• vec2.create now explicitly creates vectors initialized to [0, 0]
• vec2.clone handles the functionality previously provided by vec2.create(vecToCopy)
• vec2.createFrom is now called vec2.fromValues
• The output of vec2.str has changed from "[0, 0]" to "vec2(0, 0)" to address Won's concern about string representations being ambiguous for certain types (quat/vec4)
• Since vec2.cross requires an output vector the old alternate code path returning the z component has been removed
• The following functions have been given aliases:
  • vec2.subtract -> vec2.sub
  • vec2.multiply -> vec2.mul
  • vec2.divide -> vec2.div
  • vec2.distance -> vec2.dist
  • vec2.squaredDistance -> vec2.sqrDist
  • vec2.length -> vec2.len
  • vec2.squaredLength -> vec2.sqrLen

There's still a few missing functions, and I'm working on implementing a couple more of the requests (like min/max), but this should Any of the above changes (except for the unit tests) are up for discussion, so if you feel strongly about them one way or the other please speak up! I promise no offense will be taken if you don't agree with me! :)

[gero3]

you should provide a copy functionality too

vec2.copy = function(out,a){

  out[0] =a[0];
  out[1] =a[1];
  return out;

};

[ioquatix]

@toji I made a rake script for building minified release of a collection of JS files for another of my projects jQuery.Syntax, it would be perfect for building a minified release automatically, let me know if you are interested.

[sinisterchipmunk]

Sorry I didn't remember this one sooner, but how do we feel about CoffeeScript? It has a concise and pretty syntax, and bills itself as generating JS that runs as fast as or faster than its human-typed counterpart. In my own experience, this has proven true. I've watched the code it produces carefully because I use it for webgl, and it has even generated faster code than my own a few times.

We can hook it up to rake to automate the build process if we decide to use it. Thoughts?

[sinisterchipmunk]

Added Rake tasks for building, minifying, testing and releasing. Please see 4d6e2c8 and c4c1cf8. Doing so required some directory restructuring. Hope I didn't step on anybody's toes.

I agonized for a while about whether dist/gl-matrix.js and dist/gl-matrix-min.js should be committed or ignored, and whether they belong in lib instead.

My reason for committing them under dist is that they really always represent the entire build, so they belong in dist; and that HEAD basically represents the "Edge release" (if that's the right term) so its product should be accessible to people who are living on Edge but not developing the library itself. It also adds a layer of safety, in that the release task requires all changes to be committed before it'll go ahead with the release, to prevent releasing old code. If the dist files are ignored, then they can't be validated that way and release would just tag the most recent commit (assuming no other changes were made) -- which could feasibly lead to tagging the wrong code.

Naturally, this decision is open to debate if anyone strongly disagrees with my approach.

[ioquatix]

@sinisterchipmunk For my build system, I included a YAML configuration file which includes the destination path. This way it makes it easy to deploy into a separate location. The YAML configuration file could be updated to include whether the result was minified, for example, rather than building separate minified and non-minified versions (which, technically, in my opinion isn't really fantastic).

[ioquatix]

@sinisterchipmunk I don't know the dist convention, but I am familiar with tagging versions or using a stable branch. Wouldn't either of those two approaches be more canonical?

[sinisterchipmunk]

@ioquatix the release task committed above does tag versions, but the files themselves needed to go somewhere (and we aren't publishing them to any website, to my knowledge). As you can see in the first commit, I was originally going to have them ignored during development and only re-generated at release. What made me flip-flop was the release process itself. The release task checks whether there are changed files waiting to be committed; if so, the release is aborted for fear of tagging the wrong commit (and therefore, tagging outdated/unstable code). This check seemed like a good idea to me, but only mattered if the files were committed in the first place. It condemns us to committing frequently-changing generated files, but I thought the pros outweighed the cons there. Do you disagree?

I think your YAML configuration idea is an interesting one; I hadn't thought about it from that angle. Are you suggesting that the configuration could control which type of file is generated (minified vs not), depending on what the action being taken is?

One more thought. The code under src essentially contains our non-minified base. So, we could have all code generation default to a single minified file, discarding the non-minified one entirely; even the unit tests would run against the minified version. This is a very safe option, because not only does it remove human error from the release process, but if something should go wrong with the minifier itself, we'd know as much during development, and probably right away. This change would throw away useful backtraces, but we could always leave a rake task in place for generating a non-minified version in order to reproduce issues where a backtrace is necessary.