-
Notifications
You must be signed in to change notification settings - Fork 131
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
noise()
generic function
#42
Comments
Hi @abernier , Thanks for your suggestion. I'm not quite sure I see the benefits outweighing the drawbacks of this extension. Here are the drawbacks I can see: As suggested noise() is very slow. It leads to a ~4x reduction in speed for the 2d case. Even just calling noise2D would be quicker:
That could be addressed to some extent by turning it into a simple switch case. The signature suggests that the function can handle more than 4 dimensions which isn't true. Finally it would add multiple ways to achieve the same goal. What are the benefits and drawbacks you see to this change? |
Yeah, I didn't even considered it on the performance point of vue... It was more like a kind of optional "smart"/"magic" call, obviously slower then. I understand your concerns, please just ignore then ;) |
Just to be clear, performance really isn't the be all end all of this discussion. Performance can be improved by writing stupid enough code for jit to understand: function fasterNoise(x,y,z,w) {
if(x === undefined) return 0;
if(y === undefined) return simplex.noise2D(x,x);
if(z === undefined) return simplex.noise2D(x,y);
if(w === undefined) return simplex.noise3D(x,y,z);
return simplex.noise4D(x,y,z,w);
}
I'll leave this issue open for a while for others to weigh in as well. Just because I'm not a fan doesn't mean this isn't something other users would like to see. :) |
I think this is better left up to the end-user. Not on the basis of performance, but code clarity. Resulting code becomes ambiguous as to its purpose when you abstract away the interface. With that said, though not tested, a slight improvement to jwagner's implementation may be to encapsulate the various noise functions: const MagicNoise = simplex => {
const { noise2D, noise3D, noise4D } = simplex;
return (x, y, z, w) => {
if (x === undefined) return 0;
if (y === undefined) return noise2D(x, x);
if (z === undefined) return noise2D(x, y);
if (w === undefined) return noise3D(x, y, z);
return noise4D(x, y, z, w);
};
};
// usage
const noise = MagicNoise(new SimplexNoise('seed'));
noise(1,1); |
what about a "generic"
noise
function, with variable-length arguments, ie:noise()
noise2D(0, 0)
0
by defaultnoise(1)
noise2D(1,1)
noise(1,2)
noise2D(1,2)
noise(1,2,3)
noise3D(1,2,3)
noise(1,2,3,4)
noise4D(1,2,3,4)
noise(1,2,3,4,5)
noise4D(1,2,3,4,5)
noise4D
This is inspired from https://processing.org/reference/noise_.html
NB: I've chosen to normalise the value to
]0;1[
but maybe you prefer staying]-1;1[
working demo: https://codepen.io/abernier/pen/ExvqNyj
The text was updated successfully, but these errors were encountered: