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

WebGPU: support vertex buffers with non multiple of 4 bytes strides #14413

Merged
merged 10 commits into from
Oct 12, 2023

Conversation

Popov72
Copy link
Contributor

@Popov72 Popov72 commented Oct 11, 2023

I added the support as a whole and not to WebGPU specifically, this way it can benefit to any engine that requires that strides be a multiple of 4 bytes (like probably Native does).

If the stride is not a multiple of 4 bytes, each time you call VertexBuffer.update(), it will create a new GPU buffer. But calling VertexBuffer.update() is slow anyway, because we must create a new buffer on the js side to make space for padding data. So, I prefered not to handle this case and simply create a new GPU buffer instead of making the PR more complex. If necessary, we can update the docs to explain what happens with non aligned strides and how to handle it on the user side.

[EDIT] Ready for review again!

@bjsplat
Copy link
Collaborator

bjsplat commented Oct 11, 2023

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 Oct 11, 2023

@Popov72 Popov72 marked this pull request as draft October 11, 2023 19:41
@Popov72 Popov72 marked this pull request as ready for review October 11, 2023 20:45
@Popov72 Popov72 marked this pull request as draft October 11, 2023 22:32
@Popov72 Popov72 marked this pull request as ready for review October 12, 2023 15:18
@deltakosh deltakosh merged commit 8a5077e into BabylonJS:master Oct 12, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants