Add prepass_reads_material flag in material that sets PREPASS_READS_M…

[VictorElHajj]

…ATERIAL

Objective

• Fixes Prepass vertex shader crashes when there is a bind group #24690

Solution

• There was already a half implemented PREPASS_READS_MATERIAL introduced by Batch prepasses that use depth only #22813 (which also introduced the bug) , but it was only read in a few places and never written to.

Testing

• I tested this by using a custom vertex prepass shader which tries to read a material. Works after setting the new material flag.

[IceSentry]

I hate that this is adding even more stuff to the Material trait but I don't have an easy answer to that right now so I guess that's fine. Maybe in the future we should pull some of those into some kind of MaterialProperties struct and have a single function that returns it.

Anyway, the fix itself does what I would expect here.

[beicause]

Is there a way to do this automatically?

I suspect we can get whether the bind group is empty from the material, thus eliminating the need for this function.

[VictorElHajj]

I don't think so. The issue here is that the depth only prepass for shadow maps skips the material bind group for performance reasons, as when vertex displacement is not used the material is not needed to determine depth. But when custom vertex shaders are used the vertex prepass shader needs to access the material to match the vertex displacement.

So this flag essentially says to always include the material even in depth prepass, because the prepass needs to read it.

This isn't related to the material bind group being empty or not, but to prevent skipping the material bind group for prepasses that assume its not needed.

[VictorElHajj]

Digging a little deeper, I think its impossible to know from the rust side which material bindings the prepass shader actually uses, so that is why the flag is needed. We can only know what the material provides, not what the shader uses.

[beicause]

Won't the resource bindings used by the shader match those from AsBindGroup?

[VictorElHajj]

AsBindGroup can't distinguish between a shader that has material bindings that is used in the prepass vertex shaders and a material that has bindings that is not used in the prepass vertex shader. Just because they exist doesnt mean they're used in that specific shader. (Could be used in the normal fragment shader for example but not the prepass vertex shader).

In the case where its not used we still want to keep the existing behaviour to speed up the depth prepass.

[beicause]

I think the current solution could only batch depth prepasses for materials that have a default prepass vertex shader and a default prepass fragment shader.

[VictorElHajj]

we can only use the same material bind group layout. But that doesn't make sense - usually depth prepass doesn't need the material bind group.

This is why a optimization was introduced in 0.19 where the material bind group was skipped for the depth prepass, but its still needed in some cases, which is why I added this flag.

Im not sure what you want me to change?

[beicause]

See #24843

[VictorElHajj]

The drawback with that solution is that the performance gain is always lost work custom shaders, while avoiding a new flag. While this PR adds a new flag but still allows the performance gain for custom shaders if not set (by default)

[beicause]

The drawback with that solution is that the performance gain is always lost work custom shaders, while avoiding a new flag. While this PR adds a new flag but still allows the performance gain for custom shaders if not set (by default)

It's ture. But my point is:

1. We shouldn't expose this setting. It's an internal detail and error-prone. Here correctness is more important than performance.
2. In the future, we could allow using different materials for different passes, so you can have full control over the prepass bind groups.