Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ImageFormatSetting::Guess to image loader #13575

Merged
merged 2 commits into from
May 31, 2024

Conversation

Azorlogh
Copy link
Contributor

@Azorlogh Azorlogh commented May 30, 2024

Objective

  • Allow using image assets that don't have an extensions and whose format is unknown. This is useful for loading images from arbitrary HTTP URLs.

Solution

  • This PR adds a new variant to ImageFormatSetting called Guess. The loader will use image::guess_format to deduce the format based on the content of the file.

Testing

  • I locally removed the extension of bevy_bird_dark, and ran a modified version of the sprite example:
//! Displays a single [`Sprite`], created from an image.

use bevy::{
    prelude::*,
    render::texture::{ImageFormatSetting, ImageLoaderSettings},
};

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_systems(Startup, setup)
        .run();
}

fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
    commands.spawn(Camera2dBundle::default());
    commands.spawn(SpriteBundle {
        texture: asset_server
            .load_with_settings("branding/bevy_bird_dark", |s: &mut ImageLoaderSettings| {
                s.format = ImageFormatSetting::Guess
            }),
        ..default()
    });
}

Changelog

Added

ImageFormatSetting::Guess to automatically guess the format of an image asset from its content.

@Azorlogh Azorlogh marked this pull request as draft May 30, 2024 10:18
@Azorlogh Azorlogh marked this pull request as ready for review May 30, 2024 10:29
@Azorlogh
Copy link
Contributor Author

Note: when loading images without extension, a warning gets logged to the console:
Multiple AssetLoaders found for Asset: Some(TypeId { t: (6046691006513703432, 8515571476300803229) }); Path: Some(branding/bevy_bird_dark); Extension: None
However this warning was already present on main when using ImageFormatSetting::Format. I believe it's because of HdrTextureLoader. ImageLoader is always registered after HdrTextureLoader, so it should take precedence over it.

@alice-i-cecile alice-i-cecile added C-Feature A new feature, making something new possible A-Rendering Drawing game state to the screen A-Assets Load files from disk to use for things like images, models, and sounds S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels May 30, 2024
@alice-i-cecile alice-i-cecile added C-Usability A targeted quality-of-life change that makes Bevy easier to use X-Uncontroversial This work is generally agreed upon D-Straightforward Simple bug fixes and API improvements, docs, test and examples and removed C-Feature A new feature, making something new possible labels May 30, 2024
@mockersf mockersf added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels May 30, 2024
@mockersf
Copy link
Member

random ci failure, trying to retrigger it

@alice-i-cecile alice-i-cecile added this pull request to the merge queue May 30, 2024
Merged via the queue into bevyengine:main with commit 31955cc May 31, 2024
27 checks passed
@alice-i-cecile
Copy link
Member

The first commit from this PR was taken from 6b3ffac by @robtfm. @Azorlogh, in the future, either add Co-authored by: to your Git commit message or base your branch directly on old work to make sure credit is preserved properly.

@Azorlogh
Copy link
Contributor Author

The first commit from this PR was taken from 6b3ffac by @robtfm. @Azorlogh, in the future, either add Co-authored by: to your Git commit message or base your branch directly on old work to make sure credit is preserved properly.

Oh wow. I don't recall seeing that PR/commit at all 😲 I just wrote this for internal use in my app (as an ugly wrapper around ImageLoader) and then up-streamed it here after some cleanup. I didn't realize there was already another one opened.
I should have checked for duplicates, my apologies.

@alice-i-cecile
Copy link
Member

No worries! This change was pretty mechanical: very reasonable that the first step looked extremely similar <3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Assets Load files from disk to use for things like images, models, and sounds A-Rendering Drawing game state to the screen C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it X-Uncontroversial This work is generally agreed upon
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants