RandomGamingDev
commented
1 year ago Maybe we could try using this or something similar: https://mathjs.org/
Open RandomGamingDev opened 1 year ago
RandomGamingDev
commented
1 year ago Maybe we could try using this or something similar: https://mathjs.org/
nickmcintyre
commented
1 year ago @RandomGamingDev thanks for starting the discussion.
The current p5.js math component is heavily cluttered and isn't nearly as optimized as it could be which makes it hard to not only use if you're trying to understand and interact with the code, but also to contribute to, especially for a library designed to increase accessibility to beginners.
I recently read the source code of src/math and didn't get the impression it was "cluttered". Quite the opposite, most of it's a thin wrapper around the native JavaScript Math object.
Plus, it's hard to integrate p5.js with other math libraries when you need any math that's more complex, for instance if someone wanted to do something that involved linear algebra, or calculus that isn't included in p5.js, like 4d scenes. Not only would using another library mean a better documented and managed library with more features for p5.js, but it'll also be more performant, and we could choose a library that can utilize the GPU for especially heavy operations.
I wrote a simple sketch with Math.js and found it approachable. Skimming some issues, it doesn't look like Math.js uses hardware acceleration. They did mention stdlib's ndarray as a possible route forward. You'd have to ask @davepagurek how either approach might benefit internal calculations for WebGL.
As for users, I saw you mentioned #5210 in Discord. @golanlevin made a good suggestion to start with an addon library and examples. He also mentioned my library número. It might provide some inspiration for how to make matrices play nicely with p5.Vector. I actually used Math.js in an earlier iteration–happy to discuss that in a separate thread.
Personally, I'd study how three.js approaches the math section of their API for enhancements that could increase access. I used their Raycaster in a recent project and it's great. The implementation depends on their Ray object.
@limzykenneth, @ericnlchen, @ChihYungChang, @bsubbaraman, @albertomancia, @JazerUCSB, @tedkmburu, @perminder-17, @Obi-Engine10, @jeanetteandrews, and @GregStanton may have some thoughts here, too.
RandomGamingDev
commented
1 year ago I recently read the source code of
src/mathand didn't get the impression it was "cluttered". Quite the opposite, most of it's a thin wrapper around the native JavaScript Math object.
While I agree that most of the library's fine there are major parts that I do believe are cluttered and could use some improvement.
p5.Vector.js is hard to read and has a lot of points that could really use improvement in how it's coded, and isn't expandable or collapsible to smaller or larger vectors. This leads to annoying to deal with vectors, and a repetitive codebase with code segments like this.
Then there's also p5.Matrix.js which for some reason isn't in the math section, is even more cluttered, not very expandable since it only has options for mat3 & mat4, the documentation isn't exactly the best with errors like saying that it's for describing 4x4 matrices despite it supporting mat3, it doesn't have a lot of mathematical functions for certain operations like a functions for getting the ref (Row echelon form) & rref (Reduced row-echelon form) form of matrices, and has code using it using p5.Matrix.set() for every element in the matrix inside and outside of the library which led to more clutter in things like p5.Camera.js which is probably from the same system being used everywhere else in the library.
While I could see the system getting polished and fixed to remove this clutter, better organize everything, make everything more expandable, better document things, and add more math functions, it won't be very easy to maintain because of p5.js's size, and the fact that p5.js isn't a heavily math oriented library. I feel that the best solution, at least at the current moment, would be to replace most of the internals of p5.js's math that have been causing this, and to just add a light wrapper around a more advanced, reliable, better documented, and more powerful library, rather than reinventing the wheel.
Skimming some issues, it doesn't look like Math.js uses hardware acceleration.
No, they don't use hardware acceleration, but I'm not really sure p5.js needs hardware acceleration rn, and I thought that recommending math.js could make for a good starting point.
I wrote a simple sketch with Math.js and found it approachable
Yes, it can be decently approachable, but it doesn't integrate very well with p5.js if you're trying to do some manipulation of p5.js's internal systems while relying on another math library (especially for more complex things like 4d scenes) and is even harder for beginners to use, understand, and learn from. I think p5.js should prioritize making its math easy to use and powerful from from the very beginning instead of relying on outside libraries or plugins, especially since this would help speed up the development of p5.js, bug testing, feature additions & enhancement, as well as make it easier for people who are new to certain topics understand them better, since documentation would likely be better with a math dedicated library.
davepagurek
commented
1 year ago Just to chime in about the additional WebGL-specific aspects of the p5.Matrix class: In addition to everything else you all have mentioned, it needs to store its data in a format that we can quickly drop into a setUniform call without needing to convert. Based on the docs for sending a uniform matrix to a shader, this means:
Float32ArrayIt looks like Math.js doesn't do typed arrays just yet? We might need to either benchmark how much of a slowdown it is to convert, or patch in typed array support (it seems like one commenter in that discussion has successfully patched in support for it for their own uses.)
In the past I've also used the glMatrix library, which is sort of on the other end of the spectrum, aiming to be an optimized library with WebGL compatibility. Another option could be to use something like that under the hood and add a p5 style API on top of it for easier use.
@nickmcintyre from making numero, maybe you have some more knowledge about its Tensorflow backend and whether that fits the format?
RandomGamingDev
commented
1 year ago Converting between formats for the small matrices used by p5.js's shaders shouldn't be too large of a problem altho yeah, using another library for performance could be good too if we want to go down the performance route, altho I don't feel like that's p5.js's main goal and that choosing an easier to use library would be better for contributors and users of the library.
nickmcintyre
commented
1 year ago @davepagurek here's a quick sketch with a simple example that creates a couple of Float32Arrays. It uses TensorFlow's linear algebra engine and WebGL backend. There's also an advanced example using dataToGPU that I don't really understand. Seemed interesting.
@RandomGamingDev just my two cents, but I think your critique would be more helpful if you identified specific problems with the codebase. Implementing a bunch of vector/matrix operations from scratch in JavaScript is a complex job. Style is subjective, but I think the contributors to p5.Vector and p5.Matrix did solid work. You might skim three.js' math folder for comparison. Their implementations have a similar feel. They also manage to do a lot with a few extra primitives that are implemented from scratch.
A few high-level questions come to mind: Are there specific performance issues that a third-party library would solve? Do people working on p5's core feel like any math is "missing"? Are there potential core features that are only feasible with a more powerful toolkit?
GregStanton
commented
1 year ago Thanks for tagging me @nickmcintyre.
I think it would help if we could answer the questions @nickmcintyre posed in terms of concrete user stories, or "contributor stories" in the case of the codebase. For now, I'll share my current thinking, although I'm not really sure about any of this!
For some of my p5 sketches, I've written numerical integrators, I've used math.js to parse algebraic expressions, and I've used a different outside library for nonlinear regression:
These types of sketches require a lot of foundational code (e.g. for a coordinate system that conforms to mathematical conventions, for axes, for drawing arrows, etc.). So, I recently started a p5 add-on library called Mathemagical.js. It will likely contain a lot of the computational power under discussion here.
"Any math that's more complex" (to quote @RandomGamingDev) is an immense scope. Exposing this kind of functionality to the p5 user would mean a big addition to the API, so it should have a compelling justification. Once something goes into the API, we're basically stuck with it.
Adding something the size of the math.js function reference would probably double the size of the API, at least. Most of it will be incomprehensible to beginners who are interested in creative coding, rather than math, and they may feel intimidated.
Regarding additional math features, @RandomGamingDev noted that "it won't be very easy to maintain because of p5.js's size, and the fact that p5.js isn't a heavily math oriented library." The argument, as I understood it, is that p5.js should therefore trust another project to develop and maintain the math functionality.
Trusting another project with a large portion of the p5 API seems risky, especially right now. As far as I know, the project lead of math.js works on it in his spare time and there's not a larger organization backing it, in case he has to step out. More generally, WebGPU seems likely to shake up the technical computing landscape.
Wrapping a dependency using the facade pattern or adapter pattern may mitigate the risk somewhat, but we may still end up being tied to the structure of whatever dependency we add.
Another option is to depend on, and expose, only a small subset of features from a library like math.js. The only obvious way to narrow it down is to consider the math p5 already uses, e.g. p5.Matrix. For that, it probably makes sense to borrow from a library that's more specifically geared toward the purposes of p5, such as glMatrix or the three.js math folder, which others have mentioned. Maybe those libraries could inspire performance improvements or refactoring. Here are a few other projects that may help if p5.Matrix needs improvement (some have already been mentioned):
It probably makes sense to relegate any large addition of math features to an add-on library like Mathemagical.js or número. For example, this may include numerical analysis, linear algebra, geometry, optimization, probability, and statistics. Since these features would be core to the purpose of the project, they'll attract users rather than intimidate them. Those same users can become contributors, so developing or maintaining extra math features will be less of a stretch.
Math visualizations are another consideration. It seems likely that users of a graphics library with advanced math features will want to make math visualizations. But in p5, even something as simple as axes with tick marks feels out of scope. If p5 goes beyond graphical primitives and starts providing domain-specific visualizations (math or otherwise), the scope becomes unbounded. (Composite objects like axes also work better with an object-based interface, rather than a function-based interface like p5's. In Mathemagical, we provide both types of interfaces. Matplotlib is an example of a library from the Python ecosystem that does the same thing. The complexity is forced by the use case, but p5 works without this.)
Edit: Made reply more concise, fixed a link.
RandomGamingDev
commented
1 year ago @RandomGamingDev just my two cents, but I think your critique would be more helpful if you identified specific problems with the codebase. Implementing a bunch of vector/matrix operations from scratch in JavaScript is a complex job. Style is subjective, but I think the contributors to p5.Vector and p5.Matrix did solid work. You might skim three.js' math folder for comparison. Their implementations have a similar feel. They also manage to do a lot with a few extra primitives that are implemented from scratch.
I did identify specific problems in https://github.com/processing/p5.js/issues/6527#issuecomment-1793477773 and in the post itself, where I detailed the problems with the current math system. While I agree that the job is complex and that a lot of the job is done quite solidly, I don't believe that all of it's solid, or that a lot of the things added are a matter of just style. For instance, while three.js's math libraries and p5.js's math libraries have a similar feel, there are a lot of points where p5.js could stand to be more like three.js. Let's show a few examples from just the Matrix libraries since they're the most tedious to deal with and the most apparent (a lot of other parts have these same issues too):
p5.Matrix.js isn't in the math section, but rather the p5.js section
Then there's also p5.Matrix.js which for some reason isn't in the math section, is even more cluttered, not very expandable since it only has options for mat3 & mat4, the documentation isn't exactly the best with errors like saying that it's for describing 4x4 matrices despite it supporting mat3
.transpose()
Three.js version: The transpose function here, although you could say that it looks cluttered, actually isn't that cluttered especially since it's pretty easy to take a look at the code and instantly tell that lines that look like this:
tmp = te[ 1 ]; te[ 1 ] = te[ 4 ]; te[ 4 ] = tmp;just swap different numbers in the matrix, and then to tell that they're getting swapped over the diagonal, with clear naming, and a nice clean split between 2 groups of 3 lines of swaps to signify which parts of the matrices they're operating on. It also always works for the matrix it's operating on, which isn't true for p5.Matrix.js.
p5.js version:
For some reason this works only when doing it on another matrix and specifically flipping it and then copying it over to this matrix and not on just this matrix I think that just copying the data over and then swapping is cleaner, but shouldn't there be an option to do it for this specific matrix rather than just being forced to do mat.transpose(mat)? For some reason, there are variables allocated like this despite them being used nowhere else and including every other part of the writing to this.mat4 which don't use the variables which just directly use a.mat4[<foo>]:
let a01, a02, a03, a12, a13, a23;Then there's also the major, and overarching issue that's constantly present in a lot of p5.js's code, which is repetition, where there absolutely shouldn't be repetition since a whole 23 lines which could just be replaced by adding something like:
const aMat4 = a instanceof p5.Matrix ? a.mat4 : a;and then using the just the part after the else with the variable aMat4. The coding practices used within the math library and that's spreading out of it is I believe, unmaintainable, and even if it's usable for the user, it makes the experience of coding for the library a lot more tedious. Not only that, but this doesn't even work for all p5.Matrix.js matrices since, as it turns out, although p5.Matrix.js is documented to be "A class to describe a 4×4 matrix" it also supports mat3 for some reason, which this code cannot account for, makes a lot harder to track down as a bug, and if we continue using the current coding style for it, we'll also get a massive function, massive class, and extra checks. Then there's also the expandability and collapsibility issue I talked about again, where not only is this sorta code harder to transfer over to say a 5x5 matrix where it'd be completely unrealistic to repeat massive code segments like that. I get writing down the formula for efficiency and so that you can't easily read it, but there's a major problem when that formula is 23+ lines large, will be constantly repeated, and will be even larger in the future.
This type of repetitive code is not only within p5.Matrix.js, but has also spread to things using p5.Matrix.js like this part in p5.Camera.js where set is used for, yet again, each individual number in the matrix. This is also despite there being this:
if (inMatrix instanceof p5.Matrix) {
this.mat4 = inMatrix.mat4;
return this;and p5.Matrix.copy() which would work together just find for copying the values of a matrix as well as using a for loop or = [...toCopy].
This type of code is not only clearly apparent, and clearly full of bad practices rather than just being a certain style, but it's also spreading through p5.js and will likely continue to spread unless we clean up the code somehow, which is why I'm proposing that we do so by replacing the math library with a library with contributors that can add extra features in a clean and simple to use API for p5.js that can then be wrapped around with a custom p5.js API so that p5.js can safely swap between libraries and make the features of the underlying math library easily accessible to people who want to use them without having to add addons that quite likely won't work very well with p5.js's internals
A few high-level questions come to mind: Are there specific performance issues that a third-party library would solve? Do people working on p5's core feel like any math is "missing"? Are there potential core features that are only feasible with a more powerful toolkit?
I probably should've clarified, but by optimization I mainly meant optimization of the coding experience and workflow as well as optimization in terms of performance since I don't believe that performance is currently p5.js's main concern. However, I believe that there will be a significant performance increase from using a system that doesn't have the same problems as shown before with the p5.js codes and things like tons of large type checks and other checks. While I don't believe any truly core math is missing, I believe that we should make and effort to expand past that and provide the best experience possible instead of just the core because the core of p5.js is making things beginner friendly and easily accessible, which means that we should be making these more accessible. For instance, differently sized matrices and better more intuitive matrix types and operations (there are a lot of functions that apply only to mat4 when mat3 appears to be supported at least partially). While you could argue that these aren't that bad and that all of the core features needed to make programs are there, for which you'd be right, implementing a lot of these features without another library which again would likely have terrible compatibility with p5.js's current system (the compatibility would also have to have you have a decently deep understanding of p5.js's math classes,) would require a lot of work, work that I don't believe would be very easy or even possible for beginners.
"Any math that's more complex" (to quote @RandomGamingDev) is an immense scope. Exposing this kind of functionality to the p5 user would mean a big addition to the API, so it should have a compelling justification. Once something goes into the API, we're basically stuck with it. Adding something the size of the math.js function reference would probably double the size of the API, at least. Most of it will be incomprehensible to beginners who are interested in creative coding, rather than math, and they may feel intimidated.
Another option is to depend on, and expose, only a small subset of features from a library like math.js. The only obvious way to narrow it down is to consider the math p5 already uses, e.g. p5.Matrix. For that, it probably makes sense to borrow from a library that's more specifically geared toward the purposes of p5, such as glMatrix or the three.js math folder, which others have mentioned. Maybe those libraries could inspire performance improvements or refactoring. Here are a few other projects that may help if p5.Matrix needs improvement (some have already been mentioned):
It probably makes sense to relegate any large addition of math features to an add-on library like Mathemagical.js or número. For example, this may include numerical analysis, linear algebra, geometry, optimization, probability, and statistics. Since these features would be core to the purpose of the project, they'll attract users rather than intimidate them. Those same users can become contributors, so developing or maintaining extra math features will be less of a stretch.
First off, I believe that you took my quote out of context since the full quote's "Plus, it's hard to integrate p5.js with other math libraries when you need any math that's more complex, for instance if someone wanted to do something that involved linear algebra, or calculus that isn't included in p5.js, like 4d scenes." Also, I wasn't saying to add every other bit of math that could possibly be needed to the p5.js's math API. To be more specific, I think that we should have a system where p5.js's current wrapper is kept (like what you're suggesting), but the internals are replaced with a better math library in a way that makes it a lot easier to switch between math libraries if needed, and that makes it a lot easier to expand by wrapping over the math library's function's with p5.js style coding. Also, even if we were to expand the API to wrap over the entire math library, I think that those who are interested in creative coding would either find it interesting, useful, encouragement to use this library over others that don't provide these things, or simply skip over them rather than being scared away from the intuitive API of p5.js by a few math functions grouped under a math part of the library. Plus, I feel that adding more math functions to the API to make it easier for people fits perfectly with p5.js's goals for ease of use, especially for beginners.
Regarding additional math features, @RandomGamingDev noted that "it won't be very easy to maintain because of p5.js's size, and the fact that p5.js isn't a heavily math oriented library." The argument, as I understood it, is that p5.js should therefore trust another project to develop and maintain the math functionality.
Yes, the argument is that p5.js should trust another project's math functionality and use it within itself.
Trusting another project with a large portion of the p5 API seems risky, especially right now. As far as I know, the project lead of math.js works on it in his spare time and there's not a larger organization backing it, in case he has to step out. More generally, WebGPU seems likely to shake up the technical computing landscape. Wrapping a dependency using the facade pattern or adapter pattern may mitigate the risk somewhat, but we may still end up being tied to the structure of whatever dependency we add.
Wrapping it should mitigate the risk enough for it to be worthwhile considering how much easier it'll be to switch between math libraries and the potential benefits.
RandomGamingDev
commented
1 year ago Just another note: There are also multiple instances of every index of an array being manually indexed. One example's here where every index of the arguments is manually indexed to be written to manually indexed indexes in the matrix for 16 whole lines instead of just using a for loop like this:
for (let i = 0; i < 16; i++)
this.mat4[i] = arguments[i];or the even simpler approach:
this.mat4 = [...arguments];
lindapaiste
commented
11 months ago @RandomGamingDev has raised a lot of excellent points in this thread. IMO we should rethink the design of the Matrix object, regardless of whether we choose to implement the core mathematics ourselves or rely on a third party.
The current Matrix class is a collection of specific use-case functions. There is not a lot of thought into what a Matrix object is and what methods and properties it provides. This leads to confusing and overly-complex code upstream in classes like Camera which use Matrix instances. I would love to see a clearer interface.
Most of the methods on the Matrix class are only applicable to some Matrix objects, which is a sign of bad design. A good place to start detangling would be to separate this into three objects: a core Matrix (or SquareMatrix, since our internal uses only need to support nxn shapes) and a Matrix4x4 and Matrix3x3 class which extend the common core and add their own specific methods.
When looking at the current functions of the Matrix class, we need to ask:
this? (ortho and perspective do not use any and therefore should be static methods or external utility functions)i inside of a loop? The diagonal method is a good candidate for this.A note on performance regarding that last point. The hard-coded approach is faster at runtime since there's no for loops or i+2 math for the browser to execute. However it leads to a less maintainable codebase (and a larger bundle size) so that's something that we need to weigh the pros and cons of. My suggestions prioritize clean code over runtime performance.
Looking at the current Matrix class, here's how the methods break down:
4x4 only
set [1]get [2]static identitytranspose [7]invertdeterminant [3]multscalerotate (and rotateX, rotateY, rotateZ)translateperspective [4]ortho [4]multiplyVec4multiplyPointmultiplyAndNormalizePointmultiplyDirectioncreateSubMatrix3x33x3 only
invert3x3transpose3x3inverseTranspose [5]mult3x3column [6]row [6]multiplyVec3both
copy [2]diagonal[1] The entire matrix is replaced by set, so this should be a static method.
[2] get and copy both do the same thing.
[3] determinant appears to be unused. There an internal calculation of a determinant inside invert, but it doesn't use this.
[4] As previously mentioned, perspective and ortho are factory functions which create a new 4x4 matrix and don't make sense as instance methods.
[5] inverseTranspose is a method of a 3x3 matrix which takes a 4x4 matrix as an argument. It seems like it doesn't use the original 3x3 matrix at all, and should instead be a method on the 4x4 instance with no arguments. That is, instead of calling this.renderer.uNMatrix.inverseTranspose(this.renderer.uMVMatrix); it would be this.renderer.uNMatrix = this.renderer.uMVMatrix.inverseTranspose();.
[6] I would love to see column and row be generalized, as they can have a lot of uses in the internal logic of other methods.
[7] Why does transpose take an argument? It should be an operation that creates and returns the transpose of the current matrix.
davepagurek
commented
11 months ago Thanks for taking an interest in this @lindapaiste!
I think the first big decision to make is whether we default to having all methods mutate the current matrix or whether all methods should all return new objects by default.
Points for mutation:
p5.Vector, so that could be good for newcomersPoints for returning new objects:
[1] The entire matrix is replaced by set, so this should be a static method.
I think this could still make sense as an instance method if we choose to keep matrix methods mutating the current object, similar to p5.Vector.prototype.set.
[5] inverseTranspose is a method of a 3x3 matrix which takes a 4x4 matrix as an argument. It seems like it doesn't use the original 3x3 matrix at all, and should instead be a method on the 4x4 instance with no arguments. That is, instead of calling this.renderer.uNMatrix.inverseTranspose(this.renderer.uMVMatrix); it would be this.renderer.uNMatrix = this.renderer.uMVMatrix.inverseTranspose();.
I think this is an unfortunate consequence of the above point where it tries to use a mutate-first API like p5.Vector. The problem is that it goes from a 4x4 matrix to a 3x3 matrix, which would change the type of the current matrix. Instead, it ends up getting an argument, so that it can be a mutating method of a 3x3 matrix.
If we decide to stick with an API based on mutation, maybe we can introduce a naming convention for methods that return new objects, like makeInverseTranspose(), that returns a brand new 3x3 matrix?
Alternatively, we could make all methods return new objects, similar to the native DOMMatrixReadOnly class. That might make more memory pressure when chaining together a few operations, so maaaaybe it's a bit slower? I'm not sure that that's a big enough issue for it to affect our decision though.
limzykenneth
commented
11 months ago I just want to mention quickly that for now, we can focus on optimizing the internal implementation if necessary but not to go too far in terms of new implementation and potential breaking changes. Mainly because I have something in the works to review this part of the code base as well that will give more flexibility to explore things further if necessary.
nickmcintyre
commented
11 months ago I tend to agree with @GregStanton that we should probably relegate any significant expansion of p5.js' math API to add-on libraries. Don't get me wrong, I have a copy of Math Art on my coffee table, but most creative coders don't need anything close to Math.js or TensorFlow.js most of the time. The "you aren't gonna need it" (YAGNI) principle applies.
What do people think about adapting the three.js math folder to work with p5.js? I believe three.js has every core matrix math feature we need and then some. There may be some upsides to sharing a little DNA with that project specifically.
We started discussing code style in #6607. @lindapaiste @RandomGamingDev I'd love to hear your opinions about p5.js source code. My opinions in that discussion are limited to documentation. Past contributors volunteered their time without many of JavaScript's nice modern features or a style guide to follow. Perhaps we can chart a better course moving forward.
RandomGamingDev
commented
11 months ago I tend to agree with @GregStanton that we should probably relegate any significant expansion of p5.js' math API to add-on libraries. Don't get me wrong, I have a copy of Math Art on my coffee table, but most creative coders don't need anything close to Math.js or TensorFlow.js most of the time. The "you aren't gonna need it" (YAGNI) principle applies.
Honestly, I personally believe that the option to use a math library (assuming that we use an external one in the future) should always be a choice regardless of whether or not you need it. However, I do understand what you, @nickmcintyre & @GregStanton are talking about. That's why I agreed that in order to keep the main API decently small and easily searchable while not overcrowding everything, and also maintaining the p5.js code asethetic we should keep the current p5.js wrapper, or something close to it and then wrap that around the external math library like I said here:
I feel that the best solution, at least at the current moment, would be to replace most of the internals of p5.js's math that have been causing this, and to just add a light wrapper around a more advanced, reliable, better documented, and more powerful library, rather than reinventing the wheel.
This would solve all of our problems with the idea, like the idea that it'd be hard to approach as a beginner and might even intimidate them, and that it'd be too hard to search through. This will allow us to take the best of both sides, providing beginners with the same, or maybe make it even easier to use while providing powerful mathematical functions and interoperability for those who need the library.
p5.Math) isn't better for thisWhile some of these arguments make sense, I don't see how the current math system is any improvement over a wrapper based system around an external library for these arguments since the outer API would again be the same, not to mention the increased code quality, and thus code clarity, as well as things like better debug features and better documentation, all of which would benefit beginning developers much more than the current and much vaguer system. (p5.Matrix saying that it only supports mat4, but also having partially support for mat3 in certain areas, but not in others for instance)
What do people think about adapting the three.js math folder to work with p5.js? I believe three.js has every core matrix math feature we need and then some. There may be some upsides to sharing a little DNA with that project specifically.
While I think that it'd work ok, I don't think we'll need a good chunk of the features nearly as much since many are more so targeted towards three.js specifically rather than towards general math (whereas general math libraries can be more commonly, and be more generally used even outside of p5.js itself), since it's three.js's math library. I'm still going to stand with the idea of using an external general math library like math.js over things like keeping p5.Math or taking chunks out of three.js's math library.
lindapaiste
commented
11 months ago What do people think about adapting the three.js math folder to work with p5.js? I believe three.js has every core matrix math feature we need and then some. There may be some upsides to sharing a little DNA with that project specifically.
I'm looking at the three.js code now. They have separate classes for Matrix3 and and Matrix4 so that's nice ✔️.
Other than that it actually seems very similar to our code, including the parts that I don't like lol. For example they also have a makeOrthographic instance method that doesn't use any instance variables, and they made identity an instance method too. I'm assuming that they did this for performance reasons. I see in one of their examples that they reuse the same single tempMatrix instance in every render. So I do see the benefits of a mutation-first approach that limits new instantiations.
They also have code like this 📜 which is 20 lines that could easily be a 3-line loop. Maybe for performance?
It does feel a bit more readable overall. The intermediate assignment of const te = this.elements; helps clean things up. I personally like that they have explicit methods for fromArray, set, setFromMatrix3, setFromMatrix4, etc. but I know that's not the p5.js style. We tend to have methods with multiple override signatures.
GregStanton
commented
11 months ago Hi @RandomGamingDev! Thanks so much for your thoughtful reply. It's really nice to be able to see this issue from different sides while working toward a consensus.
There's some chance that I'm misunderstanding your proposal. For example, it sounds like you're talking about namespacing the math functions? If so, how do you see that working with the current API, where the math functions are all in the global namespace (at least in global mode).
Do you think you could post a code snippet that outlines the user-facing API you have in mind? Maybe you could just include a representative sample of the types of functions you'd like to see, and include some comments where you think other functions should probably be added. Seeing your proposal concretely like this should help us to be sure we're talking about the same thing.
GregStanton
commented
11 months ago Also @lindapaiste, yeah, I suspect the code snippet you shared from three.js is like that for performance reasons. I didn't go through it all in detail, but it seems like a good bet to me because (a) whoever wrote that code almost certainly knows about loops and probably chose not to use one for a reason, and (b) the related glMatrix library is quite popular and applies the principle "Unroll EVERYTHING" for the sake of speed. A blog post from the creator of glMatrix says there's not a single loop in the code (the post is old, but we could inspect the code to see if that's changed). So speed seems like it's probably the reason.
davepagurek
commented
11 months ago I will also mention that while I don't think we need to go so far as to avoid loops in our code, the last time I was doing a bunch of matrix code in js from scratch, I found that if I didn't try to reuse the same matrix object where possible, itd end up with stuttery animation. In a chrome profile, it seemed that while most frames were fast, every garbage collection caused a frame or two to be dropped. It still would happen occasionally even with reused matrices, but it happened much less in that one project.
That's not to say that we absolutely have to do the same thing in p5, since we might be unavoidably doing some of that in push/pop stacks? But probably worth stress testing a bit before committing to one way or another.
RandomGamingDev
commented
10 months ago @GregStanton
Do you think you could post a code snippet that outlines the user-facing API you have in mind? Maybe you could just include a representative sample of the types of functions you'd like to see, and include some comments where you think other functions should probably be added. Seeing your proposal concretely like this should help us to be sure we're talking about the same thing.
Here's a demo of what I'm thinking of with one of the possible functions (in this case the inv() function):
https://editor.p5js.org/PotatoBoy/sketches/yVCSWHMnz
There's some chance that I'm misunderstanding your proposal. For example, it sounds like you're talking about namespacing the math functions? If so, how do you see that working with the current API, where the math functions are all in the global namespace (at least in global mode).
Yes, I'm discussing namespacing the math functions in order to introduce what p5.js needs while still keeping the API small, the code in that class p5.js style, and also providing interoperability with a more powerful math library like math.js if someone needs it. I could see the functions like inv() and det() working in global mode and I think they should go in global mode just to maintain that p5.js coding style, although I think it'd be best if there were also instance methods for the same tasks.
RandomGamingDev
commented
10 months ago Also @lindapaiste, yeah, I suspect the code snippet you shared from three.js is like that for performance reasons. I didn't go through it all in detail, but it seems like a good bet to me because (a) whoever wrote that code almost certainly knows about loops and probably chose not to use one for a reason, and (b) the related glMatrix library is quite popular and applies the principle "Unroll EVERYTHING" for the sake of speed. A blog post from the creator of glMatrix says there's not a single loop in the code (the post is old, but we could inspect the code to see if that's changed). So speed seems like it's probably the reason.
Also, I'd just like to note that I'm actually fine with things being unrolled for performance reasons, however, I do believe that code quality is still the most important and that three.js did a good job maintaining that code quality. p5.js's code doesn't have that level of quality, nor is p5.js a library that really has speed as its first priority. Not to mention that p5.js's code is likely less efficient rather than more. So while I'm fine and actually in many cases endorse unrolled code for the sake of speed, I don't believe that that fits the coding clarity and priority on understandability that p5.js's doctrine is all about and the developers, whose main priority is creating a great and understandable processing library with new features, not remaking math functions already made in other libraries. That's why I think that it'd be better to leave things like the intricacies of these math functions and their design choices, whether for performance or clarity to larger, more general, more performant, and more powerful math libraries.
GregStanton
commented
9 months ago @RandomGamingDev and @lindapaiste: Thanks for taking the time to flesh out your points! I think it's worthwhile to question the idea that performance is not really a top priority relative to subjective measures of codebase readability. This may sound controversial! So I'll explain what I mean.
Performance is an accessibility issue
I'll quote @nickmcintyre, who made some compelling points on this topic:
- Optimizing performance would make sketches more accessible to people with modest devices.
- Doing so would also reduce power consumption while sketches are running.
People with less powerful devices (for example, people who can only access the p5.js Web Editor on a mobile phone) may face a variety of barriers. Not being able to run code due to performance would make that list longer. Their devices may also tend to be older, which means they may have trouble holding a charge. So power consumption also has immediate consequences on accessibility, in addition to the global considerations.
Performance and readability are not always mutually exclusive
Sometimes, readability and performance may be at odds with each other, but this needs to be determined on a case-by-case basis. For example, it's not clear to me that unrolling for loops in features related to 3x3 and 4x4 matrices makes the code harder to understand, at least not significantly. Arguably, removing a for loop makes the code more understandable in some ways, rather than less: a total beginner who hasn't learned about loops yet will still understand hard-coded iteration. Of course, I'm not arguing that we avoid loops for this reason, but in some performance-critical code, the benefits of hard-coded iteration may outweigh any downsides associated with the extra verbosity.
Performance is valued in p5.js Based on the p5.js issue forms, its website, and its contributor docs, p5's literal top priority is accessibility (accessibility actually appears at the top of these documents). As explained above, performance can contribute significantly to this top-line priority.
In practice, we don't have to look very far to see performance being emphasized in p5.js. The biggest focus right now is the p5.js 2.0 RFC that @limzykenneth just released, and it cites performance as the reason for multiple changes. It also explicitlly calls for an effort to identify performance optimizations in a new math module, which is the issue we're discussing here.
Clarifying update to the p5.js contributor guide? In short, understandability and performance both contribute to accessibility. If we can reach a consensus on this point, it may be helpful to codify it in the contributor guide. What do you all think?
RandomGamingDev
commented
9 months ago Sorry for the long wait, I've been pretty busy recently.
Performance is an accessibility issue
I'll quote @nickmcintyre, who made some compelling points on this topic:
- Optimizing performance would make sketches more accessible to people with modest devices.
- Doing so would also reduce power consumption while sketches are running.
People with less powerful devices (for example, people who can only access the p5.js Web Editor on a mobile phone) may face a variety of barriers. Not being able to run code due to performance would make that list longer. Their devices may also tend to be older, which means they may have trouble holding a charge. So power consumption also has immediate consequences on accessibility, in addition to the global considerations. Performance is valued in p5.js Based on the p5.js issue forms, its website, and its contributor docs, p5's literal top priority is accessibility (accessibility actually appears at the top of these documents). As explained above, performance can contribute significantly to this top-line priority.
In practice, we don't have to look very far to see performance being emphasized in p5.js. The biggest focus right now is the p5.js 2.0 RFC that @limzykenneth just released, and it cites performance as the reason for multiple changes. It also explicitlly calls for an effort to identify performance optimizations in a new math module, which is the issue we're discussing here.
Because of your point here I created a benchmark for calculating the projection of 1000 4d hypercubes to demonstrate what I meant by "4d scenes" as @GregStanton requested all of which were ran on Version 115.6.0esr (64-bit) of Firefox and all on the same machine under the same conditions alongside the benchmarks themselves (there's some extra non-math stuff which I will explain in a moment): https://docs.google.com/spreadsheets/d/1mFdgrF_HZ4TspuwsaE2bNa_Ke8dNIny-s9ekJ_WHg9I/edit#gid=0
This benchmark includes results for:
with:
beginShape() & `endShape())Now something that might get asked is "Why wouldn't I just do this on the GPU" and the answer is that this is supposed to be a benchmark, and also that the vertices are oftentimes needed, for instance, if we were actually using 4d projections for a project we might want this for say, collisions, which would be best done on the CPU.
Something that you might notice is that math.js is horridly ~28.6ms slower for the 1000 hypercubes aka 0.0286 milliseconds or 28600 nanoseconds slower per hypercube :scream: (so not actually that bad). However, that isn't mentioning the clearly unoptimized nature of the code through using math.js for everything, including parts it wasn't meant for, its more general nature, its features, as well as a PR I made that should bring better performance for many of the methods seen through type inferencing. Another thing that may seem confusing is the fact that I used a system where I attached an extra "w" variable to p5.Math instead of using a custom vector, but again, this sort of Frankenstein creation definitely isn't something that p5.Math isn't aware of and is used quite heavily as can be seen from some of my previous comments:
While I do agree that performance is an accessibility issue, the issue is getting made out to be worse than it actually is when the main issue is clearly the readability and manpower spent deciphering the code base, which dissuades people from contributing to the library. This all isn't even mentioning the fact that p5.Math gets absolutely beat by all the math libraries other than math.js in the benchmarks including MatrixJs, which is a dynamic matrix library I wrote made for handling general mathematical computations, not graphical computations, is written to use a wrapper which goes around a regular list for all numbers, with the list simply containing more wrappers around more lists, and all the wrappers using loops for everything and unrolling nothing. And how much does the apparently performance focused p5.Math with all of its unrolling for "performance" compare to my absolutely-not-made-for-this-nor-optimized-for-this library? My library's 148% its speed. Also, if we're going to talk about performance, we should talk about how the few hundreth's of a ms per hypercube spent by a slightly worse math library pales in comparison to how p5.js doesn't use batching, which is one of the easiest ways to drastically improve performance. Using p5.js's very own tools I was able to increase performance by ~200ms for the 1000 hypercubes aka ~0.2ms for each which should be able to be pushed far farther since p5.js's tools for batching themselves are very unoptimized! Something that I'd like to note is the fact that this issue has been ignored and deprioritized for so long, as well as the fact that the performance docs for p5.js itself suggest that p5.js does not care about performance nearly enough to consider this amount of performance an issue with accessibility.
In conclusion, p5.Math clearly demonstrates the bad design choices for not only usability, but performance that get made when developing a library "along the way" like what p5.js has been doing and plans to do again and the performance "issues," if we're going to talk about them despite every other math library other than math.js being faster, with math.js only being hundreths of a ms slower (per hypercube in this example) when using math.js for everything including what it wasn't designed for clearly apply to the math portion much less than when compared to p5's rendering.
https://github.com/pppp606/numjs/tree/fix/multiply_and_divide_argument_types
Performance and readability are not always mutually exclusive Sometimes, readability and performance may be at odds with each other, but this needs to be determined on a case-by-case basis. For example, it's not clear to me that unrolling
forloops in features related to 3x3 and 4x4 matrices makes the code harder to understand, at least not significantly. Arguably, removing aforloop makes the code more understandable in some ways, rather than less: a total beginner who hasn't learned about loops yet will still understand hard-coded iteration. Of course, I'm not arguing that we avoid loops for this reason, but in some performance-critical code, the benefits of hard-coded iteration may outweigh any downsides associated with the extra verbosity.
I never argued that performance and readability are mutually exclusive and in fact happily support code unrolling and believe that it can be quite readable like in my previous comment.
a. The benchmarks (both on Google Sheets & with p5.js sketches) clearly demonstrate that yes, there are major performance improvements.
b. While I'd love to pick a specific math library for my proposal, the issue is that every single one is a tradeoff, and the different responses make it hard to understand what to select for so here's my list:
projected.forEach((l, v, i) => v * w.getNum([i[0], 0]))although the difference in readability isn't that much once comments are added into the mix, like how for this example forEach is basically just map for the matrix.
Personally, I'd exclude math.js for now assuming that p5.js doesn't care about the math visualization side enough to make the change for them, choose tfjs (cpu backend) if p5.js wants to be extra dynamic, glMatrix if p5.js wants to be extra performant, and MatrixJs if p5.js wants something inbetween for dynamicism and performance.
If I had to guess, based off of what I've seen I think it's best to either go with glMatrix or MatrixJs.
davepagurek
commented
9 months ago If we're not going to expand the feature set of our math API, I'd support replacing internals with glMatrix calls surrounded by the friendly errors + unit conversion code we currently have as a way of providing the same features with better performance.
I don't have a very strong opinion either way in expanding the math operations to support more arbitrary matrix sizes and more linear algebra. If we opt to not do it now, and instead use something like glMatrix, how feasible would it be for an add-on library to interoperate with that format? Using a mathjs-like library now definitely keeps that door open, but do others do that too?
I'd like to note is that the fact that this issue has been ignored and deprioritized for so long as well as the performance docs for p5.js itself suggest that p5.js does not care about performance nearly enough to consider this amount of performance an issue with accessibility.
I just want to mention that not having the bandwidth to make those changes between last summer and now doesn't mean it isn't important to us, just that for now there have been lower hanging fruit given our limited resources. Although I think it's probably true in general that we don't have a super clear stance on when to prioritize performance.
RandomGamingDev
commented
9 months ago If we're not going to expand the feature set of our math API, I'd support replacing internals with glMatrix calls surrounded by the friendly errors + unit conversion code we currently have as a way of providing the same features with better performance.
That sounds perfect, although I do think that we should expand our math API so support more features.
I don't have a very strong opinion either way in expanding the math operations to support more arbitrary matrix sizes and more linear algebra. If we opt to not do it now, and instead use something like glMatrix, how feasible would it be for an add-on library to interoperate with that format? Using a mathjs-like library now definitely keeps that door open, but do others do that too?
My opinion on this is that it should be done especially considering the fact that p5.js is oftentimes used for visualizations that rely heavily on math and that would benefit heavily from that. My thoughts on using glMatrix now is that using glMatrix should provide a stable API that can be better supported as opposed to the again, frankenstein nature of the current p5.Math. For instance, instead of matrices being able to be mat3, mat4, or both at the same time with most operations supporting mat4, but not mat3, glMatrix, with its documentation and non-frankenstein nature provides a stable ground for interfacing. However, you'll most likely still have to implement the interfacing between it and a more general math library yourself.
I'd like to note is that the fact that this issue has been ignored and deprioritized for so long as well as the performance docs for p5.js itself suggest that p5.js does not care about performance nearly enough to consider this amount of performance an issue with accessibility.
I just want to mention that not having the bandwidth to make those changes between last summer and now doesn't mean it isn't important to us, just that for now there have been lower hanging fruit given our limited resources. Although I think it's probably true in general that we don't have a super clear stance on when to prioritize performance.
Don't worry, I know that resources are limited. I just meant that p5.js clearly isn't flocking over this issue declaring it "an accessibility issue" so why should we say using a slightly slower math library is one when a slightly lower math library is much less of a performance issue or "accessibility issue" relative to that? And yeah I agree, p5.js doesn't really care about performance so much as to deem that small of a change in performance an "accessibility issue".
Increasing Access
The current p5.js math component is heavily cluttered and isn't nearly as optimized as it could be which makes it hard to not only use if you're trying to understand and interact with the code, but also to contribute to, especially for a library designed to increase accessibility to beginners. Plus, it's hard to integrate p5.js with other math libraries when you need any math that's more complex, for instance if someone wanted to do something that involved linear algebra, or calculus that isn't included in p5.js, like 4d scenes. Not only would using another library mean a better documented and managed library with more features for p5.js, but it'll also be more performant, and we could choose a library that can utilize the GPU for especially heavy operations.
Most appropriate sub-area of p5.js?
Feature enhancement details
Instead of using p5.js's current math component, revamp it so that it uses another more powerful math library for its calculations.