Changed GLTF Asset Label to use Strings instead of indexes

[Hexorg]

• Changed GLTF Asset Label to use Strings instead of indexes
• Removed Primitive name field as GLTF Spec does not allow Primitive names
• Changed all examples to use new asset label format.

Objective

It's hard to find out GLTF scene/meshes/materials/animation indexes from gltf, which can also change on every export from Blender. Let's introduce String labels instead.

Solution

Replace compatible GltfAssetLabel variants from usize to Cow<'static, str> instead.

Testing

I ran gltf unit tests, and each example that this pull request touches.

Showcase

You can now load, e.g. animations with assets.load("models.glb#Animation:Walk");

Migration Guide

GLTF Assets are now keyed by their names, not indexes. Assets that don't have a name, still use their index. For example

assets.load("fox.glb#Animation:Walk"); // load by name
assets.load("fox.glb#Scene:0"); // load by index

It is theoretically possible to have a gltf that has names only for some objects, but not all. In these cases indexes will NOT be continuous.

If an object has a name it is now also impossible to load it by index.

[Hexorg]

This technically addresses Issue#10715 though not directly.

[Hexorg]

@andriyDev I was trying to be a bit memory conscious since most asset labels are likely to be &'static str, but there's a chance someone will need it to be built dynamically.

I can just use String instead of Cow and let everyone call .to_owned() on their labels without helper functions.

[Hexorg]

Actually wait, those are unrelated. I have the helper methods just to avoid calling .into()

[alice-i-cecile]

I much prefer names, but indexes need to continue to be supported, especially for migration ease.

[hukasu]

@andriyDev I was trying to be a bit memory conscious since most asset labels are likely to be &'static str, but there's a chance someone will need it to be built dynamically.

I can just use String instead of Cow and let everyone call .to_owned() on their labels without helper functions.

the label wouldn't be 'static if it were defined on a file like .animgraph

[Alainx277]

I much prefer names, but indexes need to continue to be supported, especially for migration ease.

Unfortunately that is (afaik) still not possible due to asset aliases never being implemented and has been the blocker for named GLTF asset labels for multiple years (see https://github.com/bevyengine/bevy/issues/2948#issuecomment-939562038).

I strongly feel that Bevy should start using GLTF labels, as a manual index based workflow is extremely error prone (from my own experience). If backwards compatibility is a concern, https://github.com/bevyengine/bevy/pull/11279 is an old PR that gates labels behind a setting.

[aloucks]

Does the GLTF spec guarantee that names are unique?

EDIT: It does not.

https://registry.khronos.org/glTF/specs/2.0/glTF-2.0.html#indices-and-names

Whereas indices are used for internal glTF references, optional names are used for application-specific uses such as display. Any top-level glTF object MAY have a name string property for this purpose. These property values are not guaranteed to be unique as they are intended to contain values created when the asset was authored.

Loading by a named alias sounds great, but I don't think referencing everything internally with names is a good idea. The spec clearly states that the index is the way to go.

[mockersf]

indexes must still be the main way to do labels for gltfs, names can be added but can't replace those.

maybe the fix will come with the editor and a user friendly way to display the label but use the index in code

[viridia]

I only ever use names. Typically a GLTF character asset in my game is a file which contains multiple characters, all of which share the same rig and animations. Both individual characters and animation tracks are referenced by name. The reason for combining these together is twofold: ease of authoring in Blender, and reducing the number of i/o operations for loading a large population of actors.

For example, the NPC "Sergeant Stone" has an actor definition file that looks like this:

  "SergeantStone": {
    "type": "Actr",
    "display_name": "Sergeant Stone",
    "aspects": {
      "Skin": "characters/humanoid-male.glb#WithCoatHeavy",
      "Colors": {
        "Hair": "#918981",
        "FacialHair": "#91806b",
        "Coat": "#24378b",
        "CoatTrim": "#435575",
        "Shirt": "#775e31",
        "Belt": "#452e2e",
        "Gauntlets": "#5f4646",
        "Legs": "#4e4437",
        "Boots": "#2b3a48",
        "HatPrimary": "#1c274f",
        "HatDetail": "#686f83"
      },
      "Features": {
        "HairThinning": true,
        "FacialHairMuttonChops": true,
        "HatTricorn": true
      }
    },
    "extends": "#Proto.HumanMaleCoatHeavy"
  },

The GLTF model is referenced by the path "characters/humanoid-male.glb#WithCoatHeavy", in the file "humanoid-male" which contains about a dozen different character models (including skeletons) and about two dozen animation tracks. Because I may add more characters or animation tracks at a later time, I cannot depend on the numeric indices remaining stable; as a result I never use the indices if I can help it.

[aloucks]

Because I may add more characters or animation tracks at a later time, I cannot depend on the numeric indices remaining stable; as a result I never use the indices if I can help it.

Why do you need the index to remain stable? Why can't you query all animations from the asset and then build a mapping between the name and the index?

[andriyDev]

Why do you need the index to remain stable? Why can't you query all animations from the asset and then build a mapping between the name and the index?

The trouble with that is the deferred nature of assets: you can't use that mapping until after the GLTF has been loaded. This means you can't spawn anything until your asset has loaded, and you need a system to listen for asset events to fetch that handle, and only then do whatever you need to do. It's possible, but not exactly ideal from code.

[viridia]

The trouble with that is the deferred nature of assets: you can't use that mapping until after the GLTF has been loaded. This means you can't spawn anything until your asset has loaded, and you need a system to listen for asset events to fetch that handle, and only then do whatever you need to do. It's possible, but not exactly ideal from code.

Yes, exactly. What I do now is split the string on the # character, load the whole GLTF file, and then trawl through the root looking for the piece I need. This subverts the whole notion of asset labels.

[aloucks]

I'm not disputing the idea that loading it by an alias would be nice. I'm just saying changing the internal representation to use a non-unique identifier is just not the right solution, imo.

[mockersf]

looking at the graph in https://registry.khronos.org/glTF/specs/2.0/glTF-2.0.html#concepts-general

I think we should be ok using names for types that don't have a path coming to them.

To start moving forward, I think we should:

• limit this change to scenes and animations
• add a check that verify names are unique on those, and warn and fallback to indices if they are not

[aloucks]

I think we should be ok using names for types that don't have a path coming to them.

To start moving forward, I think we should:

• limit this change to scenes and animations
• add a check that verify names are unique on those, and warn and fallback to indices if they are not

That's going to lead to a lot of inconsistency. Can't this be addressed in the asset loader with a way to provide aliases?

[andriyDev]

Can't this be addressed in the asset loader with a way to provide aliases?

The problem is not providing a way for an AssetLoader to register aliases - that is mostly trivial. The problem is what to do with it. When you load a subasset through the AssetServer we don't know whether its an alias or not, so we always create a new strong handle to an asset. But once the subasset gets loaded, where does it go? Mesh0 or MyMeshName?

P.S. This may be worth documenting in a separate issue. I feel like "asset aliases" is a concrete enough feature and at least documenting why we can't do it today is useful.

[aloucks]

P.S. This may be worth documenting in a separate issue. I feel like "asset aliases" is a concrete enough feature and at least documenting why we can't do it today is useful.

This sounds like more of an issue with the current asset system in general, not a GTLF specific one.

[Alainx277]

I did spend an hour or two to check if adding multi-label support would be an easy win, unfortunately (as @andriyDev points out) the way handles work currently makes this quite difficult (at least for someone who does not have an in-depth understanding of the asset subsystem).

In addition to associating multiple labels to a subasset, 1-N handles need to be tracked per subasset. Perhaps this is not as hard as I imagine it, but I stopped at that point.

It would be great if someone who knows the asset system well could more precisely assess how difficult such a change would be. I can imagine it being useful for other kinds of assets as well (ex. sprite sheets, BSN).

[mockersf]

It would be great if someone who knows the asset system well could more precisely assess how difficult such a change would be. I can imagine it being useful for other kinds of assets as well (ex. sprite sheets, BSN).

it's been tried several times, and either you hit a perf issue when you alias, or a memory issue when you duplicate

[Alainx277]

If https://github.com/bevyengine/bevy/pull/15813 (@andriyDev again ❤️ ) is merged this problem would get solved by proxy, as two handles could share an Arc'd asset, paying only the setup cost of cloning the Arc once.

[andriyDev]

@Alainx277 Thanks for noticing #15813, haha! The reason I didn't being it up in this discussion is because using it for asset aliases results in two issues:

1. Cloning the arc means that you can't modify the asset in place anymore - modifying a subasset requires cloning the subasset first (and now your alias refers to different data)
2. The aliases are practically two separate assets. They have different handles, and these handles can independently be dropped. So if you drop all the handles to the alias, you need to reload the asset just to get it back (even if another alias is still being used).

(Also feel free to leave a review for #15813, I'd love to move it forward haha)

[Alainx277]

1. Cloning the arc means that you can't modify the asset in place anymore - modifying a subasset requires cloning the subasset first (and now your alias refers to different data)

I think that's an acceptable compromise, considering the lack of other feasible proposals.

2. The aliases are practically two separate assets. They have different handles, and these handles can independently be dropped. So if you drop all the handles to the alias, you need to reload the asset just to get it back (even if another alias is still being used).

Couldn't some additional bookkeeping be added so the asset system knows the asset is still available?

Edit: is it possible to move comments in GitHub? it is relevant to the feature, but this would be nice to have its own issue for.

[andriyDev]

@Alainx277 if you could make an issue, that would be great. Linking to this PR and then summarizing the ideas here would also be fantastic.

I also might have an idea on how to do asset aliases but I feel like I'm missing something so I'll tinker with it soon.

Edit: I actually just went ahead and did it myself, sorry for the ping!

[Igor-dvr]

Proposal mentioned in discord:

Getting an asset by its name is problematic because names are optional and can be duplicate. We can solve this by introducing duplicate number after an asset name when loading an asset.

Example: Let's say we have 2 animations with the name "Walk" and 3 without a name.

First animation with the name "Walk" can be loaded like this: assets.load("models.glb#Animation:Walk 0"); or shortcut: assets.load("models.glb#Animation:Walk"); Number after a name is not a real asset id. Its number of a duplicate.

Second animation with the name "Walk" can be loaded like this: assets.load("models.glb#Animation:Walk 1");

To load an asset that doesn't have a name we only specify duplicate number. This loads nameless asset 3: assets.load("models.glb#Animation: 2");

To simplify migration we can add an option in the loader settings that indicates if we want to use old id based approach or new approach with a name + duplicate number.