Skip to content
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

support more procedural shader uniform types, including arrays #1178

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

HifiExperiments
Copy link
Member

procedural shaders previously supported uniforms of types float, vec2, vec3, and vec4. this PR adds support for mat3, mat4, vec3[], vec4[], mat3[], and mat4[].

here is a script that demonstrates this feature. it creates a procedural particle entity and places a new particle every time you click on an entity (up to 50 particles in this example but it would support more). in the vertex shader, the click positions (and normals) are sent as a uniform vec3[].

@HifiExperiments HifiExperiments added enhancement New feature or request needs CR This pull request needs to be code reviewed needs QA This pull request needs to be tested labels Oct 22, 2024
@@ -356,16 +356,53 @@ void Procedural::prepare(gpu::Batch& batch,
}
}
// Then fill in every reflections the new custom bindings
int customSlot = procedural::slot::uniform::Custom;
size_t customSlot = procedural::slot::uniform::Custom;
_slotMap.clear();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

per the spec, float/vec2/vec3/vec4/mat3/mat4 take up a single uniform slot, whereas array types take up as many as the length of the array.

we compute how many slots each uniform is going to need up here, and then in Procedural::setupUniforms we can just look it up

std::string uniformName = key.toLocal8Bit().data();
std::string trueUniformName = uniformName;
if (isArrayUniform) {
trueUniformName += "[0]";
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

due to the internals of how our shader management classes work, reflection->uniforms expects the slot to be specified for just the first element of array types, with [0] included at the end of the string

Copy link
Member

@ksuprynowicz ksuprynowicz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome :)
Could we add example script to our scripting API documentation?

@HifiExperiments
Copy link
Member Author

This is awesome :) Could we add example script to our scripting API documentation?

great point, maybe the best place is to update the procedural shader guide? I added some more info and an example here: overte-org/overte-docs-sphinx#54

@ksuprynowicz
Copy link
Member

great point, maybe the best place is to update the procedural shader guide? I added some more info and an example here: overte-org/overte-docs-sphinx#54

Thank you! I think it's the best way to do it.

@ksuprynowicz ksuprynowicz added CR approved This pull request has been successfully code reviewed and removed needs CR This pull request needs to be code reviewed labels Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CR approved This pull request has been successfully code reviewed enhancement New feature or request needs QA This pull request needs to be tested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants