Open abernier opened 2 years ago
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:
noise2D: 50,350,911 ops/sec ±0%
noise4D: 21,945,180 ops/sec ±0%
That could be addressed to some extent by turning it into a simple switch case. It also makes optimizations for bundlers more difficult because it's harder to determine which code is unused. At least for now this is a minor issue since afaik no bundler/minimizer performs dead code elimination on a method level.
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);
}
noise: 13,020,651 ops/sec ±0%
fasterNoise: 50,688,165 ops/sec ±0%
noise2D: 50,697,667 ops/sec ±0%
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