Add GLSL-based noise(vec2) function to p5.strands (#7897)#7921
Add GLSL-based noise(vec2) function to p5.strands (#7897)#7921perminder-17 merged 10 commits intoprocessing:dev-2.0from
Conversation
- Created src/webgl/shaders/functions/noise.glsl.js - Provides a simple fractal noise function using 3 octaves
|
Looks good to me :D |
|
Thanks for tackling this! Are you able to show a test sketch using the noise so we can take a look at what it looks like? Either by making a test sketch in |
src/webgl/ShaderGenerator.js
Outdated
| } | ||
|
|
||
|
|
||
| GLOBAL_SHADER.output.fragmentDeclarations.add(noiseGLSL); |
There was a problem hiding this comment.
I think this might not be defined yet, as we only set GLOBAL_SHADER = this in the ShaderGenerator constructor. Maybe rather than adding noise to the list of glsl functions, we could do a custom function for it like you did earlier for lerp, and in that custom function, you do GLOBAL_SHADER.output.fragmentDeclarations.add(noiseGLSL)? (And possibly add it to the vertex declarations too, it could be used in updating positions as well.)
There was a problem hiding this comment.
Thanks @davepagurek! That makes sense. I’ve moved the noiseGLSL injection into a custom fn.noise function wrapper, similar to what was done for lerp. That way it’s only added during shader generation and avoids undefined GLOBAL_SHADER.
Also working on a test sketch under src/preview/global/sketch.js as suggested. Appreciate the review!
perminder-17
left a comment
There was a problem hiding this comment.
Great work on this so far, just some minor thoughts and would be good to.
| @@ -0,0 +1,15 @@ | |||
| export default /* glsl */ ` | |||
There was a problem hiding this comment.
Hi @LalitNarayanYadav, when you put a .js file inside src/webgl/shaders/ that folder is treated as “raw-GLSL only” during the p5.js build because of the string plugin:
Lines 19 to 20 in 257227f
So the bundler thinks your .js file is just a literal string, not executable JavaScript. "Probably you could get errors saying export is not defined" or maybe something like that.
Simple fix would be :
-
Keep JavaScript out of that folder i.e. don't place it inside
src/webgl/shadersOR, -
Rename your helper to something like noiseGLSL.glsl and import it the same way; it already just contains GLSL text. ( I think this one could be good, what you think)?
There was a problem hiding this comment.
Thanks a lot for pointing that out @perminder-17 and @davepagurek ! I've renamed the file to noiseGLSL.glsl and updated the import accordingly to avoid the Rollup plugin conflict. Let me know if anything else needs adjusting!
src/webgl/ShaderGenerator.js
Outdated
| } | ||
| }; | ||
| fn.noise = function (...args) { | ||
| if (GLOBAL_SHADER?.isGenerating) { |
There was a problem hiding this comment.
Hi @LalitNarayanYadav a very small suggestion, we are on the last bits now.
What do you think about having an early return guard. Like rather than doing
if (GLOBAL_SHADER?.isGenerating) {
// your code ....
} else {
// friendly error message
}Use an early-return guard inside the function.
Something like :
fn.noise = function (...args) {
if (!GLOBAL_SHADER?.isGenerating) {
// friendly-error messages
return;
}
// Now your code here without else block:
}There was a problem hiding this comment.
Thanks @perminder-17, great suggestion! I've refactored the function to use an early return guard as you described. Looks much cleaner now.
perminder-17
left a comment
There was a problem hiding this comment.
here's the result looks so far : https://editor.p5js.org/aman12345/sketches/83SNq9iF2 which I think looks nice. Maybe we should merge it if creating a functions directory makes sense @davepagurek ?
| @@ -0,0 +1,13 @@ | |||
| float baseNoise(vec2 st) { | |||
There was a problem hiding this comment.
Are we going to have more such functions in the future like noise? I am not sure if we should create a functions directory for it? @davepagurek What's your thought on this one?
There was a problem hiding this comment.
off the top of my head I think we were considering at least random(), so I think we can keep this folder structure for that.
| @@ -0,0 +1,13 @@ | |||
| float baseNoise(vec2 st) { | |||
There was a problem hiding this comment.
Hi @LalitNarayanYadav while I tested it locally, I found that the things are little jittery and we want the noise to be smoother, maybe we just need a different basis for each octave.
e.g. something like the examples here, but, maybe finding where they came from to double check the licenses, or doing something similar but different https://gist.github.com/patriciogonzalezvivo/670c22f3966e662d2f83
src/webgl/ShaderGenerator.js
Outdated
| fn.noise = function (...args) { | ||
| if (!GLOBAL_SHADER?.isGenerating) { | ||
| p5._friendlyError( | ||
| `It looks like you've called noise() outside of a shader's modify() function.` |
There was a problem hiding this comment.
noise is also a regular p5 function, so in this case we can call the original implementation, similar to how you did lerp.
|
Thanks @perminder-17 and @davepagurek for the feedback! I've updated the GLSL noise to use a smoother implementation based on this MIT-licensed source, and added fallback support for core |
| ], | ||
| 'sqrt': { args: ['genType'], returnType: 'genType', isp5Function: true}, | ||
| 'step': { args: ['genType', 'genType'], returnType: 'genType', isp5Function: false}, | ||
| 'noise': { args: ['vec2'], returnType: 'float', isp5Function: false }, |
There was a problem hiding this comment.
I think in the glsl code, we have a baseNoise() function returning float. So, either you have to rename at the js file or in the glsl file. Probably changing from baseNoise() to noise() in the glsl is what I would prefer.
Summary
This PR implements a
noise(vec2)function inp5.strandsto enable shader-compatible noise effects. It addresses #7897, which is part of the broader effort to bridge core p5.js functions with p5.strands (#7849).Changes Made
src/webgl/shaders/functions/noise.glsl.jswith a basic fractal noise implementation (3 octaves).noise(vec2)inbuiltInGLSLFunctionsinShaderGenerator.jswithisp5Function: false.GLOBAL_SHADER.output.fragmentDeclarations.add(noiseGLSL).PR Checklist
npm run lintpasses