Skip to content

[naga glsl-out] Differentiate between support for std140 and std430 #7579

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

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

Conversation

cloone8
Copy link
Contributor

@cloone8 cloone8 commented Apr 19, 2025

Description

When outputting code for older OpenGL versions, I noticed that Naga does not differentiate between not having support for explicit bindings/locations and outputting Uniform memory layouts. Furthermore, if no global binding was given, no attempt at writing an explicit memory layout is made at all. This PR attempts to improve that by making a distinction between:

  • Supporting explicit locations
  • Supporting std140 layout
  • Supporting std430 layout

...And then outputting the memory layout if at all possible, even if no explicit location is supported/exists.

The result is that globals that have no explicitly given bindings now get a memory layout anyway.

Testing

I ran the whole test suite with cargo xtask test, of course, which showed a small amount of weird failures on my own system that were present on previous commits too. I also examined and validated (with glslang) the resulting changed shaders.

Furthermore, I reasoned (with my admittedly limited knowledge) that this change should be fine, because:

  • Naga emits these layouts anyway for newer GLSL versions
  • The previous versions with the default layouts required the use of reflection, which should still work perfectly with even with an explicit memory layout

But I admit I don't think I grasp the complete scope of how these changes could break things. Specifically, I saw some shader snapshots that were testing padding related things (naga/tests/out/glsl/wgsl-struct-layout.needs_padding_comp.Compute.glsl and naga/tests/out/glsl/wgsl-struct-layout.no_padding_comp.Compute.glsl) that worry me slightly, as changing memory layouts could break things if padding was manually inserted using different rules. In theory, however, it should be fine as the previous default was std140 anyway (for Vulkan, I believe) and shared for OpenGL, which leaves layout fully up to the driver and thus makes little sense to have padding inserted manually.

If these changes turn out to break other code, I'd be happy to take another look if someone points me in the right direction!

Checklist

  • Run cargo fmt.
  • Run taplo format.
  • Run cargo clippy --tests. If applicable, add:
    • --target wasm32-unknown-unknown
  • Run cargo xtask test to run tests.
  • If this contains user-facing changes, add a CHANGELOG.md entry.

NOTE: I added the changelog entry to "new features" because I didn't know how you all differentiate between new features and changes (or if this is seen as a bug), but feel free to shuffle that around if you wish.

…0` layout, and emit `std140` in Uniforms when possible
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