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

feat: Use explicit pipeline layouts from shaderLayout #2355

Merged
merged 3 commits into from
Mar 29, 2025

Conversation

Kaapp
Copy link
Collaborator

@Kaapp Kaapp commented Mar 28, 2025

Background

Solves an issue raised in: visgl/deck.gl#9531

Unused uniforms are stripped from WebGPU shaders by the compiler as they are not part of the shader's resource interface (as per spec)

When shader source injection is performed as a result of using modules in getShaders, if the bindings injected by that call (such as lighting) go unused, the automatic layout generated by WebGPU will not contain these unused bindings.

Attempting to create the BindGroupLayout in this case will result in luma attempting to pass a binding for each uniform it's aware of from our WGSL introspection library. Ideally, we want to avoid this mismatch as it makes for a poor developer experience as commenting out lines of code to debug shaders will result in these sorts of errors being thrown.

Possible solutions

  1. Use the existing WGSL introspection to attempt to determine which bindings are used, not just present and remove those unused bindings from the shaderLayout in order to match the automatic layout.
  2. Create an explicit pipeline layout based on the shaderLayout, which will allow us to bind values to the unused bindings without throwing an error.

Here I've experimented with implementing option 2 for the following reasons:

  • Option 1 is brittle and that level of source parsing is probably best avoided
  • Option 2 opens up the possibility of caching the BindGroup/BindLayouts so that they can be shared across layouts/layers. This could be quite useful for luma-injected bindings like project and lighting.
  • Option 2 is required if you want to be able to use dynamicOffsets or unfiltered textures, which users creating custom layers may well want to do in the future.

Change List

  • Add a new pipeline layout object which parses the shaderLayout and produces a layout for the render pipeline to use.
  • It does not yet handle multiple binding groups, but we have this limitation elsewhere too, so not sure that needs to be addressed here.

@Kaapp Kaapp self-assigned this Mar 28, 2025
@Kaapp Kaapp mentioned this pull request Mar 28, 2025
19 tasks
@Kaapp Kaapp requested review from felixpalmer and ibgreen March 28, 2025 18:48
Copy link
Collaborator

@ibgreen ibgreen left a comment

Choose a reason for hiding this comment

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

I kind of expected that we would want to expose bind groups and pipeline layouts to the programmer at some point, so this could be a reasonable first step.

To take this all the way later I expect that we would

  • add an abstract createPipelineLayout() in the core Device class
  • Implement a dummy WebGLPipelinelayout
  • Have the Pipeline classes accept an optional PipelineLayout
  • Default to the auto created layout you are generating.
  • Add docs for Pipeline

@ibgreen ibgreen merged commit 4a83600 into visgl:master Mar 29, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants