-
-
Notifications
You must be signed in to change notification settings - Fork 35.6k
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
(Wrong) Fix webgpu attribute code - but might inspire correct fix #25663
Conversation
This works but is probably the wrong fix. I don't know three.js that well so I thought I'd post this as an example of a fix and it might lead to the correct fix. The issue is WebGPU does not support itemSize = 1 or itemSize = 3 for 8bit or 16bit values. When using KHR_mesh_quantization some attributes are like Int16x3. But, according to the KHR_mesh_quantization spec they are actually padded to Int16x4 In WebGL it's fine to set the attribute to ``` size = 3, type = gl.SHORT, normalize = true, stride = 8. ``` But in WebGPU there is no such thing as `size = 3, type = gl.SHORT` If it existed it would be called `sint16x3` or `uint16x3` or `snorm16x3` or `unorm16x3` but none of those exist. Instead you have to set the attribute to `???x4` for these situations. You then need to be careful the shader doesn't actually use Z. The shader can still declare its attribute has vec3f/vec3<f32> but if it declares it as vec4/vec4<f32> the z value would be undefined (whatever happens to be in the file in the padding) My guess is the correct fix, rather than this hack which expands the type, it should look at the stride. The stride to figure out when to expand the type from x3 to x4 or from x1 to x2 or x1 to x4
6dd385a
to
6751276
Compare
Would it be fair to say that THREE.WebGPURenderer does not support interleaved vertex attributes at all yet? That appears to be the case, and because WebGPU requires padding on uint16x3 and other vertex layouts, we'll be using Related — we will need to consider |
I added support for this here ( #24117 ), but following this issue it appears to be incomplete. I will check and release a possible fix as soon as possible. |
@sunag A user in the forum recently wanted to load an optimized glTF asset with KTX2 textures and compressed attribute data in its WebGPU three.js app (https://discourse.threejs.org/t/load-glb-models-in-webgpu/50397). Both features did not work however the KTX2 side is fixed via recent PRs (#25864, #25865, #25867). I have remembered this discussion/PR and wonder now if you already have made progress in fully supporting interleaved vertex data. TBH, this topic isn't trivial and more complicated to implement than the missing texture compression support. Simply because the WebGPU spec is so different in that regard compared to WebGL. |
BTW: An ideal asset for testing is https://github.com/mrdoob/three.js/blob/dev/examples/models/gltf/coffeemat.glb. |
We could consider improvements to the InterleavedBuffer API, which could benefit both WebGL and WebGPU. Related: |
I didn't really like the results I got, in the tests I had forced to do the attribute conversion via CPU, I thought I'd go back to that when I had a better idea, I'd try again using Compute but that would be a trial too. I am finalizing the implementation of an API for be used in transmission and I will start PMREM next week, I think I can finish like this after implementing the API that will be used in transmission. |
@Mugen87 Your asset made it easy, thanks. It because I didn't need to resize the array, we're making progress on that. Tomorrow I'll be posting even not fully finished, it remains to resize the arrays if necessary and improve the pipeline to optimize gpu calls |
Closing in favor of #25924. |
Related issue: #24005
Description
Wrong fix for WebGPU gLTF support?
This works but is probably the wrong fix? I don't know three.js
that well so I thought I'd post this as an example of a fix
and it might lead to the correct fix.
The issue is WebGPU does not support itemSize = 1 or itemSize = 3
for 8bit or 16bit values.
When using KHR_mesh_quantization some attributes are like Int16x3.
But, according to the KHR_mesh_quantization spec they are actually
padded to Int16x4
In WebGL it's fine to set the attribute to
But in WebGPU there is no such thing as
size = 3, type = gl.SHORT
If it existed it would be called
sint16x3
oruint16x3
orsnorm16x3
orunorm16x3
but none of those exist. Insteadyou have to set the attribute to
???x4
for these situations.You then need to be careful the shader doesn't actually use Z.
The shader can still declare its attribute has vec3f/vec3
but if it declares it as vec4/vec4 the z value would be
undefined (whatever happens to be in the file in the padding)
My guess is the correct fix, rather than this hack which expands
the type, it should look at the stride to figure
out when to expand the type from x3 to x4 or from x1 to x2
or x1 to x4? Even that isn't entire clear though because the
stride might be for interleaved vertex data so it's not clear
how to communicate at a lower level, in WebGPURenderPipeline,
that one of these unsupported types should be promoted to
a supported type.
In any case, with the code in this PR both of compressed glb files that @donmccurdy posted in this PR thread work.