-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
[p5.js 2.0 RFC Proposal]: Make shaders define what they get used for #6759
Comments
Hi @davepagurek , I appreciate your proposal, but I must admit I'm a bit confused, I'm sorry for any misunderstanding. I completely agree with your point about reducing the need for users to delve deep into p5.js's internal material systems. The idea of separating fill and stroke shaders makes sense to me. Rather than having a single shader for both stroke and fill, having two distinct shaders sounds like a good approach.
In this point, what is being conveyed? Is it something like for Strokes to be applied previously we need to include uniform uStrokeWeight? But after the implementation we would not have such requirment?? I apologize once again if my question seems silly. So, in a scenario where a user has lighting enabled and includes 13 attributes and uniforms in their shaders, will our shader be ignored at that point after the implementation? Can lights be handled in any cases? Mind explaning a bit on this context :) |
Hello! @perminder-17
From what I could understand, the options object allows us to explicitly specify whether a shader should apply to fills or strokes. I think creation of this object avoids the need to rely on uniforms,
I don't think the shader will be ignored at that point.. I think the main aim is to provide a more controlled way to specify where the shader should be applied at the time of creation. Their functionality might not be affected. Please correct me if I'm wrong. |
The suggested changes seem very helpful. @davepagurek It would be great to introduce an options object, separate fill and stroke shaders. The ability to specify different combinations of strokes and fills provide great flexibility to the user. |
@deveshidwivedi that's right, it would be used for strokes if the user says it should be used for strokes regardless of whether or not it uses @perminder-17 The idea would be, if a user wants to override fills or strokes, they shouldn't need to implement every fill/stroke feature p5 supports, and we should just use whatever parts of their shader happen to work with p5. So regardless of what uniforms/attributes their shader uses, if they define the shader as applying to strokes, we will try to use it for strokes, and report any encountered errors to the user rather than silently ignoring it. I think it'd also be useful to make it easier for shaders to include functionality like lighting or stroke weight without having to do it all themselves, but that would be addressed in a separate effort (something like #6144.) |
I like the idea to specifically target fills / strokes separately but am a little concerned about the breaking changes this would introduce. Do you see any path to do this that could leave in place existing functionality? I do think that removing the ability for a shader to affect If a user omits the settings object, what would the defaults be (stroke: false, fill: true)? Do we actually need a settings object or could it just be an optional enum as a third parameter |
Right, I was imagining this is the most common usage.
The 2.0 release already will have breaking changes, which is why I was thinking this could be an opportunity to simplify. But if we don't include this in 2.0, then to make this fully backwards compatible, I think we'd also need The problem with the current state of those defaults is that even if we list them, that's still kind of not really enough for users who want to make use of them: we'd need to explain what each one does and why they'd need to use them. I feel like that dumps a lot of info at once onto users. Somewhat related, I haven't written up a proposal for this in an issue yet, but in terms of documenting built-in shader features for users to tap into, in the future I want to selectively expose some shader internals people can build off of via an interface like this: const myShader = p5.RendererGL.lightingShaderGraph.fillColor.replaceInput(`
uniform float millis;
vec4 makeColor() {
return vec4(sin(millis)*0.5 + 0.5, 0.0, 0.0, 1.0);
}
`).build(); ...and then we could document the properties available on a shader graph, and let people replace their inputs with shader snippets. The benefit is that we can keep many of the properties abstracted if we choose, document properties in the way other properties in the reference are documented (shader uniforms/attributes don't really fit in with the current reference setup), and lets us change implementation under the hood without breaking shaders built this way.
Possibly! I think if these are the only two properties we'll ever have, then that works. I kind of like settings objects in general as it gives us room to grow in the future if we need (e.g. if we ever want break up |
btw, just another thought: it might make more sense to not add an options object, and instead:
This might make it even more clear for users since everything happens at the site where you try to use the shader, and not having part of the logic up where the shader was created. |
Increasing access
Shaders are arguably the part of p5 with the biggest barrier to entry. There's some existing confusion about why they get applied some times rather than others. Resolving this confusion would help to make shaders easier for users to learn and grasp.
Which types of changes would be made?
Most appropriate sub-area of p5.js?
What's the problem?
Currently, p5 tries to determine if a shader should be used for fills, strokes, images, and more based on what uniforms/attributes they include. This is currently documented like so:

If a shader doesn't need these properties, adding it into a shader unused is an unreliable solution, as some browsers optimize away unused shader properties.
Additionally, this doesn't fully address other open issues (#6327, #6564) as there are other scenarios where p5 will opt instead for a default shader, such as when using
image(...)
.What's the solution?
I propose simplifying the scenarios where user shaders can be applied to just fills and strokes.
image(...)
: if one wants to send an image to a shader, one must calltexture(...)
or set a uniform, and then draw a rectangle.To define what a shader applies to, I suggest adding an options object to
createShader
andloadShader
that looks like this (in Typescript syntax):This means that every shader you create must define what it should be applied to upon creation. By default, shaders just apply to fills and not strokes, as I think this is the most common use case. If one wants, one can specify a different combination of strokes and fills.
This also means that rather than having a single user shader set, one can possibly have a separate stroke and fill shader set.
Pros (updated based on community comments)
Cons (updated based on community comments)
Proposal status
Under review
The text was updated successfully, but these errors were encountered: