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

Feat: Improved GLTF loader, implement typed asset loading #378

Closed
wants to merge 8 commits into from

Conversation

skairunner
Copy link

Two features:

(1) Issue #367 : I updated asset loading and implemented load_typed() to use alongside load_untyped(). I did this by changing the extensions-to-loader hashmap to have a key of (TypeId, String) instead of the previous String. This cleanly supports having multiple AssetLoader with different T but the same file extension.

(2) Implemented texture reading for GLTF.

(2a) Retouched the GLTF loading procedure so that instead of checking just the first node, it reads all nodes, meshes and textures. Then it returns just the first valid mesh/texture, depending on what you're requesting. This opens the door for potentially loading multiple assets from a GLTF file, but that is a much larger scope than this PR, because it is not possible to create and return multiple handles from within an AssetLoader at the moment.

Future work:

  • This work was mostly orthogonal to loading GLTFs as scenes.
  • A GLTF has to be parsed multiple times in order to get everything needed to load a model. This is highly inefficient -- as you can see in the 3d/load_model example, it takes a moment for the cubes to appear -- but as per (2a) it's not possible to easily solve right now.
  • We still only allow returning the first mesh or texture from a GLTF, when it can contain any number of files.

@skairunner
Copy link
Author

@karroffel karroffel added A-Assets Load files from disk to use for things like images, models, and sounds C-Enhancement A new feature labels Aug 28, 2020
crates/bevy_asset/src/asset_server.rs Outdated Show resolved Hide resolved
crates/bevy_render/src/color.rs Outdated Show resolved Hide resolved
crates/bevy_render/src/color.rs Outdated Show resolved Hide resolved
@andreasterrius
Copy link

Would this PR solve #367 ?

@skairunner
Copy link
Author

Would this PR solve #367 ?

I think it should. You can't request, say, a Texture from a hypothetical OBJ loader because the list of extensions to loaders is keyed by type as well as extension.

@skairunner
Copy link
Author

Merge conflict resolved.

@Lowentwickler
Copy link
Contributor

One of the possible workarounds for scene loading could be a load_scene method in AssetServer that will work similar to load_from_folder, but instead of the recursive directory walk, it can open a GLTF file and walk through assets, adding them.

I have suggested to use URI-like notation to access the assets and @cart liked it. For example, if you load spider.gltf, you can access assets using artificial paths like:

  • spider.gltf#model -> mesh
  • spider.gltf#skin -> texture
  • spider.gltf#walk -> walk animation
  • spider.gltf#jump -> jump animation

Alternative to # characters could be ? or :

@skairunner
Copy link
Author

I was going to implement the URI-like notation as well, way back when I first wrote this PR, but I ran out of steam, partially because it didn't seem like it would get accepted soon.

@cart
Copy link
Member

cart commented Sep 17, 2020

Yeah sorry for not responding sooner. I intentionally put this off because the asset system is either going to be replaced by atelier-assets or modified to account for cases like this better.

I don't want to merge anything that extends and/or hacks around the current asset system, as it will almost certainly be replaced in short order.

If we go with atelier, aspects of the implementation in this pr will need to change. If we go with extending the current asset system, aspects of this pr will need to change.

Again, I'm really sorry for not keeping you in the loop. I really appreciate the work you put in here and leaving you in the dark on my current thoughts for so long was disrespectful.

@skairunner
Copy link
Author

skairunner commented Sep 17, 2020 via email

@Lowentwickler
Copy link
Contributor

I believe current bevy_asset can stay as a part of the engine for longer, even as bevy_asset_legacy or bevy_legacy/bevy_asset, being deprecated, whatever path you choose to go either rewrite or switch to atelier.

Another thought on the topic, is that it could be optional choice to stay with atelier, or native bevy_asset, or even a 3rd party bevy_asset package, because for some projects atelier could be kind of an overkill.

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-Enhancement A new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants