-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Experiment: flatter strands API #8314
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev-2.0
Are you sure you want to change the base?
Conversation
|
Thanks for this @davepagurek ! A few thoughts:
After fiddling with the sketch a little, I'm wondering if there's a more p5-feeling alternative for assigning getFinalColor.begin()
let myNewColor = mix(
[1, 1, 1, 1],
getFinalColor.color,
abs(dot(myNormal, [0, 0, 1]))
);
getFinalColor.set(myNewColor);
getFinalColor.end()Is one idea, what do you think?
Not sure I understand this, is the code below interpreting the alias idea correctly? baseMaterialShader().modify(() => {
let myNormal = sharedVec3()
pixelInputs.begin()
myNormal = pixelInputs.normal
pixelInputs.end()
finalColor.begin()
finalColor.result = mix(
[1, 1, 1, 1],
finalColor.color,
abs(dot(myNormal, [0, 0, 1]))
);
finalColor.end()
});If yes, then I support it, because it more closely matches the begin/end pattern on framebuffer.
Same here, is the code below interpreting the alias idea correctly? let myNormal = sharedVec3()
getPixelInputs.begin()
myNormal = inputs.normal // is this what you meant?
getPixelInputs.end() // if so, is begin/end here this needed?
getFinalColor.begin()
getFinalColor.result = mix(
[1, 1, 1, 1],
inputs.color, // does this make sense too or no?
abs(dot(myNormal, [0, 0, 1]))
);
getFinalColor.end()
});Putting it together, it seems much more p5-like: let myNormal = sharedVec3()
pixelInputs.begin()
myNormal = inputs.normal
pixelInputs.end()
finalColor.begin()
let myNewColor = mix(
[1, 1, 1, 1],
inputs.color,
abs(dot(myNormal, [0, 0, 1]))
);
finalColor.set(myNewColor);
finalColor.end() |
Returning resultsI like the Renaming
|
|
Thanks @davepagurek ! Based on the discord chat, here is another potential idea which assumes that we wouldn't want to support interleaving or nesting of the hooks, and that there's just one hook at a time, and that aside from hook definitions, there's maybe some declarations in the beginning but no other code. Starting with this begin/end example: baseMaterialShader().modify(() => {
let myNormal = sharedVec3()
pixelInputs.begin()
myNormal = inputs.normal
pixelInputs.end()
finalColor.begin()
let myNewColor = mix(
[1, 1, 1, 1],
inputs.color,
abs(dot(myNormal, [0, 0, 1]))
);
finalColor.set(myNewColor);
finalColor.end()
});What about something like: baseMaterialShader().modify(() => {
let myNormal = sharedVec3()
Strands.hookMode(PIXEL_INPUTS);
myNormal = Strands.inputs.normal // Potential for confusion?
Strands.hookMode(FINAL_COLOR);
// instead of begin/end, it's a mode? but it's basically both end and begin?
let myNewColor = mix(
[1, 1, 1, 1],
inputs.color,
abs(dot(myNormal, [0, 0, 1]))
);
Strands.finalColor.set(myNewColor);
});In the example above I was also thinking about "Is inputs too common of a name / would this be clashing with variables users declare?" and wondering if there's value to a |
|
I think using a mode does make the changes in Looking back at one of the original inspirations, the Luma Gaussian Splats API (see the Custom Shaders section here https://lumalabs.ai/luma-web-library), that still feels a lot clearer to me, but maybe also because it's so upfront about the fact that it's just letting you modify little bits of a shader. It feels like when you're juggling inputs from multiple parts of the shader, having that control flow be more explicit helps with readability? But then for other cases where you're not doing that (e.g. a filter shader where you're always filling out only the |
This partially addresses #7994 and #7992.
The core issue is that a single p5.strands shader involves two callback functions, which is kind of a lot. The outermost one is there because we need to isolate the code that we need to transpile, so that isn't immediately going away (a short term option could be to let people load that from a file rather than a function; a longer term option could be to transpile the whole file via a custom script tag type.) This PR doesn't address that outer one. In this I'm trying to chip away at the inner callback that currently is used for each hook.
Changes:
In general, instead of doing a callback for a hook, you can use begin/end. e.g.:
Live:
https://editor.p5js.org/davepagurek/sketches/oTsFO63lk
Both forms still work for backwards compatibility.
The rules currently are this:
.begin()/.end()block.resultproperty of the hookSome other potential thoughts that we could try out:
getPixelnputs.normal, should we make a globalinputsthat aliases the hook within itsbegin/endso you could writeinputs.normal?get*-prefixed hooks, likegetPixelInputs, topixelInputsso it reads more clearly in this form?getColorhook has two arguments,inputsandcanvasContent, the properties ofinputsaren't directly accessible, e.g. you'd have to dogetColor.inputs.texCoordrather than justgetColor.texCoord. So maybe we'd need to update the rules to give direct access to properties when there's only one object input? (I just don't want name clashes if someone makes a hook that takes two object arguments of the same type.)PR Checklist
npm run lintpasses