Material bind group shader def #20069
Conversation
tychedelia
added
C-Feature
| @group(2) @binding(0) var tileset: texture_2d_array<f32>; | ||
| @group(2) @binding(1) var tileset_sampler: sampler; | ||
| @group(2) @binding(2) var tile_indices: texture_2d<u32>; | ||
| @group(#{MATERIAL_BIND_GROUP}) @binding(0) var tileset: texture_2d_array<f32>; |
There was a problem hiding this comment.
I think these should be MATERIAL_2D_*?
There was a problem hiding this comment.
Why not reuse the same shader def? Under the hood we can map it 2/3 for 2d/3d, and users won't have to care. That's the whole point :)
| @group(2) @binding(0) var<uniform> material: ColorMaterial; | ||
| @group(2) @binding(1) var texture: texture_2d<f32>; | ||
| @group(2) @binding(2) var texture_sampler: sampler; | ||
| @group(#{MATERIAL_BIND_GROUP}) @binding(0) var<uniform> material: ColorMaterial; |
| @group(2) @binding(0) var<uniform> material_color: vec4<f32>; | ||
| @group(2) @binding(1) var base_color_texture: texture_2d<f32>; | ||
| @group(2) @binding(2) var base_color_sampler: sampler; | ||
| @group(#{MATERIAL_BIND_GROUP}) @binding(0) var<uniform> material_color: vec4<f32>; |
|
Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke! If it's expected, please add the M-Deliberate-Rendering-Change label. If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it. |
|
Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke! If it's expected, please add the M-Deliberate-Rendering-Change label. If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it. |
There was a problem hiding this comment.
I still think it's a bit confusing that the material 2d def gets renamed. It's only confusing for people touching engine internals. For most users it's probably simpler to keep this approach so LGTM.
(I still don't like how verbose this makes everything but I don't have a better suggestion)
Use a shader def for the material bind group index to make it easier for when we want to switch back to group 2 in the future without breaking everyone again.