Add Image constructor specialised for rendering to a texture

[tigregalis]

Objective

Fixes #7358
Redo of #7360

Ergonomics. There's a bunch of enigmatic boilerplate for constructing a texture for rendering to, which could be greatly simplified for the external user-facing API.

Solution

• Take part of the render_to_target example and turn it into a new constructor for Image, with minimal changes beyond the Default implementation.
• Update the render_to_target example to use the new API.

Strictly speaking, there are two small differences between the constructor and the example:

1. The example sets the size when initially constructing the Image, then resizes, but resize sets the size anyway so we don't need to do this extra step.

2. The example sets Image.texture_descriptor.format to TextureFormat::Bgra8UnormSrgb, but the default impl sets this to TextureFormat::Rgba8UnormSrgb via wgpu::TextureFormat::bevy_default(). I don't know what sort of impact this has, but it works on my machine.

I've deliberately chosen to only include width and height as parameters, but maybe it makes sense for some of the other properties to be exposed as parameters.

---

Changelog

Added

Added Image::new_target_texture constructor for simpler creation of render target textures.

---

Notes:

• This is a re-do of Add Image constructor specialised for rendering to a texture #7360 - there's some relevant discussion on code style there.
• The docs for the method want to refer to bevy_render::camera::Camera and bevy_render::camera::RenderTarget::Image. bevy_image used to be part of bevy_render and was split out in the past, and bevy_image doesn't depend on bevy_render. What's the recommendation here?

[tigregalis]

@BenjaminBrienen I've "rebased" this

[JMS55]

Shouldn't hard code to default the texture format. HDR cameras need a different format.

[IceSentry]

Can you also update headless_renderer, gpu_readback, and compute_shader_game_of_life

I think those are all the examples that use this pattern.

(Sorry, that wasn't an approve, just a comment)

[IceSentry]

Shouldn't hard code to default the texture format. HDR cameras need a different format.

Yeah, this should either take a hdr parameter or just a texture format. A hdr parameter could be enough since this is just for simpler use cases.

[tigregalis]

Shouldn't hard code to default the texture format. HDR cameras need a different format.

Yeah, this should either take a hdr parameter or just a texture format. A hdr parameter could be enough since this is just for simpler use cases.

So to clarify, the suggestion is to either:

1. add a hdr parameter, I assume a bool, but if true, what's the intended TextureFormat variant here? Is it TextureFormat::AstcHdr { block: ???, AstcChannel::Hdr }
2. have the user pass in the TextureFormat themselves, but same question as 1. what would the user use if they wanted to use a HDR camera?

[tigregalis]

In the discord there were a few comments:

1. BevyDefault which is defined within bevy_image and only has a single implementer (TextureFormat) should be removed and any calls to TextureFormat::bevy_default() should be replaced with (now) explicitly bevy_image::TEXTURE_FORMAT_SDR.

2. Perhaps this new constructor really wants to live in bevy_render, either as a free function or as an extension trait.

3. Perhaps TEXTURE_FORMAT_SDR and TEXTURE_FORMAT_HDR should really live in bevy_render as associated constants on ViewTarget, but the other constructors on Image also want to use TEXTURE_FORMAT_SDR (as a "default" TextureFormat).

4. This exact expression is used in many, many places throughout the bevy codebase, with the only change being the predicate (hdr), but will be addressed by Allow specifying intermediate textures for cameras. #13146

if (hdr) {
    ViewTarget::TEXTURE_FORMAT_HDR
} else {
    TextureFormat::bevy_default()
}

For now I have left the constructor in bevy_image, added a hdr: bool parameter, created the two constants, and have ViewTarget::TEXTURE_FORMAT_HDR point to bevy_image::TEXTURE_FORMAT_HDR.

[tigregalis]

Can you also update headless_renderer, gpu_readback, and compute_shader_game_of_life

I think those are all the examples that use this pattern.

(Sorry, that wasn't an approve, just a comment)

I've done this but the discrepancies make me think maybe these should have their own helper constructors, possibly with guidance in the doc comment.

• headless_renderer: additional texture usage COPY_SRC
• gpu_readback: additional texture usage COPY_SRC + STORAGE_BINDING, different texture format R32Uint, different asset usage RENDER_WORLD
• compute_shader_game_of_life: different texture usage COPY_DST + STORAGE_BINDING + TEXTURE_BINDING, different texture format R32Float, different asset usage RENDER_WORLD

[IceSentry]

Hey, sorry for forgetting about this PR. After looking at the change again, I think I'm almost happy with everything, but I realize now that maybe we should just have the texture format be a direct parameter to the function instead of an HDR flag. That way it can easily be used in other contexts and people can inline the if hdr pattern if they need it and if they don't it's pretty easy to just provide the right texture format.

I think it's fine if more advanced use cases just modify the resulting image like you did in the examples. It's still nicer to have the constructor as a starting point for that boilerplate.

[tigregalis]

@IceSentry

I've updated Image::new_target_texture to take TextureFormat parameter instead of hdr flag.

I've improved the documentation.

Also fixed a bug in the headless_renderer example: since no windows are opened, it immediately exits, so it never actually executes the render and write to disk. This is possibly more of a bug in the WindowPlugin though?

The examples I've updated are:

• compute_shader_game_of_life
• render_to_texture
• headless_renderer

Since example gpu_readback now uses the Image::new_uninit constructor, I've opted to revert that change and leave that as is; Image::new_target_texture felt like a misuse for it anyway.

[IceSentry]

This looks good, but could you make the changes to add the TEXTURE_FORMAT consts to bevy_image in a separate PR? I think it's a worthwhile change, but I'd prefer if it wasn't bundled with a feature PR.

[IceSentry]

Approved, but I would still like the texture format changes in bevy_image to be in a separate PR

[tigregalis]

Approved, but I would still like the texture format changes in bevy_image to be in a separate PR

Dropped the constants out of this PR.