WebGL2 rendering broken: correct transform are not used #9375
Comments
mockersf
added
C-Bug
alice-i-cecile
added
the
P-High
|
I'll have a look and see if I can find out what's up. |
|
Well, my bad for testing the GpuArrayBuffer stuff on WebGL2 via applying the limits. I thought I’d tested it on web at some point and seen it work. The problem is that WebGL2 GLSL only supports gl_InstanceID which is the index within the range of instances being drawn. So for example, if you’re drawing instances 10..12 in rust notation, the gl_InstanceID values would be 0..2. naga and wgpu currently don’t implement any workaround for this shortcoming such that the wgsl instance_index builtin gets translated to plain gl_InstanceID. They would have to provide the base instance via a uniform and polyfill this into the shader. As we’re doing one draw per instance at the moment, this would mean wgpu+naga would have to provide a new uniform per draw. I don’t know if it would do this via a buffer or using the direct uniform submission that exists in GL. Either way, it adds back at least some if not all of the per-draw binding overhead that GpuArrayBuffer and instance_index were trying to avoid. One way that it would still be achieved on WebGL2 is when we get to the ‘encode instance index into the upper bits of the index buffer indices’ part of batching. For now I suppose it’s going to be worth testing whether direct uniform submission by value is faster than rebinding a uniform buffer or updating its dynamic offset to see if it’s worth keeping the approach anyway. And a short term fix will be to basically have WebGL2 one way or another go back to a dynamic offset per MeshUniform. Sad. |
|
Talking with @cwfitzgerald it sounds like when instance_index is properly polyfilled in naga+wgpu then the uniform for the base index would always be pushed regardless of whether we use it or not, so we may as well use it. |
|
And while I was looking into implementing that in wgpu and naga, I noticed that push constants could be used as a workaround for now, as they are translated to uniforms, which is exactly what I would implement anyway. Just that the workaround introduces more ugliness into bevy. :/ |
…n WebGL2 (#9383) naga and wgpu should polyfill WGSL instance_index functionality where it is not available in GLSL. Until that is done, we can work around it in bevy using a push constant which is converted to a uniform by naga and wgpu. # Objective - Fixes #9375 ## Solution - Use a push constant to pass in the base instance to the shader on WebGL2 so that base instance + gl_InstanceID is used to correctly represent the instance index. ## TODO - [ ] Benchmark vs per-object dynamic offset MeshUniform as this will now push a uniform value per-draw as well as update the dynamic offset per-batch. - [x] Test on DX12 AMD/NVIDIA to check that this PR does not regress any problems that were observed there. (@Elabajaba @robtfm were testing that last time - help appreciated. <3 ) --- ## Changelog - Added: `bevy_render::instance_index` shader import which includes a workaround for the lack of a WGSL `instance_index` polyfill for WebGL2 in naga and wgpu for the time being. It uses a push_constant which gets converted to a plain uniform by naga and wgpu. ## Migration Guide Shader code before: ``` struct Vertex { @Builtin(instance_index) instance_index: u32, ... } @vertex fn vertex(vertex_no_morph: Vertex) -> VertexOutput { ... var model = mesh[vertex_no_morph.instance_index].model; ``` After: ``` #import bevy_render::instance_index struct Vertex { @Builtin(instance_index) instance_index: u32, ... } @vertex fn vertex(vertex_no_morph: Vertex) -> VertexOutput { ... var model = mesh[bevy_render::instance_index::get_instance_index(vertex_no_morph.instance_index)].model; ```
Bevy version
mainsince #9254[Optional] Relevant system information
On WebGL2, all browsers
What you did
Run one of the 3d examples, easily visible on the
pbrexampleWhat went wrong
The text was updated successfully, but these errors were encountered: