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

Fix GLB parser index buffer initialization #7042

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

LeXXik
Copy link
Contributor

@LeXXik LeXXik commented Oct 16, 2024

Fixes #5869

Fixes a bug, where GLB parser passes a typed array instead of an array buffer to the index buffer constructor.

I confirm I have read the contributing guidelines and signed the Contributor License Agreement.

@LeXXik
Copy link
Contributor Author

LeXXik commented Oct 16, 2024

This breaks the GLB examples.

@LeXXik LeXXik marked this pull request as draft October 16, 2024 13:34
@LeXXik
Copy link
Contributor Author

LeXXik commented Oct 17, 2024

@mvaligursky hmm, why does vercel preview deployment have this webgpu index buffer?

Edit: nevermind, those are different between webgpu and webgl

@LeXXik LeXXik marked this pull request as ready for review October 17, 2024 12:09
@LeXXik LeXXik marked this pull request as draft October 17, 2024 12:37
@LeXXik
Copy link
Contributor Author

LeXXik commented Oct 17, 2024

Ok, the issue is that when Index Buffer sets data, it uses byte length with bytes count to check the size. It expects it to be an array buffer, but receives and acts as if it is a typed array:

/**
* Set preallocated data on the index buffer.
*
* @param {ArrayBuffer} data - The index data to set.
* @returns {boolean} True if the data was set successfully, false otherwise.
* @ignore
*/
setData(data) {
if (data.byteLength !== this.numBytes) {
Debug.error(`IndexBuffer: wrong initial data size: expected ${this.numBytes}, got ${data.byteLength}`);
return false;
}

Array Buffer in this case has a byte length of 4288, while the typed array has 72 with an offset:

image

So, the numbers will match only if a typed array is passed, when using 72, instead of 4288. @mvaligursky please advice.

@LeXXik
Copy link
Contributor Author

LeXXik commented Oct 18, 2024

I suppose one option could be to change the docs, so the index buffer takes a typed array, instead of array buffer, and change the procedural so that it passes a typed array as well. Not sure about the code base though, but it seems a typed array is used everywhere or we would have noticed this earlier.

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.

GLB parser incorrect index buffer construction
1 participant