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

Allow GltfLoader to load user defined materials. #13564

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

mintlu8
Copy link
Contributor

@mintlu8 mintlu8 commented May 29, 2024

Objective

Allow GltfLoader to load materials other than StandardMaterial. This allows the GltfLoader to load user defined materials, for example a toon shader.

Solution

  • Allow GltfPlugin, GltfLoader to specify a generic parameter Material, by default StandardMaterial.

  • Added FromStandardMaterial that constructs a new material from a StandardMaterial and gltf_extras.
    Notable types that implement this are StandardMaterial and ExtendedMaterial<StandardMaterial, impl FromStandardMaterial>, which can serve to extend the StandardMaterial's functionalities.

  • Added with_default_material on GltfLoaderSettings to change the default material used by GltfLoader. Added with_standard_material on GltfPlugin to do it globally.

  • Allow user to register named FromStandardMaterial materials onto GltfPlugin. We will seach for "material": "name" in gltf_extras, if the corresponding name is found, the corresponding material will be used instead of the default one.

toon

Notable Footguns Introduced

Assets are always cached by path. Meaning loading a different standard material with a previous handle alive will instead obtain the previously loaded handle. This is hard to avoid given the current implementation of bevy_assets.

Testing

Added an example showcasing these features.

Changelog

Added FromStandardMaterial trait that constructs a new material from a StandardMaterial and gltf_extras json string.

Added a setting to GltfPlugin for swapping out StandardMaterial globally.

Added the ability to register material deserializers onto GltfPlugin.

Added a setting to GltfLoaderSettings to use a different default material.

GltfLoader now reads gltf_extras attribute material to determine which material to use.

Gltf now contains untyped handles. Added method for retrieving typed handles.

NEED REVIEW
MetaTransform now copies to parent Handles (i.e Handle<Gltf>) from loading child assets.

assets.load_with_settings::<Scene>("scene.glb", |s| s.with_default_material::<ToonShader>());

Migration Guide

Gltf now contains untyped handles. Added method for retrieving typed handles from Gltf.

Copy link
Contributor

The generated examples/README.md is out of sync with the example metadata in Cargo.toml or the example readme template. Please run cargo run -p build-templated-pages -- update examples to update it, and commit the file change.

@alice-i-cecile alice-i-cecile requested a review from viridia May 29, 2024 14:24
@alice-i-cecile alice-i-cecile added C-Feature A new feature, making something new possible A-Assets Load files from disk to use for things like images, models, and sounds M-Needs-Release-Note Work that should be called out in the blog due to impact S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels May 29, 2024
@alice-i-cecile
Copy link
Member

@MalekiRe you wanted this as well, correct?

@viridia
Copy link
Contributor

viridia commented May 29, 2024

I'm not sure this solves my issue. As described, the PR swaps out every material in a GLTF, and they must all be the same type. That doesn't describe my use case at all:

  • When loading a GLTF, some materials will be swapped out, while others will not be.
  • Not all swapped materials will be the same type.

Here's an example: Say I have a fireplace created in Blender. The fireplace has a special "flames" material that uses an animated texture. In Blender, I create a custom property "flames: true" an add it to the material. However, the rest of the fireplace - the bricks and so on - use normal materials.

When I load the model (in my previous non-Bevy engine), I examine the GLTF extras field and decide what type of material to substitute based on the custom properties. For most meshes I leave the material as-is - the standard material is good enough. However, for the flames material, I replace the standard material with the special animated shader material.

There are more complex scenarios. In my blender files, I also have a custom property called "outline", which creates a cartoon outline around the object. This requires two materials (because the outline is a separate pass). There are two ways to accomplish this:

  • Clone the mesh, and replace the clone's material with the outline shader.
  • Insert two materials into a single mesh - because of the way Bevy's pipeline works, a mesh with two different material components (which must be different types) will be rendered twice.

As you can see, simply globally replacing the standard material with a different type isn't useful to me at all.

@mintlu8
Copy link
Contributor Author

mintlu8 commented May 29, 2024

I'm not sure this solves my issue. As described, the PR swaps out every material in a GLTF, and they must all be the same type. That doesn't describe my use case at all:

  • When loading a GLTF, some materials will be swapped out, while others will not be.
  • Not all swapped materials will be the same type.

Here's an example: Say I have a fireplace created in Blender. The fireplace has a special "flames" material that uses an animated texture. In Blender, I create a custom property "flames: true" an add it to the material. However, the rest of the fireplace - the bricks and so on - use normal materials.

When I load the model (in my previous non-Bevy engine), I examine the GLTF extras field and decide what type of material to substitute based on the custom properties. For most meshes I leave the material as-is - the standard material is good enough. However, for the flames material, I replace the standard material with the special animated shader material.

There are more complex scenarios. In my blender files, I also have a custom property called "outline", which creates a cartoon outline around the object. This requires two materials (because the outline is a separate pass). There are two ways to accomplish this:

  • Clone the mesh, and replace the clone's material with the outline shader.
  • Insert two materials into a single mesh - because of the way Bevy's pipeline works, a mesh with two different material components (which must be different types) will be rendered twice.

As you can see, simply globally replacing the standard material with a different type isn't useful to me at all.

I've made the PR use dynamic typing and untyped handles (still stuck on a few thing so not ready yet.)
I think what we can do on top of this is load the json as an internally tagged enum, register deserializers on the GltfLoader and do the typetag thing.

@MalekiRe
Copy link
Contributor

MalekiRe commented May 29, 2024

@MalekiRe you wanted this as well, correct?

Yes

It's useful for mobile when I wanna change the material so it has less performance impact

@mintlu8 mintlu8 marked this pull request as ready for review May 30, 2024 17:35
@mintlu8
Copy link
Contributor Author

mintlu8 commented May 30, 2024

As you can see, simply globally replacing the standard material with a different type isn't useful to me at all.

The new version should fit your use case better.

@mintlu8 mintlu8 changed the title Allow GltfLoader to swap out StandardMaterial. Allow GltfLoader load user defined materials. May 30, 2024
@mintlu8 mintlu8 changed the title Allow GltfLoader load user defined materials. Allow GltfLoader to load user defined materials. May 31, 2024
@hymm hymm removed the S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged label May 31, 2024
@JMS55 JMS55 self-requested a review May 31, 2024 16:53
@alice-i-cecile alice-i-cecile added the S-Needs-Review Needs reviewer attention (from anyone!) to move forward label Jun 5, 2024
@alice-i-cecile alice-i-cecile added the X-Contentious There are nontrivial implications that should be thought through label Aug 25, 2024
@JMS55 JMS55 removed their request for review September 11, 2024 05:38
@BenjaminBrienen BenjaminBrienen added S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Oct 31, 2024
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-Feature A new feature, making something new possible M-Needs-Release-Note Work that should be called out in the blog due to impact S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged X-Contentious There are nontrivial implications that should be thought through
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants