Skip to content

Conversation

@kvark
Copy link
Member

@kvark kvark commented Jan 24, 2026

Connections
Description
In addition to samplers, buffers, and images, we need to support TLAS in binding arrays.

Testing
Included.

Squash or Rebase?

Rebase.

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.

@JMS55
Copy link
Collaborator

JMS55 commented Jan 25, 2026

I'm curious, are you planning to use more than one TLAS?

@kvark
Copy link
Member Author

kvark commented Jan 25, 2026

@JMS55 yes, exactly.
Imagine a Gaussian point cloud expressed as a TLAS with millions of instances of a single BLAS that has a triangular approximation of a mesh - https://gaussiantracer.github.io/
Now, this is a single cloud. What if you want to have a scene composited from multiple clouds? One way would be to merge all points into a single cloud on GPU every frame. But a more interesting path would be to have a TLAS at the higher level, and it would resolve to a specific cloud, which has it's own TLAS. Like a nested traversal.
AFAIK, Metal supported nested acceleration structures from day 1 instead of having a distinct TLAS thing.

@kvark kvark marked this pull request as ready for review January 25, 2026 02:41
@kvark kvark requested a review from cwfitzgerald January 25, 2026 02:42
Copy link
Contributor

@Vecvec Vecvec left a comment

Choose a reason for hiding this comment

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

Thank you for adding this into wgpu as well as naga! I think that bind groups containing binding arrays of acceleration structures should have a new limit like all the other resources. It would also be nice to mention this in the ray tracing API docs.

/// - Vulkan
///
/// This is a native only feature.
const ACCELERATION_STRUCTURE_BINDING_ARRAY = 1 << 59;
Copy link
Contributor

Choose a reason for hiding this comment

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

Because binding arrays do not validate out of bounds indexing, this should probably be listed as experimental.

@kvark
Copy link
Member Author

kvark commented Jan 26, 2026

@Vecvec thank you for reviewing!
Falling under the existing max_binding_array_elements_per_shader_stage would be most consistent with the current implementation. Unless increasing the scope of this limit would actually hurt the value, which I don't think it does.

Because binding arrays do not validate out of bounds indexing, this should probably be listed as experimental.

I don't see other non-uniform indexing declared as experimental, e.g. STORAGE_TEXTURE_ARRAY_NON_UNIFORM_INDEXING, so I believe the current state of PR is most consistent.

@Vecvec
Copy link
Contributor

Vecvec commented Jan 26, 2026

Falling under the existing max_binding_array_elements_per_shader_stage would be most consistent with the current implementation. Unless increasing the scope of this limit would actually hurt the value, which I don't think it does.

Sorry, for some reason I though that the default for normal acceleration structures (16) was significantly lower than others and therefore the same might be the case for the bindless version. If it doesn't hurt this value then it should probably just be integrated into the same limit.

I don't see other non-uniform indexing declared as experimental

They were added before experimental features were a concept, see #8619.

@kvark
Copy link
Member Author

kvark commented Jan 26, 2026

Small comparison on the limits:

So, indeed the inclusion of acceleration structures would lower the bounds if the feature is enabled. Not a good thing.
At the same time, there is already a big difference between this limit for different object types... I've added a new limit for the acceleration structures, but we may need to also separate the buffer limit from the image limit there in another PR.

They were added before experimental features were a concept, see #8619.

Can we defer to the moment where this issue will be implemented to convert them in bulk?

@Vecvec
Copy link
Contributor

Vecvec commented Jan 26, 2026

For most of the API it isn't really my design choices, so @cwfitzgerald probably has final say on most of this.

Can we defer to the moment where this issue will be implemented to convert them in bulk?

I'm perfectly happy to defer the change.

Small comparison on the limits

This is very confusing, I thought the update after bind limits had a significantly higher limit (50,000) vs 'normal' limits. That said, they do seem to be much lower for acceleration structures even for the limits higher than that, so I think it would be safer.

@inner-daemons inner-daemons self-requested a review January 26, 2026 21:09
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.

3 participants