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

Tonemapping example image loader not working #10395

Open
DGriffin91 opened this issue Nov 5, 2023 · 5 comments
Open

Tonemapping example image loader not working #10395

DGriffin91 opened this issue Nov 5, 2023 · 5 comments
Labels
A-Assets Load files from disk to use for things like images, models, and sounds C-Bug An unexpected or incorrect behavior D-Complex Quite challenging from either a design or technical perspective. Ask for help!

Comments

@DGriffin91
Copy link
Contributor

DGriffin91 commented Nov 5, 2023

Bevy version 32a5c7d

Previously, in the tonemapping example an exr or hdr image file could be dragged and dropped onto the window to view the image with the selected tonemapping settings. This no longer works.

The issue was introduced at 35073cf with #9885

thread 'Compute Task Pool (21)' panicked at 'called `Result::unwrap()` on an `Err` value: InvalidSourceSyntax', crates\bevy_asset\src\path.rs:115:70
@DGriffin91 DGriffin91 added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Nov 5, 2023
@cart
Copy link
Member

cart commented Nov 5, 2023

General answer is "its complicated".
The AssetSource / AssetReader / AssetWriter abstraction is intentionally scoped to specific folders to allow us to make assumptions about them (ex: this folder is processed, this source might not come from a filesystem, etc). AssetPaths are also intended to be platform (and source-type) agnostic. But absolute paths like C:\some\path are a windows-specific convention.

Of course, we should support loading assets in these cases, but I think how we do that needs to change. I think it should probably be a combination of two things:

  1. Enable mounting new asset sources at runtime (instead of needing to anticipate at startup)
  2. Create an abstraction that can convert from absolute (sometimes OS-specific) file paths to equivalent platform-agnostic AssetPaths. In the case of a windows-specific path like the one above, this would involve mounting a new C asset source and converting windows-specific paths to platform-agnostic AssetPaths. (ex: C:\some\asset.png would become C://some/asset.png.

Some discussion here on discord:
https://discord.com/channels/691052431525675048/1164297864441249823

@cart
Copy link
Member

cart commented Nov 5, 2023

Note that we are planning on being more principled (and platform-agnostic) about AssetPaths in the future. Ex: you should use / instead of \, windows-style "drives" would not be allowed (ex: C:\), etc.

@DGriffin91
Copy link
Contributor Author

DGriffin91 commented Nov 5, 2023

I think this makes sense as long as there's a reasonable way for an end user to specify a file path in a typical OS native fashion. There should be a reasonably straightforward way to make a bevy app that allows the end user to use C:\, or \ instead of /

It sounds like the abstraction in 2. is designed for this purpose. Just clarifying where the more principled paths are required.

(Some example applications would be: digital content creation, scientific visualization, anything with a OS file dialog)

@cart
Copy link
Member

cart commented Nov 5, 2023

Yup fully agreed that this is necessary. One "quick fix" for these apps that doesn't require Bevy changes: change the root asset folder to the root of the current drive (ex: / on linux/macos C:\ on windows). Then you can just pass in the full absolute paths (unchanged on macos/linux and with the C: stripped out on windows).

@DGriffin91
Copy link
Contributor Author

What about for files on other drive letters? I personally only have windows, users, etc... on C:\ and keep everything else on other drives.

@alice-i-cecile alice-i-cecile added A-Assets Load files from disk to use for things like images, models, and sounds D-Complex Quite challenging from either a design or technical perspective. Ask for help! and removed S-Needs-Triage This issue needs to be labelled labels Nov 8, 2023
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 C-Bug An unexpected or incorrect behavior D-Complex Quite challenging from either a design or technical perspective. Ask for help!
Projects
None yet
Development

No branches or pull requests

3 participants