guess image formats

[robtfm]

Objective

provide an option to use image::guess_format to determine the image format, and make it the default.

useful when loading images of unknown format (eg from urls without extension/mimetype), and more robust when loading mislabelled files.

[alice-i-cecile]

Thoughts:

1. This is a useful feature!
2. This is a good default.
3. I'd really like to try to slowly improve the documentation status of Bevy's rendering: documenting the code we're adding and touching here seems reasonable.
4. On some level I'd like to check and warn if the extension does not match the inferred type. However I'm not sure if this is the right spot for it: it will slow down loading in the happy path.

[robtfm]

4. On some level I'd like to check and warn if the extension does not match the inferred type. However I'm not sure if this is the right spot for it: it will slow down loading in the happy path.

we could do that here, i don't think the perf impact would be noticeable (when compared to loading the binary data, converting internally, pushing it to gpu, etc) .

if so i'd like to add another extension (say .image) which would never trigger the warning. i'm downloading files with no extension where all i know is that they are images, so i have to hack some recognised extension onto the "path" (which is acually a urn plus base url encoded into a path) to get the loader to pick up the asset, and the warning would be noisy and useless for me in that case.

[alice-i-cecile]

@robtfm that makes sense! Let's add that check and this special extension type here.

[rparrett]

so i have to hack some recognised extension onto the "path"

That is still necessary after #10153?

[robtfm]

That is still necessary after #10153?

Nice, I hadn’t seen that - I’ll check there’s no issues with using extensionless and then remove the “.image” extension

[bushrat011899]

Nice change! There's a little more work required to get something like:

commands.spawn(SpriteBundle {
    texture: asset_server.load("branding/bevy_bird_dark"),
    ..default()
});

Since there are multiple asset loaders registered which can handle an extensionless path for an Image. But that's beyond the scope for this PR.

[bushrat011899]

Minor nit, could this be replaced with an implementation of TryFrom instead?

Example

/// Error returned when converting between Bevy and [image] image format types.
#[derive(Debug, Error)]
#[error("Image format '{image:?}' maps to '{bevy:?}'")]
pub struct UnsupportedImageFormatMapping {
    /// Image format type from [image]
    pub image: Option<image::ImageFormat>,
    /// Bevy's image format type
    pub bevy: Option<ImageFormat>,
}

impl TryFrom<image::ImageFormat> for ImageFormat {
    type Error = UnsupportedImageFormatMapping;

    fn try_from(format: image::ImageFormat) -> Result<Self, Self::Error> {
        let format = match format {
            image::ImageFormat::Avif => ImageFormat::Avif,
            image::ImageFormat::Bmp => ImageFormat::Bmp,
            // ...
            _ => {
                return Err(Self::Error {
                    image: Some(format),
                    bevy: None,
                })
            }
        };

        Ok(format)
    }
}

impl TryFrom<ImageFormat> for image::ImageFormat {
    type Error = UnsupportedImageFormatMapping;

    fn try_from(format: ImageFormat) -> Result<Self, Self::Error> {
        let format = match format {
            ImageFormat::Avif => image::ImageFormat::Avif,
            ImageFormat::Bmp => image::ImageFormat::Bmp,
            // ...
            _ => {
                return Err(Self::Error {
                    image: None,
                    bevy: Some(format),
                })
            }
        };

        Ok(format)
    }
}

impl ImageFormat {
    // ...

    /// Attempts to convert to [`image::ImageFormat`].
    ///
    /// Unsupported formats will become [`None`].
    pub fn as_image_crate_format(&self) -> Option<image::ImageFormat> {
        (*self).try_into().ok()
    }

    /// Attempts to convert from [`image::ImageFormat`].
    ///
    /// Unsupported formats will become [`None`].
    pub fn from_image_crate_format(format: image::ImageFormat) -> Option<Self> {
        format.try_into().ok()
    }

    // ...
}

I personally think it's nicer to use the core traits when appropriate.

[robtfm]

i did this a bit more succinctly - i don't think the error type adds any value since you always have the inputs at the callsite.

sadly it removes some of the comments, sorry alice!

[robtfm]

Since there are multiple asset loaders registered which can handle an extensionless path for an Image. But that's beyond the scope for this PR.

right ... the built-in ones could probably be unified, but users might want to register other image loaders. i guess we may want a "default" loader for a given asset type (or possibly to try all the loaders?), but for now i'll leave the magic extension.

edit: probably asset_server.load_with::<ImageLoader>("bevy_bird") would be easiest.

[alice-i-cecile]

@robtfm once merge conflicts are resolved I'll merge this in for you.

[alice-i-cecile]

@robtfm I'll merge this ASAP once merge conflicts are fixed.

[alice-i-cecile]

Completed in #13575