-
-
Notifications
You must be signed in to change notification settings - Fork 481
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
Documenting available attributes and uniforms on the Reference page #1017
Comments
I think this would be a great addition to the docs. I've often found myself struggling to find this information; it would be great if we could more easily surface it for more people :) It warrants a separate issue, but looking at this I also think that it would be worth also making an additional uModelViewProjectionMatrix uniform that has already done the matrix multiplication on the cpu side. I think this is a pretty common pattern and should be an easy performance win. |
Ah, sounds good! I'll make generous use of the great descriptions you have written in the comments in the examples =) |
Hi, just wanted to check in and mention that I do plan on creating a pull request for this issue. I haven't been able to find as much time as I would like to tackle this. |
Thanks @JetStarBlues ! I think to make it easier (both on yourself and to get the PR approved) I suggest starting with smaller PR's than trying to do the whole thing. Would it make sense to add a separate documentation page for "Built in Uniforms and Attributes", and that can be linked to from the I think that expanding the breadth and completeness of our documentation fits into the accessibility statement. We can make these features easier to find, read, use, and understand for people who are new to shaders / graphics / p5. Providing small examples of usage will also help people understand how each variable might be used. @stalgiag Any notes? Does this proposal seem sound to you? |
Thank you for the feedback 🙏 |
Hi all! I love the idea and I agree with @aferriss' suggestion of separating it. This will free us up to have an extensive entry on attributes and uniforms without adding information overload to the reference. I am unsure of what the best approach would be for making this separate page. Would you want to have embedded examples or would it also work as a wiki? |
Placing it on the wiki is probably the lowest friction option, though I do think it would be a good addition to the official reference as well. I'm actually not sure how to spin off a separate page for something that doesn't already have a dedicated method, since all the docs are auto-generated. I'm sure it's possible but might take some poking around. |
Another place it can potentially go is examples. Where it can be considered "optional" reading for the curious (linked to by the setUniform reference page), and can support embedded examples. |
Hm it depends on how you want to design it. This is a bit outside of the norm for the examples page but I think there is some flexibility if you were to think through it with a specific example in mind. I don't know if it would be possible to make a multi-sketch 'Example'. |
I've had a chance to start working on this. Here's some screen captures of what I have so far. I can create a pull request if the direction is agreeable. Screen capture of rendered page Screen capture of example in index page Placing the descriptions as an example page has the upside of supporting a live demo. The downside, is that the vertex and fragment shader source code (demonstrating the use of these values) is not immediately visible. Like other shader examples, it is up to the user to dig into the p5.js-website repo to find them. To make the page slightly more useful/accessible, it would be ideal to have a link to it in the Here is the full (modified) source code of the example provided. Here are the descriptions in text form.
🙏 For the matrices, I am not familiar enough with them to write proper descriptions. Suggestions are very much welcome. For the view matrix, which camera is used? Does each WEBGL sketch start with a default camera initialized? I have excluded the following reserved uniform keywords from the documentation (see list below). These uniforms are set by a call to p5.RendererGL.prototype._setFillUniforms = function(fillShader) {
fillShader.bindShader();
if ( fillShader !== this.userFillShader ) // NEW
{
fillShader.setUniform('uMaterialColor', this.curFillColor);
...
}
fillShader.bindTextures();
}
|
This is a great start! I think you could create a draft PR, it may be easier to keep track of changes that way. A couple suggestions...
we do this instead: aPosition Alternatively, I think a table could work as well:
Here's a sample description for normal matrix: The transpose inverse of the model matrix. Typically used in lighting calculations. I believe we're also missing |
Sounds good.
I see, will do!
I agree, it would definitely be a cleaner way to present the material. However, the example descriptions are constrained to using only elements that can appear within a
Noted. Thank you!
Unfortunately no such uniform exists. The RendererGL code uses only the following matrices (which are passed to uniforms here). I like the idea of having a /**
* model view, projection, & normal
* matrices
*/
this.uMVMatrix = new p5.Matrix();
this.uPMatrix = new p5.Matrix();
this.uNMatrix = new p5.Matrix('mat3'); |
It seems the answer is yes: // Camera
this._curCamera = new p5.Camera(this);
this._curCamera._computeCameraDefaultSettings();
this._curCamera._setDefaultCamera(); The default camera has position and orientation:
It is also defaults to perspective projection:
In the p5.Camera context, would it be better to replace focal length with projection in the description? (Disclaimer, I don't have a strong background on this topic.) And have the description updated as,
|
I was thinking about how to add
It seems that matrix division is not a thing. Luckily the p5.Matrix class already has the inversion function that is used instead. So p5.Shader.prototype._setMatrixUniforms = function() {
...
const projectionMatrix = this._renderer.uPMatrix;
const viewMatrix = this._renderer._curCamera.cameraMatrix;
const modelViewMatrix = this._renderer.uMVMatrix;
/* Extract modelMatrix from modelViewMatrix
view * model = modelview
viewInverse * view * model = viewInverse * model
model = viewInverse * model
*/
let inverseViewMatrix = new p5.Matrix();
inverseViewMatrix.invert(viewMatrix); // set as inverse of viewMatrix
inverseViewMatrix.mult(modelViewProjectionMatrix);
const modelMatrix = inverseViewMatrix; // alias
// Order of multiplication is: projection * view * model
const modelViewProjectionMatrix = projectionMatrix.copy();
modelViewProjectionMatrix.mult(modelViewMatrix);
//
this.setUniform('uModelMatrix', modelMatrix.mat4);
this.setUniform('uViewMatrix', viewMatrix.mat4);
this.setUniform('uModelViewMatrix', modelViewMatrix.mat4);
this.setUniform('uProjectionMatrix', projectionMatrix.mat4);
this.setUniform('uModelViewProjectionMatrix', modelViewProjectionMatrix.mat4);
...
}; Not sure what this extra calculation (per What do you think @aferriss? |
This feels a little backwards to me. Possibly this is just a byproduct of the way that the p5 renderer is set up, but for whatever reason the model matrix never exists on its own and is always applied to the view matrix. I think conceptually it would be good to separate the construction of the two, which would eliminate the need for the inverse calculation you've come up with. You can see where uMVMatrix gets created here and then later is translated / rotated / scaled. I think what should happen instead is that we have a separate model matrix object that has the translation / rotation / scale applied, and then is later multiplied with the view matrix. In general I think exposing and separating those matrices like you did is a good thing and feels much closer to how matrices are set up in other environments. Your solution is possibly the simplest to implement, but I feel it's more of a hack than a proper fix. Unfortunately, I think the uMVMatrix get's used in many places around the code base so it could be a tricky thing to fix and debug. I doubt it would have much impact on performance. Doing that calculation on the cpu side is vastly more efficient than if you had to do it on the gpu. I think we're veering off topic a bit here as well, and since we're talking about adding some additional functionality to the library, would you mind making a new topic to outline these ideas? It may be a tough sell in terms of accessibility. Maybe there is a case to be made that this makes p5 more aligned with the wider ecosystem of tutorials, books, and other educational materials, and thus makes p5 an easier stepping stone to learn graphics / shader programming with. Thanks again for looking into this, it's all super interesting! |
Thank you for the feedback! I agree that drawing a distinction between the two within the source code would be the ideal solution.
Will do |
Here's a screenshot of the current documentation state: And here is the content as text:
I've added I've removed |
I've added p5.Shader.prototype.isStrokeShader = function() {
return this.uniforms.uStrokeWeight !== undefined;
}; If a shader is marked as a stroke-shader, then the list of attributes available to it changes. This documentation assumes that a user shader is always created as a fill-shader. We could potentially encourage the use of user-defined stroke-shaders. This would require documenting:
If we go down this path, the code that marks a user-defined shader as a stroke-shader is currently faulty and would need to be fixed. (Maybe this should be fixed regardless...)
p5.prototype.shader = function(s) {
...
if (s.isStrokeShader()) {
this._renderer.userStrokeShader = s;
} else {
this._renderer.userFillShader = s;
this._renderer._useNormalMaterial = false;
}
s.init();
return this;
}; p5.RendererGL.prototype.drawBuffers = function(gId) {
...
if (this._doStroke && geometry.lineVertexCount > 0) {
// user shader is used here
}
if (this._doFill) {
// and also here!
}
...
};
|
@JetStarBlues I wanted to see if this was still in progress and where you ended up with documenting the uniforms. I've seen some other issues lately for people asking for documentation. Since this work was already done it might be worth putting up as a PR to review. Thanks! |
Hi @aferriss I will clean this PR up. There are a few outstanding questions, but I'll compile a list at the end. One thing I would love your opinion on is this PR (and related issue) that limits the scope of the default uniforms... this would greatly reduce the number of things we have to tell people they can't use. I will also create a single PR that updates all the shader examples. It will be a combination of the discussions (and draft PRs) here:
Two PRs in total.
|
Thanks @JetStarBlues! I left some comments on your PR |
I found this issue trying to get to the bottom of the |
It would be greatly appreciated if the p5.js team could prioritize updating the documentation to include a comprehensive explanation of the Default Shader Input. This would greatly benefit the community and ensure a better learning experience for all users. |
Actual Behaviour
There is currently no documentation on the attributes and uniforms that are available when writing shaders in p5. For example, the
aPosition
attribute is used in all vertex shader examples, but there is no associated documentation on the Reference page. Similarly, there are a lot of useful attributes and uniforms (such asuProjectionMatrix
), which do not appear in any of the available examples. The only way to learn about them is to dig into the p5 source code.Expected Behaviour
Ideally, a list of the available uniforms would be listed in the
setUniform()
documentation page. Each listing would have a brief description of what the uniform does. (This would have the added bonus of giving the user a hint as to which uniform names are unavailable for use).Similarly, a list of available attributes would be listed. If a
setAttribute()
method existed in p5, its documentation page would be the ideal place to list them. However, they can be (together with the default uniforms for consistency) be listed in thep5.Shader
documentation page.Since a lot of these default attributes and uniforms are used for the default shaders, not all are immediately relevant to outside use. Here is a list of the most useful I have been able to identify so far (source1, source2):
Ideally, additional examples (such as this one (live, src by aferriss ) would be added to demonstrate the usage of some of these attributes and uniforms.
Would you like to work on the issue?
Yes
The text was updated successfully, but these errors were encountered: