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

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

Closed
cshenton-work opened this issue May 22, 2024 · 6 comments · Fixed by #13712
Closed

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

cshenton-work opened this issue May 22, 2024 · 6 comments · Fixed by #13712
Labels
A-Rendering Drawing game state to the screen C-Docs An addition or correction to our documentation D-Trivial Nice and easy! A great choice to get started with Bevy

Comments

@cshenton-work
Copy link

Bevy version

  • bevy 0.13.2
  • cargo 1.77.1 (e52e36006 2024-03-26)
  • windows 11

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());
    }
}
@cshenton-work cshenton-work added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels May 22, 2024
@bugsweeper
Copy link
Contributor

bugsweeper commented May 22, 2024

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
Copy link
Contributor

bugsweeper commented May 22, 2024

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
Copy link
Member

mockersf commented May 22, 2024

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

@alice-i-cecile alice-i-cecile added C-Docs An addition or correction to our documentation D-Trivial Nice and easy! A great choice to get started with Bevy A-Rendering Drawing game state to the screen and removed C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels May 22, 2024
@alice-i-cecile alice-i-cecile changed the title .exr image loading does not work Image loading docs are not clear enough about required feature flags May 22, 2024
@bugsweeper
Copy link
Contributor

bugsweeper commented May 22, 2024

@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
Copy link
Author

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
Copy link
Member

mockersf commented May 31, 2024

@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

github-merge-queue bot pushed a commit that referenced this issue Aug 16, 2024
# Objective

- Add "Available on crate feature <image format> only." for docs of
image format related types/functions
- Add warning "WARN bevy_render::texture::image: feature "<image
format>" is not enabled" on load attempt
- Fixes #13468 .

## Solution

- Added #[cfg(feature = "<image format>")] for types and warn!("feature
\"<image format>\" is not enabled"); for ImageFormat enum conversions

## Testing

ran reproducing example from issue #13468 and saw in logs
`WARN bevy_render::texture::image: feature "exr" is not enabled`

generated docs with command `RUSTDOCFLAGS="-Zunstable-options
--cfg=docsrs" cargo +nightly doc --workspace --all-features --no-deps
--document-private-items --open` and saw

![image](https://github.com/bevyengine/bevy/assets/17225606/820262bb-b4e6-4a5e-a306-bddbe9c40852)
that docs contain `Available on crate feature <image format> only.`
marks

![image](https://github.com/bevyengine/bevy/assets/17225606/57463440-a2ea-435f-a2c2-50d34f7f55a9)

## Migration Guide
Image format related entities are feature gated, if there are
compilation errors about unknown names there are some of features in
list (`exr`, `hdr`, `basis-universal`, `png`, `dds`, `tga`, `jpeg`,
`bmp`, `ktx2`, `webp` and `pnm`) should be added.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Docs An addition or correction to our documentation D-Trivial Nice and easy! A great choice to get started with Bevy
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants