bevyengine / bevy

A refreshingly simple data-driven game engine built in Rust
https://bevyengine.org
Apache License 2.0
34.21k stars 3.34k forks source link

Image loading docs are not clear enough about required feature flags #13468

Open cshenton-work opened 1 month ago

cshenton-work commented 1 month ago

Bevy version

What you did

Loaded a known good .exr from hdri-haven with AssetServer.

What went wrong

2024-05-22T04:28:44.401424Z ERROR bevy_asset::server: Failed to load asset 'C:\Users\...\...exr' with asset loader 'bevy_render::texture::image_loader::ImageLoader': Could not load texture file: Error reading image file C:\Users\...\...exr: failed to load an image: The image format OpenExr is not supported, this is an error in `bevy_render`.

Additional info

Here's a minimal repro, you can download a sample .exr from Poly Haven To test, run the application, and drag and drop the .exr into the window.

use bevy::prelude::*;

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

fn drag_and_drop_exr(
    mut events: EventReader<FileDragAndDrop>,
    asset_server: Res<AssetServer>,
) {
    // Add the new entity
    for event in events.read() {
        let FileDragAndDrop::DroppedFile {
            window: _,
            path_buf: path,
        } = event
        else {
            continue;
        };

        let Some(ext) = path.extension() else {
            continue;
        };

        if ext != "exr" {
            continue;
        }

        let _exr: Handle<Image> = asset_server.load(path.clone());
    }
}
bugsweeper commented 1 month ago

I tried your example, it doesn't load exr if I don't enable exr feature bevy = { version = "0.13.2" } But if enable it, example doesn't print any error bevy = { version = "0.13.2", features = ["exr"] } @cshenton-work That because this feature affects loader initialization here @alice-i-cecile Do we need make some note in docs anywere or we should just close issue?

bugsweeper commented 1 month ago

I think ImageFormat or ExrTextureLoader are good places for such note

In case of ImageFormat note should include mention of exr, hdr, basis-universal, png, dds, tga, jpeg, bmp, ktx2, webp and pnm feature

mockersf commented 1 month ago

#[cfg_attr(docsrs, doc(cfg(feature = "exr")))] on what is behind the feature should mark it in docs.rs https://github.com/rust-lang/rust/issues/43781

bugsweeper commented 1 month ago

@mockersf Please, help: I added attributes

/// Loads EXR textures as Texture assets
#[derive(Clone, Default)]
#[cfg(feature = "exr")]
#[cfg_attr(docsrs, doc(cfg(feature = "exr")))]
pub struct ExrTextureLoader;

and at fields

    Gif,
    #[cfg_attr(docsrs, doc(cfg(feature = "exr")))]
    OpenExr,

then ran command cargo doc --workspace --all-features --no-deps --document-private-items --open and saw that nothing changes image image What did I wrong? Or maybe this rustdoc feature is not ready yet?

cshenton-work commented 1 month ago

Worth noting that this issue is true across the board. If you have hdr enabled but not png, and try to load a .png (say, if you're writing an example for a bevy plugin that itself does not require png support), then you get a cryptic error message about the .hdr loading failing.

There should probably be feature flag checks in the loader that just say "this feature is not enabled" when you try to load an image type you don't have the feature for.

mockersf commented 1 month ago

@mockersf Please, help: I added attributes

What did I wrong? Or maybe this rustdoc feature is not ready yet?

Sorry, missed your message

I think you need to add RUSTDOCFLAGS: -Zunstable-options --cfg=docsrs as an environment variable to build docs as docs.rs, and use a nightly version of rust