Use ShaderDefVal to avoid redundant code

[MoskalykA]

Objective

The purpose of this pull request is to complete this issue (closes #9256)

Migration guide

The MESH_BINDGROUP_1 shader def has been replaced by the MESH_BINDGROUP ShaderDefVal.

If in your shader you previously used:

#ifdef MESH_BINDGROUP_1
@group(1) @binding(0)
var<storage> mesh: array<Mesh>;
#else // MESH_BINDGROUP_1
@group(2) @binding(0)
var<storage> mesh: array<Mesh>;
#endif // MESH_BINDGROUP_1

You can now write it:

@group(#{MESH_BINDGROUP}) @binding(0)
var<storage> mesh: array<Mesh>;

Other important change: You should now add the MESH_BINDGROUP define even if the binding group is 0.
Add this line to your shader specialization pipeline:

shader_defs.push(ShaderDefVal::UInt("MESH_BINDGROUP".into(), 2));

[github-actions]

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

[github-actions]

It looks like your PR is a breaking change, but you didn't provide a migration guide.

Could you add some context on what users should update when this change get released in a new version of Bevy?
It will be used to help writing the migration guide for the version. Putting it after a ## Migration Guide will help it get automatically picked up by our tooling.

[nicopap]

You need to update the CPU-side code as well.

Replace all instance of:

shader_defs.push("MESH_BINDGROUP_1".into());

with

shader_defs.push(ShaderDefVal::UInt("MESH_BINDGROUP".into(), 1));

You'll also need to add

shader_defs.push(ShaderDefVal::UInt("MESH_BINDGROUP".into(), 2));

to existing pipelines that do not add the MESH_BINDGROUP_1 binding.

Also, could you add a "closes #9256" to your PR description? So that it automatically closes the linked issue when this gets merged.

A summary on writing the migration guide lines:

---------

### Migration guide

The `MESH_BINDGROUP_1` shader def has been replaced by the `MESH_BINDGROUP` `ShaderDefVal`.

If in your shader you previously used:
``\`
#ifdef MESH_BINDGROUP_1
@group(1) @binding(0)
var<storage> mesh: array<Mesh>;
#else // MESH_BINDGROUP_1
@group(2) @binding(0)
var<storage> mesh: array<Mesh>;
#endif // MESH_BINDGROUP_1
``\`

You can now write it:

``\`
@group(#{MESH_BINDGROUP}) @binding(0)
var<storage> mesh: array<Mesh>;
``\`

**Other important change**: You should now add the `MESH_BINDGROUP` define even if the binding group is 0.
Add this line to your shader specialization pipeline:

``\`
shader_defs.push(ShaderDefVal::UInt("MESH_BINDGROUP".into(), 2));
``\`

[MoskalykA]

Thank you very much for these details. I have a quick question about the pipelines. Should I add the code for the SpecializedMeshPipeline implementations or for literally all pipelines?

[nicopap]

I'm not 100%, try only SpecializedMeshPipeline.

Run shader_prepass and swap between prepass mod to see if it panics or not.

[robtfm]

Can we please have a default of 2 for when the def is not specified. It used to be necessary to specify for every pipeline and it was very annoying, I would prefer not to go back to that situation again.

[nicopap]

Ooops my bad, my instructions mentioned a default of 0, I updated them to 2.

[nicopap]

So how would we setup a default? Maybe passing the shader_defs as an argument to the specialize method on SpecializedMeshPipeline?

[MoskalykA]

I'm not 100%, try only SpecializedMeshPipeline.

Run shader_prepass and swap between prepass mod to see if it panics or not.

Only on SpecializedMeshPipeline works perfectly

[robtfm]

So how would we setup a default? Maybe passing the shader_defs as an argument to the specialize method on SpecializedMeshPipeline?

I was thinking the shader could branch on #ifdef as it does currently (I didn’t look at the code yet though)

[JMS55]

MESH_BIND_GROUP_1 was removed #10485