Expand FallbackImage to include a GpuImage for each possible TextureViewDimension

[bonsairobo]

Objective

Fixes #6920

Solution

From the issue discussion:

From looking at the AsBindGroup derive macro implementation, the fallback image's TextureView is used when the binding's Option<Handle<Image>> is None. Because this relies on already having a view that matches the desired binding dimensions, I think the solution will require creating a separate GpuImage for each possible TextureViewDimension.

---

Changelog

Users can now rely on FallbackImage to work with a texture binding of any dimension.

[bonsairobo]

Currently a draft b/c I haven't tested that this actually fixes the panic yet. I need to get my downstream crate (and it's dependencies that depend on bevy) building based on these changes first.

OK PR is ready for review.

[github-actions]

Example alien_cake_addict failed to run, please try running it locally and check the result.

[github-actions]

Example alien_cake_addict failed to run, please try running it locally and check the result.

[bonsairobo]

Should I be putting the integration test into an example? Is that the right pattern?

Note for reviewers: You can reproduce the error from #6920 by applying only the 1st commit and running the test example. The 2nd commit fixes the error.

[IceSentry]

Would be nice if the example was able to run as an actual test case instead of being an example, but this is out of scope of this PR since we don't have the infrastructure for this right now.

[bonsairobo]

Should be good to go.

[alice-i-cecile]

Much clearer, thanks :) I still don't have quite enough expertise to review this directly though, so I'm going to hold off on an approval.

[bonsairobo]

Much clearer, thanks :) I still don't have quite enough expertise to review this directly though, so I'm going to hold off on an approval.

Ok. We have an approval from IceSentry. Is that good enough or what happens next?

[IceSentry]

It still needs a second approval, there's been a tiny bit of pushback on discord. I wouldn't say it's a blocker, but the gist of it is that this solution doesn't scale well to support the other attributes that aren't currently supported like the texture format for example.

Also, I don't mind my real name being here, but not everyone knows that it's me.

[JMS55]

Looks good to me. I'm not really concerned about combinatorial explosion due to number of parameters. Off the top of my head, I can't see why a user would care about the texture format. Binding it and using it in a shader is the same either way. The only thing that matters is: what value you get out (black/white), texture dimensions, and what format you read values in from the shader (float, uint, etc. Anything but float would be very rare though). In fact, we could use something like r32float/uint (or r8 on native) to save a very tiny amount of memory.

Tldr; I don't see a current use case for larger amounts of fallbacks beyond what we have now.

[IceSentry]

I know that @DGriffin91 needed this for deferred shading which is why it was mentioned. It may not have been texture format, but it was definitely because of some other parameter not being handled.

[DGriffin91]

@IceSentry yes, texture format and sample count.

[bonsairobo]

@DGriffin91 @IceSentry If we want to support all possible TextureFormats, then we probably don't want to have a field for each combination. We could use a HashMap<(TextureViewDimension, TextureFormat), GpuImage> and lazily create the images. This could happen in a follow-up issue/PR.

[alice-i-cecile]

I've run the example locally and while it doesn't panic I'm just getting a grey screen. Is that intentional?

Either way, please add a description to the module comment at the top of the example explaining what "correct" behavior should look like.

[bonsairobo]

I've run the example locally and while it doesn't panic I'm just getting a grey screen. Is that intentional?

Yea the shader doesn't do anything, it's just testing that it doesn't crash when attempting to bind the images.

Either way, please add a description to the module comment at the top of the example explaining what "correct" behavior should look like.

Will do.

[alice-i-cecile]

Much better, thanks!