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

ShaderProcessor partially updating shader source to support uniform buffers #4402

Merged
merged 5 commits into from
Jul 7, 2022

Conversation

mvaligursky
Copy link
Contributor

@mvaligursky mvaligursky commented Jul 5, 2022

a progress towards #4261

  • a GLSL shader processing in order to allow WebGL shader to be transformed info a format we can transpile to WebGPU. Initially in this PR, attributes, varyings and uniforms are extracted, and transforms into required form - which means a fixed slot allocation, as well as uniform buffers.
  • ProgramLibrary has been updated to allow this processing to be executed globally on any shader, which includes shaders set by the user directly on the Material, in addition to Standard, Basic, Sky, and Particle.

Additional changes:

  • small clean up to material's handling of transparency functions.

Note: The shader processing does not currently execute on WebGl, but some shader functionality refactoring is used.

@mvaligursky mvaligursky self-assigned this Jul 5, 2022
@mvaligursky mvaligursky added enhancement area: graphics Graphics related issue labels Jul 5, 2022
mvaligursky and others added 3 commits July 5, 2022 16:11
Co-authored-by: Will Eastcott <will@playcanvas.com>
Copy link
Member

@slimbuck slimbuck left a comment

Choose a reason for hiding this comment

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

It seems like we're adding an invisible layer of processing to our shader code that will ultimately be difficult for users to debug if and when it goes wrong. I'm a little concerned that we're not really documenting the expectations or limitations of this step and it will ultimately become a magical black-box.

Are you happy with our approach of regex'ing shaders like this and processing the code in this way? Or do you expect we'll be rewrite this using different tools or ...?

src/graphics/program-library.js Show resolved Hide resolved
src/scene/batching/batch-manager.js Show resolved Hide resolved
src/scene/materials/material.js Show resolved Hide resolved
Copy link
Contributor

@GSterbrant GSterbrant left a comment

Choose a reason for hiding this comment

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

Approved with a few comment/formatting comments.

src/graphics/shader-processor-options.js Outdated Show resolved Hide resolved
src/graphics/shader-processor-options.js Show resolved Hide resolved
src/scene/mesh-instance.js Show resolved Hide resolved
src/scene/batching/batch-manager.js Show resolved Hide resolved
@mvaligursky
Copy link
Contributor Author

Are you happy with our approach of regex'ing shaders like this and processing the code in this way? Or do you expect we'll be rewrite this using different tools or ...?

It's hard to know at this stage. Right now my focus is to transpile GLSL shaders so that I can directly execute them on WebGPU device, allowing me to develop the rest of the WebGPU rendering. This is a black box, but a pretty small one, and executes for now only on WebGPU.

There is also an option to execute this on WebGl to add support for Uniform Buffers in the future.

Even if we went ahead and rewrote all shaders in the WebGPU native shader language, due to the current lack of support of reflection for those, we'd need to do processing there as well and manually assign slots.

@mvaligursky mvaligursky merged commit 1294f74 into main Jul 7, 2022
@mvaligursky mvaligursky deleted the mvaligursky-shader-processor branch July 7, 2022 10:37
@mvaligursky mvaligursky mentioned this pull request Jul 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: graphics Graphics related issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants