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

GLES 3.0 Binding Model Broken with Shared Vertex/Fragment Uniform Buffers #3349

Closed
Tracked by #3674
cwfitzgerald opened this issue Jan 4, 2023 · 6 comments
Closed
Tracked by #3674
Labels
api: gles Issues with GLES or WebGL area: correctness We're behaving incorrectly type: bug Something isn't working

Comments

@cwfitzgerald
Copy link
Member

Description

Due to GLES 3.0 not having binding indexes, we need to rely completely on reflection to bind all the buffers in the right place. Naga (correctly) generates unique names and structures and such for the vertex and fragment shaders. Because there are no binding indexes, GLES matches up bindings by name of the struct, and because they are different, creates different bindings for each stage.

Our choices are to:

  • Change our binding model to bind buffers separately for vertex and fragment on GLES 3.0.
  • Try to share the binding across states. This unfortunately also means that the structs in vertex and fragment must be identical, and this is not a requirement of WebGPU at all.

In order to not add this giant requirement, we should change our binding model.

Repro steps

Bind a buffer to both vertex and fragment shaders in GLES 3.0.

Expected vs observed behavior

Only one of the buffers will be correctly bound, the other won't be. This will cause all sorts of nonsense.

@cwfitzgerald cwfitzgerald added type: bug Something isn't working area: correctness We're behaving incorrectly api: gles Issues with GLES or WebGL labels Jan 4, 2023
@cwfitzgerald cwfitzgerald changed the title GLES 3.0 Binding Model Broken with Shared Vertex/Fragment Buffers GLES 3.0 Binding Model Broken with Shared Vertex/Fragment Uniform Buffers Jan 4, 2023
@ryankaplan
Copy link
Contributor

Hi again @cwfitzgerald! Can you help me figure out if I'm running into this issue? The repro steps are Bind a buffer to both vertex and fragment shaders in GLES 3.0 but I'm not sure what this means from the perspective of the wgpu API. Is it sufficient to draw using a uniform buffer that is bound to uniforms with both wgpu::ShaderStages::VERTEX and wgpu::ShaderStages::FRAGMENT visibility?

@cwfitzgerald
Copy link
Member Author

cwfitzgerald commented Oct 28, 2023

Yeah - binding a single buffer with VERTEX_FRAGMENT doesn't work. It will only end up binding to one stage.

@ryankaplan
Copy link
Contributor

So I'm extra sure, you're saying that this is no good...

wgpu::BindGroupLayoutEntry {
  // Problem: we will bind to only one of these...
  visibility: wgpu::ShaderStages::VERTEX | wgpu::ShaderStages::FRAGMENT,
  ...
}

But I could, for example, split the above layout entry into two layout entries that bind to the same buffer that I bound to before. Does that sound right?

@cwfitzgerald
Copy link
Member Author

Correct!

@cwfitzgerald
Copy link
Member Author

This bug doesn't exist.

I do not have the original reproduction for this issue and the test I wrote for it passes every time.

Notably we use glUniformBlockBinding to bind the blocks on the CPU side, which allows us to call bind once to multiple blocks, and everything is happy.

The reproduction still found a bug in #4607, so I'm going to keep it in the PR.

@ryankaplan
Copy link
Contributor

That matches what I'm seeing too. Splitting my layouts into separate FRAGMENT | VERTEX usages didn't seem to change anything for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: gles Issues with GLES or WebGL area: correctness We're behaving incorrectly type: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants