toji / gl-matrix

Javascript Matrix and Vector library for High Performance WebGL apps
glmatrix.net
MIT License
5.32k stars 718 forks source link

Feature Discussion: gl-matrix 2.0 #43

Closed toji closed 11 years ago

toji commented 11 years ago

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:

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

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

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 commented 11 years ago

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 commented 11 years ago

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 commented 11 years ago

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 commented 11 years ago

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 commented 11 years ago

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.

toji commented 11 years ago

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

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

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 commented 11 years ago

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

toji commented 11 years ago

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:

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 commented 11 years ago

you should provide a copy functionality too

vec2.copy = function(out,a){

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

};
ioquatix commented 11 years ago

@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 commented 11 years ago

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 commented 11 years ago

Added Rake tasks for building, minifying, testing and releasing. Please see 4d6e2c81c5fb92669942772ad5a19861a15b3551 and c4c1cf8f90c45942b3ef3b95ae3e96925cb61a1d. 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 commented 11 years ago

@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 commented 11 years ago

@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 commented 11 years ago

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

ioquatix commented 11 years ago

@sinisterchipmunk While a lot of people do it, I personally try to avoid pushing compiled/minified/production versions of code back into the source code repository. I normally checkout the source code to my-website/materials/library then have a configuration file that tells it to install to my-website/public/_static/library

toji commented 11 years ago

@sinisterchipmunk - I like 'em in the dist folder, though I think I may need to restructure how the individual code files are done looking at the output. As for CoffeeScript, I'm typically not a big fan of languages that you can't debug in their original form. I would be interested to know if it does produce more efficient JS than what we've got now.

@gero3 - I was actually wondering about wether to call that function "set", like in the previous code base, or "copy". Copy seems to fit better with "create" and "clone" but I'm slightly reluctant to change the name just for the sake of changing it. Really I'm fine with it either way, anyone else want to cast their vote one way or another?

toji commented 11 years ago

@sinisterchipmunk, I can't get the bundler gem to install on Mountain Lion. Does it still work for you?

sinisterchipmunk commented 11 years ago

@toji Yes, just tested on an MBP that was upgraded to Mountain Lion yesterday. Worked fine with system Ruby (1.8.7).

gero3 commented 11 years ago

This is copy for me.

vec2.copy = function(out,a){

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

};

adn this is set for me:

vec2.set = function(out,a,b){

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

};
toji commented 11 years ago

So, an interesting performance characteristic I've discovered: It seems that creating an empty Float32Array takes just as long (and is sometimes slower!) than creating one initialized from an array. You can check it out yourself with this jsPerf.

With that in mind, it seems like a sensible idea to initialize new Matricies created with mat_.create() to an identity matrix. That eliminates the need for one function and gives every matrix created a sensible default.

sinisterchipmunk commented 11 years ago

it seems like a sensible idea to initialize new Matricies created with mat_.create() to an identity matrix.

I prefer this approach. On my first day learning to use gl-matrix, I made the incorrect assumption that this was already the case, and lost some time trying to figure out why my matrices were wrong.

Additionally, a well-written app won't be continually creating temporary Float32Arrays. So even if it were slower, I'd still be in favor of this for its sheer intuitiveness.

toji commented 11 years ago

A some new code to take a peek at and few more comments.

The basic mat2 implementation is in place, and as always I'm happy to hear comments on it before we propagate the patterns set in it throughout the rest of the matrix types.

I'm a little torn on wether or not I care to implement methods like mat2.set or mat2.fromValues, because extrapolating it out I can't see anyone really wanting to type out 'mat4.set(1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1);` or similar. Maybe I'm wrong, but those seem like methods that are simply better suited to vectors.

In a related subject to the vector set methods, though, I'm wondering what the best way to proceed with a previous request would be. @jrus mentioned at some point that he would like to see vec2(), mat2(), etc. be used as an alternate way to create vectors/matrices. I like this idea, but the more I think about it the more I would prefer the syntax to be:

var v = vec3(1, 2, 3);

Effectively making the function version of each type an alias for type.createFrom(). I like this because it mimics the syntax from GLSL, and I've found that I use this sort of initialization frequently in my own code. It does somewhat clash with my earlier statement about matrix creation functions not feeling appropriate with long lists of parameters. however, and some people may find it to be less convenient than a straight up:

var v = vec3();

I'm not opposed to doing some if/then overloading on these specific functions, though, as they are intended to simply be convenience functions and you really shouldn't be creating vectors/matrices often if you can help it. So maybe we can have it both ways?

Finally, I've moved the functionality of what used to be mat2.multiplyVec2 into vec2.transform. The diving force behind that decision is that I feel pretty strongly that the functions should be grouped under the type they output, and thus having a vec2-producing function under mat2 felt awkward and broken usage patterns in the previous versions. I'm not completely sold on the name: I could also see using multiplyMat2 or transformMat2. Either of those may be more appropriate, since there's a decent use case for transforming a vec2 with a mat 3, or a vec3 with a mat4.

A few last personal responses:

@sinisterchipmunk: It's settled then! Matrices will initialize to the identity.

@won3d: Added adjoint as requested. I'm curious to lean more about it's use in graphics dev!

@gero3: I like your distinction between copy and set. I've already implemented it in vec2

gero3 commented 11 years ago

44 should improve the speed a bit altough not tests are in it??

toji commented 11 years ago

In keeping the spirit of @gero3's proposal, I've added a common.js file that will hold various common utilities and aliases for faster calling.

I'm slightly concerned about the implementation of invsqrt, however. I tried to make use of it in vec2.normalize, but found that the accuracy was pretty bad. For example, attempting to normalize [5, 0] would yield [0.998448825932014, 0] instead of the expected [1, 0]. 0.0015 is a pretty big margin of error, relatively speaking, so I'm a little reluctant to push that as the standard normalize method. Of course, I know that the algorithm came from the Quake code, and there's a part of my brain that wants to say: "Good enough for Carmack == good enough for me", but I also don't want to limit the scenarios the code can be used in.

I think that the real solution is to have a simple configuration section at the top of the generated files (unminified) with basic options like:

and so on. These would mostly be options that take effect once during load. Then the user could choose their own performance/accuracy characteristics.

toji commented 11 years ago

Making a note to myself for future reference: I've always found the D3DX Math libraries to be extremely useful. We should seriously consider adding functionality that it provides that we lack.

http://msdn.microsoft.com/en-us/library/windows/desktop/bb172972(v=vs.85).aspx

toji commented 11 years ago

First pass at vec3 is in place, mostly just copying and updating the functions from vec2. The interesting functions like unproject and rotationTo are still in progress.

toji commented 11 years ago

Got vec4 in tonight as well. Again, these are mainly functions copied from vec2 and vec3. Still need to review and add any vec4 specific stuff.

Juriy commented 11 years ago

Not sure, if someone else would find it useful, still...

I often find myself doing the the pair of calls: to trim the vector. First normalize (to get the vector with the length 1), then scale by the value. This is the regular operation when you deal with speeds and steering: you shouldn't allow the speed to be more than the certain value, so you trim the vectors at some point. How about including the following two functions to the library (not taking the mutable arg/return type into the account):

toji commented 11 years ago

@Juriy: I actually do the same in my code all the time, and had been considering adding that functionality. Thanks for the reminder!

ioquatix commented 11 years ago

@toji, @Juriy, the best way to support this is to have a default argument to normalize, e.g. length = 1, and allow you to call normalize(vector, 3) or something similar. The code path is efficient that way and API is minimised.

toji commented 11 years ago

Okay, I'm poking at this a little bit again. Sorry for the downtime: I've started a new job AND gained a newborn son within the space of about a month. All things considered I think I have a good excuse for being a little slow!

First off, at the prodding of Won Chun quat4 is now simply quat, because a quaternion implies 4 components and that makes it the same number of characters as the rest of the types.

Secondly, I'm curious what glMatrix users think about this idea: I'm considering renaming all of the types in 2.0 to have an upper-case first letter, as in mat4.multiply would become Mat4.multiply. The reasoning is that the old an new libraries will have functions that look extremely similar but have the arguments in a different order. Thus if you're trying to update a code base, you could easily miss several calls and end up with frustrating-to-track bugs. If the names are subtly different, however, it's easy to search for all instances that haven't been upgraded yet, and the browser can give you much earlier warnings about broken functions.

I don't know how common that scenario will be, but I don't find the change objectionable (I would normally case a class like that anyway, so it feels natural for "namespaces" like these). Does anyone else feel strongly about it?

525c1e21-bd67-4735-ac99-b4b0e5262290 commented 11 years ago

Please please please try to be as consistant as possible. After all, besides performance, that's why we're choosing to come together with this project, is it not?

I'm slightly reluctant to change the name just for the sake of changing it.

vs.

I'm considering renaming all of the types in 2.0 to have an upper-case first letter, as in mat4.multiply would become Mat4.multiply.


If you're not entirely sure...

I don't know how common that scenario will be, but I don't find the change objectionable

...and the change can be emulated by the end user using something as simple as an alias, do not make the change. Save it for the footnotes, hints and tips or gotchas section of the docs.


That said I do appreciate and agree with the vast majority of changes being made.

Let us focus on catering for the 95% use case over the myriad of hypothetical what-ifs and but-thens.


CoffeeScript

As for CoffeeScript, see http://github.com/feisty/math

It's kind of old and incomplete now and there's things I'd change about the way it's structured and how things are named but there's the gist of what porting gl-matrix to CoffeeScript would involve.

IMO it's not worth straying from pure JS for something as fundamental as a 3d math library.

But it would certainly be nice to have compatible implementations, in languages other than JavaScript such as CoffeeScript (there's a TypeScript implementation here https://github.com/vexator/TSM).


A few things that bother me:

building / dependency management

lib contains ruby helpers to build the project. This is really confusing. If anything lib should contain the code which is currently in src. All this ruby can easily be crammed together into the Rakefile. It's just a build script.

On that note, IMHO sprockets really sucks. Consider using a dependency pattern and build system based on something like CommonJS modules i.e. require()

package

The project was never packaged and published to to repos like npm and bower. It's a trivial task with massive benefits. Please fix this. It's one of the only reasons I don't always use or recommend the library. I'm always forking and hacking the living daylight out of gl-matrix to beat it into shape for npm :(.

I should be able to do something like this:


$ sudo npm -g install gl-matrix
$ node
> var glm = require('gl-matrix');
{
  mat4: {...}
  vec3: {...}
  ...
}

Nevertheless kudos for the fantastic work and the new direction of the project. Keep it up!

ioquatix commented 11 years ago

IMHO type names should always be camel case.

525c1e21-bd67-4735-ac99-b4b0e5262290 commented 11 years ago

camelCase or CapitalisedCamelCase?

Technically they're not really "types" (prototypes) though since they're not used in conjunction with new.

mat4.create() vs new Mat4

ioquatix commented 11 years ago

CamelCase as normally defined with the first letter being upper case.

525c1e21-bd67-4735-ac99-b4b0e5262290 commented 11 years ago

That's generally called Pascal Case or CamelCaps http://en.wikipedia.org/wiki/CamelCase

Nevertheless the "modules" (mat{3,4}, vec{2,3,4}, etc.) of gl-matrix aren't really the traditional "types" (prototypes) you find in JavaScript such as Dates or RegExps. In the JavaScript universe, PascalCase is reserved for functions used in conjunction with new such as new Date() or new RegExp().

You will find it very difficult to locate an exemption to this policy in any standard JavaScript APIs. I personally do not know of any.

I'm not sure the container objects / namespaces / whatever you want to call them of gl-matrix which are not used as constructors in conjunction with new should be capitalised.

It might "feel" better or feel like you are promoting the "types" in gl-matrix to some elevated status but it will be flying in the face of 17 years of standard JavaScript naming conventions.

ioquatix commented 11 years ago

@pyrotechnick The first line of that article is "CamelCase (or camel case), also known as medial capitals or Pascal case".

I understand where you are coming from but can you point to any official documentation in the JS standards or specifications that support your opinions? In particular, I've never seen this statement anywhere before: "In the JavaScript universe, PascalCase is reserved for functions used in conjunction with new such as new Date() or new RegExp()"

In general, I've seen most APIs using CamelCase for namespaces, classes and I would find it hard NOT to call vec3 a type, I mean, clearly we are classifying some specific data structure with a name, in my mind this would be typically referred to as a data type, or simply a type.

525c1e21-bd67-4735-ac99-b4b0e5262290 commented 11 years ago

Define official documentation...

http://javascript.crockford.com/code.html

Most variables and functions should start with a lower case letter. Constructor functions which must be used with the new prefix should start with a capital letter. JavaScript issues neither a compile-time warning nor a run-time warning if a required new is omitted. Bad things can happen if new is not used, so the capitalization convention is the only defense we have.

http://google-styleguide.googlecode.com/svn/trunk/javascriptguide.xml#Naming

In general, use functionNamesLikeThis, variableNamesLikeThis, ClassNamesLikeThis, EnumNamesLikeThis, methodNamesLikeThis, and SYMBOLIC_CONSTANTS_LIKE_THIS.

http://en.wikipedia.org/wiki/Naming_convention_(programming)#JavaScript

The built-in JavaScript libraries use the same naming conventions as Java. Classes use upper camel case (RegExp, TypeError, XMLHttpRequest, DOMObject) and methods use lower camel case (getElementById, getElementsByTagNameNS, createCDATASection). In order to be consistent most JavaScript developers follow these conventions.[citation needed] See also: Douglas Crockford's conventions

https://github.com/styleguide/javascript

Meh. Have fun struggling to justify your naming convention to every other JavaScripter on the planet.

http://bikeshed.com/

ioquatix commented 11 years ago

@pyrotechnick Great, thanks for the update, I'm sure Toji will find it helpful.

Is "mat4" a method or a class?

525c1e21-bd67-4735-ac99-b4b0e5262290 commented 11 years ago

It's neither. There are neither classes nor methods in JavaScript; at least until v6 lands.

"mat4" and friends are merely properties of the root object of the gl-matrix library.

I'm just going to go ahead and agree to disagree on this one.

Happy coding :)

ioquatix commented 11 years ago

Okay, let me ask that question from a different perspective, in C, would mat4 be a function or a type?

toji commented 11 years ago

Well, I sparked more of a discussion with the casing question that I anticipated! After reading the comments on both sides of the fence, I think I'm going to stick with lowercase mat4 and friends vs. uppercase Mat4. This is primarily because, as @pyrotechnick pointed out, consistency is important and making changes to assist an edge case is rarely a good plan.

Also, @pyrotechnick: I really appreciate your feedback on other parts of the project. I feel terrible that I've never gotten around to making glMatrix into an npm package. You can be assured that it will be fixed for v2 (and probably for v1 in the meantime. I'm making myself a reminder now...)

As for the current build system: To be honest I hardly know Ruby, that's all been Colin's work thus far and in my opinion it's been working well. I'm not keen to muck with something that's not broken. I have been considering making a small Google App Engine service that builds custom glMatrix libs, though (Don't use quats? Leave 'em out!) and that would require a Python variant of the build script, so maybe down the road. I'll cross that bridge when I get there though.

Finally, I've never cared much for CoffeeScript, sorry. I'm not opposed to a port but I won't personally be doing it.

One last item to bring up for more general discussion: There were a couple of requests at the beginning of this thread for a way to transform arrays of vec3s in a single function call. I actually stumbled on the need to do something similar today so I decided to mock it up and get some feedback.

/**
 * Transforms an array of vec3s in-place with a mat4.
 * 4th vector component is implicitly '1'
 *
 * @param {Array} a the array of vectors to transform
 * @param {mat4} m matrix to transform with
 * @param {Number} stride
 * @param {Number} offset
 * @param {Number} count Number of vec3s to transform
 * @returns {Array} a
 */
vec3Array.transformMat4 = function(a, m, stride, offset, count) {
    var i, l, x, y, z;
    var m0 = m[0], m1 = m[1], m2 = m[2],
        m4 = m[4], m5 = m[5], m6 = m[6],
        m8 = m[8], m9 = m[9], m10 = m[10],
        m12 = m[12], m13 = m[13], m14 = m[14];

    if(!stride) stride = 3;
    if(!offset) offset = 0;

    if(count) {
        l = Math.min(count * stride, a.length);
    } else {
        l = a.length;
    }

    for(i = offset; i < l; i += stride) {
        x = a[i]; y = a[i+1]; z = a[i+2];
        a[i] = m0 * x + m4 * y + m8 * z + m12;
        a[i+1] = m1 * x + m5 * y + m9 * z + m13;
        a[i+2] = m2 * x + m6 * y + m10 * z + m14;
    }

    return a;
};

The big question I have is if this setup provides the flexibility needed for the function to be useful? Also, does the order of the parameters feel sensible?

It also got me thinking that it would be valuable to have a more generalized version of this to easily do things like updating particle systems and whatnot, so I'm also considering having something like this:

/**
 * Perform some operation over an array of vec3s.
 *
 * @param {Array} a the array of vectors to iterate over
 * @param {Function} op Function to call for each vector in the array
 * @param {Number} stride
 * @param {Number} offset
 * @param {Number} count Number of vec3s to iterate over
 * @returns {Array} a
 */
vec3Array.forEach = function(a, op, stride, offset, count) {
    var i, j, l, x, y, z;
    var vec = Float32Array(3);

    if(!stride) stride = 3;
    if(!offset) offset = 0;

    if(count) {
        l = Math.min(count * stride, a.length);
    } else {
        l = a.length;
    }

    for(j = 0, i = offset; i < l; ++j, i += stride) {
        vec[0] = a[i]; vec[1] = a[i+1]; vec[2] = a[i+2];
        op(vec, j);
        a[i] = vec[0]; a[i+1] = vec[1]; a[i+2] = vec[2];
    }

    return a;
};

(And obviously vec2 and vec4 variants of the same) It's worth noting that I wanted to use Float32Array.subarray for the inner loop rather than copying values back and forth, but apparently that's painfully expensive

525c1e21-bd67-4735-ac99-b4b0e5262290 commented 11 years ago

Thankyou for your sound decisions @toji

You can have your thread back now :)

sinisterchipmunk commented 11 years ago

Sorry I've been out of the loop. Been rather busy, as most everyone seems to be these days.

@pyrotechnick

Thanks for the input. I agree with a lot of your points but feel I have to defend myself on the build system...

For the build system, I used Ruby because I was comfortable with it, and because I know how to make it interface with just about any other tool or build system we decide to evolve towards. I'm not against switching to a different system entirely, but I like the clarity and conciseness offered by Ruby, and nobody objected at the time. :)

For calling lib what it is, that decision was made as a matter of convention. I've never not seen a project put its supporting Ruby files in lib, but I don't deny that it's done, and I don't have anything against renaming the directory if Brandon agrees there is need to do so. We could call it build or something.

I'm flatly against the idea of moving all the Ruby code into the Rakefile, though. Code is code, build script or not, and readability and maintainability of code are of paramount importance IMHO. The idea of cramming it all into one file makes me literally cringe (and I'm not exaggerating! I take a lot of pride in writing clean code). Also, cramming it all into one file seems a lot like cramming all the JS into one file, something we've specifically taken steps against in v2.0.

On that note, IMHO sprockets really sucks.

I'd love to hear some specifics on this point. When I first started working with Sprockets, I had the same feeling, but have come to understand that most of my problems were due to a lack of understanding of the library itself. It does have some real strengths and major benefits, though they aren't really utilized in this project, and I've come to respect Sprockets a great deal.

@toji

Although I like the gist of what's happening in vec3Array.transformMat4, I'm still uncomfortable. It feels like a slippery slope into a ton of extra code (e.g. vec2Array.transformMat3, vec3Array.transformQuat and other variants) that will need to be maintained. It is starting to feel like bloat to me. OTOH, I really like the forEach idea, because it's both readable and versatile, even though it brings much the same code maintenance to the table. In the latter case, however, the overhead of the extra function call makes me a little nervous, but that might be unfounded as I haven't seen any recent benchmarks testing the cost of a function call. The other thing that makes me nervous is the callback itself. They're probably one of the most powerful features of JavaScript, but they're dangerous in a performance-driven environment like a WebGL app, because most people don't bother to cache the reference to the function. They just create functions in every frame. Still, that's more their problem than ours, and the same can be said for caching vectors and matrices. I'm generally in favor of one or both of these methods, as long as the proper usage is well documented in order to help inform people who are new to writing performance apps.

I do have a qualm with the argument order, but it comes down to preference. I have always had trouble with code like this:

setTimeout(function() {
  // do some stuff
}, 1000);

because to my eye, it's easy to miss the second argument and just feels "wrong". (And don't get me started on functions that accept multiple callbacks!) Granted, if you're caching the function reference it's less of an issue, but it still bugs me. Guess that's just how I am.

So, if you move op to the end of the arguments list, stride, count and offset become mandatory arguments. I'm not super enthusiastic about that idea either, but making them mandatory would carry an ever-so-slight performance boost, so there's a silver lining I suppose.

In the end I'll be on board with @toji decides, but I wanted to make sure these things are discussed.

ioquatix commented 11 years ago

@sinisterchipmunk

Actually I did originally suggest putting together some build scripts. I've got an existing track record of Ruby based build scripts for JS projects so I've had some real experience with how to do this process.

Putting the Ruby code in lib for what amounts to a non-ruby project isn't really a good idea. I'd recommend making a master rakefile and having a directory called tasks where you put individual tasks. You can see what I'd recommend on my dream-framework which does essentially that.

Finally, while it is common, I don't recommend that people check out code directly into their application directory. Instead you'd be better of a with a build system that can copy the files somewhere else based on the configuration file. That config file can specify what parts of the library to use and whether to minify, etc.

I don't think its advisable to be checking a minified copy of the library directly into the source code tree, and having some online system to do it just seems tedious in a well engineered project.

Secondly, my original suggestion regarding array based operations is primarily centred on common operations where you can improve performance by avoiding temporary object construction.

If you have done any serious 3D programming, transforming Vec3 vertices by a Mat44 is one of the most common operations and should be optimised if possible, second only to Mat44 * Mat44 multiplications. Therefore, including an optimisation for this specific case would be very useful in general. Other cases like Vec2 x Mat44 are less useful in general.

I hate to say it but this thread reminds me of the phrase "Too many cooks spoil the broth". I think the discussion is probably pretty useful but I can't help but think this whole thing has started to become really over designed - for what its worth I'm probably just going to roll my own opinionated math library sometime soon :D

sinisterchipmunk commented 11 years ago

I'd recommend making a master rakefile and having a directory called tasks where you put individual tasks

This is the norm for Rakefiles with lots of tasks; it's done for Rails and I do it in Jax. I have only ever added actual Rake tasks to tasks and not supporting code for them, but in hindsight it does make sense, especially for a non-Ruby library where code reuse is less of an issue. As stated previously, I'm not against the idea of moving the Ruby files out of lib, and tasks sounds like a good place for it.

I don't think its advisable to be checking a minified copy of the library directly into the source code tree

In general I would agree. Minification used to be a painful black art, but it is getting easier to do; Rails does it automatically and anyone in the dark can check the Rakefile to see how to do it in 1 line with Ruby. I think this project is gradually evolving away from the need for a committed minified copy. Having said that, I don't know that we're actually to the point yet where everyone can minify on their own. Sometimes people need a hosted minified version for one reason or another. Maybe it could be moved to GitHub Pages and an extra step in the release task, but I feel that decision would have to be @toji's as he pretty much is in charge of releases.

If you have done any serious 3D programming, transforming Vec3 vertices by a Mat44 is one of the most common operations and should be optimised if possible

Agree 150%. I only wanted to point out that implementing it could segue into tons of additional rarely-used functions, and that by the nature of the math it would really imply maintaining a minimum of two variants, one for vec3 and another for vec4.

Don't get me wrong -- it could be nice to have those rarely-used functions ready for when they actually are needed, so I wasn't voicing an opinion for or against, only bringing discussion about it to the table.

I hate to say it but this thread reminds me of the phrase "Too many cooks spoil the broth".

I think the discussion is healthy. It's always good to hear arguments other than our own. When it gets bad is when we stop listening to each other, and I don't think this thread or this project has degenerated to anywhere near that point. I think we're still in the "healthy discussion" stage.

I can't help but think this whole thing has started to become really over designed

Well, it's a feature discussion, not a specification, meaning that not everything discussed here is going to become part of the library. If you ignore the above conversations and look solely at what's actually being committed, personally I have a good feeling about the architecture and direction of the project. That we're spending more time on relatively small things like directory structure indicates to me that we don't have any bigger issues to worry about, which I find very comforting.

ioquatix commented 11 years ago

@sinisterchipmunk Super :D

525c1e21-bd67-4735-ac99-b4b0e5262290 commented 11 years ago

Question: do we even need a builder?

100% of the time I use gl-matrix I do so through a package manager like npm, component or bower.

Once you've got the library into such a system you can leverage other projects which build, compile, minify and otherwise contort the code to your own particular likings.

If people want a combined and/or minified build:

Everywhere the users who want the easy way out will possibly look to find it.

I believe we'll all find it incredibly hard to agree on a build system because it's subjective to taste and application.

Let's focus on what we all actually desire from this project:

How about populating a wishlist for v2 with concrete, immediately obvious features and aspects we believe the vast majority of us want from the project?

And then let's get to crossing them all off as quickly as humanly possible.

sinisterchipmunk commented 11 years ago

100% of the time I use gl-matrix I do so through a package manager like npm, component or bower

Very good point, and this approach may lend itself to Brandon's earlier statement about picking and choosing components (e.g. quats).

I was already considering writing a Ruby gem to wrap around gl-matrix and drop it into Rails' asset management. (Maybe it's out there already; I haven't checked.) The gem would automate the build process in the Rails world too, so that's one less place we would need a builder.

How about populating a wishlist for v2 with concrete, immediately obvious features

Another smart idea. This discussion is great and all but it's hard to track what we've actually decided. The only thing I wonder about is whether we need a centralized list, or if this issue tracker itself with a 2.0 milestone attached to each decision and maybe a reference to this thread is good enough.