Skip to content

WIP - Support vertex pulling #16826

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

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

MiiBond
Copy link
Contributor

@MiiBond MiiBond commented Jun 27, 2025

Vertex pulling is where you do your own reading of vertex data in the vertex shader instead of relying on the standard vertex attribute pipeline. I'm not sure of the best approach in Babylon but I made some minimal changes to make it work. I'd appreciate some feedback and ideas for a better way to set this up.

First, some motivation:
I want vertex pulling as a way of selecting vertex data from neighboring vertices in the IBL Shadows voxelization shader. By retrieving normal info for the provoking vertex of a triangle, I can selectively swizzle the axes of the position to maximize rasterization area and avoid missing voxels. This eliminates the need for 3-pass voxelization and opens the door for doing realtime voxelization of animated geometry (in WebGPU only due to the need for 3D storage textures).
https://playground.babylonjs.com/?snapshot=refs/pull/16826/merge#XSNYAU#128

For vertex pulling to work, we typically need:

  1. An empty vertex layout.
  2. Our needed vertex buffers assigned as storage buffers
  3. A non-indexed draw call with the number of vertices to process. This causes @builtin(vertex_index) to be sequential which is critical for fetching info about neighboring vertices.

No changes are needed to Babylon for the first two requirements. For the 3rd point though, we want to do a engine.drawArraysType even though the mesh has indices. To do this, we set material.useVertexPulling.
The shader must also not declare vertex attributes.

Thoughts?

@bjsplat
Copy link
Collaborator

bjsplat commented Jun 27, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Jun 27, 2025

@bjsplat
Copy link
Collaborator

bjsplat commented Jun 27, 2025

@bjsplat
Copy link
Collaborator

bjsplat commented Jun 27, 2025

@Popov72
Copy link
Contributor

Popov72 commented Jul 1, 2025

It would be a great feature to have!

However, I think we should strive to improve integration into the engine so that we/the user don't have to manually manage storage buffers and so that it remains as transparent as possible.

Another issue is that we don't really want to modify the mesh at all since some render passes won't do vertex pulling

I wonder if we could just create the vertex buffers with STORAGE flags (in addition to the VERTEX + WRITE flags we currently use)... This way, the buffers should be usable in both pulling and non-pulling modes (to be tested, however, and I don't know if adding unnecessary flags has an impact on performance - STORAGE is not necessary in non-pulling mode, and VERTEX is not necessary in pulling mode).

In this case, we could add a useVertexPulling property to Material. When true, we would not bind vertex buffers to the WebGPU pipeline, but rather bind storage buffers to the shader. We would also inject a “USE_VERTEX_PULLING” define, so that the shader code can adapt accordingly (in case the code wants to support both modes)...

I'm probably missing a few things, but I think this would be a better way to support this mode.

cc @sebavan for the discussion.

@Popov72
Copy link
Contributor

Popov72 commented Jul 1, 2025

Also, in your PG example, I think you would want to calculate the normal yourself by performing the vector product of two edges of the triangle, because the normal to a vertex is generally the average of the normals of the faces to which that vertex belongs, so it is not the true normal to the face.

@sebavan
Copy link
Member

sebavan commented Jul 1, 2025

I really like where it is going :-) and I ll chat with Mike after his break.

@MiiBond MiiBond force-pushed the mbond/support-vertex-pulling branch from a4c7ec9 to 8e92eed Compare July 11, 2025 18:35
@bjsplat
Copy link
Collaborator

bjsplat commented Jul 11, 2025

@bjsplat
Copy link
Collaborator

bjsplat commented Jul 11, 2025

@MiiBond
Copy link
Contributor Author

MiiBond commented Jul 12, 2025

I've updated the PR with:

  • vertex pulling boolean is now on the material
  • no need to manually set storage buffers for vertex buffer attributes.

I'm not sure if the way I did this is okay but it seems to work.
I still need to handle the index buffer. My first attempt wasn't successful and I think I have an issue with 16-bit vs 32-bit indices... not sure of the right way to handle this.

@bjsplat
Copy link
Collaborator

bjsplat commented Jul 12, 2025

@bjsplat
Copy link
Collaborator

bjsplat commented Jul 12, 2025

@MiiBond
Copy link
Contributor Author

MiiBond commented Jul 12, 2025

Ah, looks like the index buffer in my test is being created as a Uint16Array and WGSL only supports u32 as a type...

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.

4 participants