docs: add documentation for mix() method in p5.strands#8463
Conversation
|
🎉 Thanks for opening this pull request! For guidance on contributing, check out our contributor guidelines and other resources for contributors! Thank You! |
| * let gradient = pixelInputs.uv.x; | ||
| * let color1 = vec3(1, 0.5, 0); // Orange | ||
| * let color2 = vec3(0.5, 0, 1); // Purple | ||
| * pixelInputs.color = vec4(mix(color1, color2, gradient), 1); |
There was a problem hiding this comment.
Heads up, you may need to set pixelInputs.ambientColor = pixelInputs.color.rgb to avoid the default color still being used for ambient light.
Separately, we may want to open an issue to decide how to deal with that, since its somewhat unintuitive and a pretty common use case.
There was a problem hiding this comment.
Heads up, you may need to set
pixelInputs.ambientColor = pixelInputs.color.rgbto avoid the default color still being used for ambient light.Separately, we may want to open an issue to decide how to deal with that, since its somewhat unintuitive and a pretty common use case.
Agreed, it does seem unintuitive that ambient color doesn't automatically follow the material color. Opening a separate issue would be a good idea. I will open one soon .
|
I think this generally looks good, would you mind testing the examples in the p5.js-website repo (see instructions here https://github.com/processing/p5.js-website/blob/main/docs/scripts.md#testing-the-docs-of-your-fork) and showing screenshots of each? This also helps ensure they run correctly. |
Anshumancanrock
force-pushed
the
mix
branch
2 times, most recently
from
b1f7b3e to
fb9dd40
Compare
|
@davepagurek Thanks for the heads up! I've updated examples to use |
|
@davepagurek I tested the examples locally on the p5.js-website by building the reference from my fork's branch. The mix() page renders correctly with all three examples. Here is the video: ps51.mp4The description, syntax, parameters, and all three examples (basic color mix, position-based mix, gradient mix) show up as expected. Please let me know if you would like any further changes. Thank you :) |
| * @param {Number|p5.Vector} x first value to interpolate from. | ||
| * @param {Number|p5.Vector} y second value to interpolate to. | ||
| * @param {Number|Boolean} a interpolation amount (0.0-1.0) or boolean selector. | ||
| * @return {Number|p5.Vector} interpolated value. |
There was a problem hiding this comment.
Tested it locally, everything looks nice! I am merging this, but I have one concern on the return value.
mix() internally returns a StrandsNode, not a raw Number or [p5.Vector]. However, introducing a new StrandNode type in the docs might be confusing for beginners. I'm not sure whether using Number | p5.Vector is the right approach here, or if there's a better way to document this that stays accurate while still being user-friendly.
From my point of view, I am okay with keeping the return value as {Number|p5.Vector}.
CC: @davepagurek @ksen0 any thoughts on this?
There was a problem hiding this comment.
So far we've been avoiding adding actual types here in curly braces and have just been describing the objects like "a three-dimensional vector" etc. We maybe would want to do a similar update here for consistency, but it might make sense later to make some types in the docs for p5.strands, but yeah, it's a little confusing too -- they'd maybe warrant having their own docs pages to describe what you can do with them.
If we do another pass on docs for the types here, we should possibly also check the .d.ts generator script. Currently I think they manually add all the glsl functions from an array of them, but now that we have them in the jsdoc too, we'll likely be double-exporting types for them. Might need to do a .filter(...) on the array of GLSL functions in the .d.ts generator so that we ignore ones we've manually documented?
Continuous ReleaseCDN linkPublished PackagesCommit hash: c2596f1 Previous deploymentsThis is an automated message. |
3 participants
Resolves #8441
Changes:
Added documentation for the
mix()method in p5.strands. This GLSL built-in function was already implemented but missing from the reference documentation.The documentation includes:
PR Checklist
npm run lintpasses