Add Image constructor specialised for rendering to a texture

[tigregalis]

Objective

Fixes #7358

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.

[wilk10]

Thanks a lot!

In my opinion this is helps reduce the cognitive load when reading the example for someone that just wants to understand how to use the feature, and reduces the boilerplate when actually using it.

I have left a couple of small comments.

[wilk10]

This is just my preference, but if we are adding a convenience method to be used in the example (and I agree with the proposal), then here I'd think it's better to specify the whole TextureDescriptor like in the example, instead of mutating only the usage field, so that when looking at the implementation of this method, the default values can be seen at a glance.

[tigregalis]

Do you mean, you would prefer that we use struct update syntax, and spread Default::default()?

Or do you mean, don't use the Default impl and explicitly specify the property values like in the example:

    let mut image = Image {
        texture_descriptor: TextureDescriptor {
            label: None,
            size,
            dimension: TextureDimension::D2,
            format: TextureFormat::Bgra8UnormSrgb,
            mip_level_count: 1,
            sample_count: 1,
            usage: TextureUsages::TEXTURE_BINDING
                | TextureUsages::COPY_DST
                | TextureUsages::RENDER_ATTACHMENT,
        },
        ..default()
    };

[wilk10]

i mean like in the example, the latter you said

[tigregalis]

Here are a couple options.
A) struct literal syntax - matches the example, but repeats a lot of the default data

    pub fn new_target_texture(width: u32, height: u32) -> Self {
        let size = Extent3d {
            width,
            height,
            ..Default::default()
        };
        let format = wgpu::TextureFormat::bevy_default();
        let data = vec![0; size.volume() * format.pixel_size()];

        Image {
            data,
            texture_descriptor: wgpu::TextureDescriptor {
                size,
                format,
                dimension: wgpu::TextureDimension::D2,
                label: None,
                mip_level_count: 1,
                sample_count: 1,
                usage: wgpu::TextureUsages::TEXTURE_BINDING
                    | wgpu::TextureUsages::COPY_DST
                    | wgpu::TextureUsages::RENDER_ATTACHMENT,
            },
            ..Default::default()
        }
    }

B) Self::new() + usage - uses the new constructor, and is similar implementation-wise to the new and new_fill constructors.

    pub fn new_target_texture(width: u32, height: u32) -> Self {
        let size = Extent3d {
            width,
            height,
            ..Default::default()
        };
        let format = wgpu::TextureFormat::bevy_default();
        let data = vec![0; size.volume() * format.pixel_size()];
        let dimension = wgpu::TextureDimension::D2;

        let mut image = Self::new(size, dimension, data, format);

        image.texture_descriptor.usage = wgpu::TextureUsages::TEXTURE_BINDING
            | wgpu::TextureUsages::COPY_DST
            | wgpu::TextureUsages::RENDER_ATTACHMENT;
            
        image
    }

[wilk10]

My preference is on A because it's more explicit, but i'm flexible

[wilk10]

This is pretty minor, but when looking at the example using resize always felt a bit "hacky". In line with improving ergonomics, maybe we could add an extra method called fill_with_zeros() with no arguments, that is called here and doesn't change the size, only calls self.data.resize() internally. Wdyt?

[tigregalis]

Technically resize extends the data with zeroes (or truncates it).
And actually for the Default impl data = vec![255; 4], so you actually end up with [255, 255, 255, 255, 0, 0, 0, 0, ...] which is probably not really what's intended, but that's how the example was originally constructed.
Maybe the Default impl should be zeroes data = vec![0; 4] but I don't know if there's any implications with that.

Alternatively for this method, we could build the data directly, rather than calling resize at all. Presumably the compiler would optimise out the first assignment to data.

    pub fn new_target_texture(width: u32, height: u32) -> Self {
        let mut image = Self::default();

        let size = Extent3d {
            width,
            height,
            ..Default::default()
        };

        image.data = vec![0; size.volume() * image.texture_descriptor.format.pixel_size()];
        image.texture_descriptor.size = size;
        
        image.texture_descriptor.usage = wgpu::TextureUsages::TEXTURE_BINDING
            | wgpu::TextureUsages::COPY_DST
            | wgpu::TextureUsages::RENDER_ATTACHMENT;

        image
    }

I'm not sure about additional methods, but something like this sort of makes sense:

    pub fn resize(&mut self, size: Extent3d) {
        self.resize_with(&[0]);
    }
    pub fn resize_with(&mut self, size: Extent3d, pixel: &[u8]) {
        todo!();
    }
    pub fn fill_with(&mut self, pixel: &[u8]) {
        for current_pixel in self.data.chunks_exact_mut(pixel.len()) {
            current_pixel.copy_from_slice(pixel);
        }
    }
    pub fn fill_with_zeroes(&mut self) {
        self.fill_with(&[0]);
    }

Unless we're intending to use these methods to implement this method, could this be a separate PR?

[wilk10]

Yes, setting the data directly I guess would work too.
Not sure about the current default, it is a bit odd indeed. Maybe let's hear what the others say.

Regarding extra methods, it can definitely be deferred to another PR and be discussed there.

[github-actions]

Your PR increases Bevy Minimum Supported Rust Version. Please update the rust-version field in the root Cargo.toml file.

[tigregalis]

Uh, I screwed this up. I'll submit a new PR.