Skip to content

Commit 86224b5

Browse files
ChristopherBiscardiChristopherBiscardi
authored
Refactor GltfExtensionHandler Hooks (#22114)
As it turns out, many glTF extensions require accessing the data from other extensions. Trying to isolate and scope access is much less helpful than exposing the glTF objects for consumers to take what they want. This includes - extension data in the "others" category (anything the gltf crate doesn't support explicitly) - the functions that return the extension data the gltf crate *does* have hardcoded support for - names and any other available data --- The diff for users is that they - no longer have to worry about defining extension ids to process - no longer have to worry about multiple calls due to those ids - now have to use `.extension_value()`, `.name()`, or similar to get relevant data - Can now access `.light()` or any other data built-in to the gltf crate, such as [`.variants`](https://docs.rs/gltf/1.4.1/gltf/struct.Document.html#method.variants) for `KHR_materials_variants`. An example diff on the user side can be viewed in the update commit for the Skein PR: rust-adventure/skein@ac1e510
1 parent 030657a commit 86224b5

File tree

3 files changed

+40
-162
lines changed

3 files changed

+40
-162
lines changed

‎crates/bevy_gltf/src/loader/extensions/mod.rs‎

Lines changed: 15 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -47,62 +47,34 @@ pub struct GltfExtensionHandlers(pub Arc<RwLock<Vec<Box<dyn GltfExtensionHandler
4747
/// The type a `GltfExtensionHandler` is implemented for can define data
4848
/// which will be cloned for each new glTF load. This enables stateful
4949
/// handling of glTF extension data during a single load.
50+
///
51+
/// When loading a glTF file, a glTF object that could contain extension
52+
/// data will cause the relevant hook to execute once per object.
53+
/// Each invocation will receive all extension data, which is required because
54+
/// many extensions require accessing data defined by other extensions.
55+
///
56+
/// The hooks are always called once, even if there is no extension data
57+
/// This is useful for scenarios where additional extension data isn't
58+
/// required, but processing should still happen.
5059
pub trait GltfExtensionHandler: Send + Sync {
5160
/// Required for dyn cloning
5261
fn dyn_clone(&self) -> Box<dyn GltfExtensionHandler>;
5362

54-
/// When loading a glTF file, a glTF object that could contain extension
55-
/// data will cause the relevant hook to execute once for each id in this list.
56-
/// Each invocation will receive the extension data for one of the extension ids,
57-
/// along with the `extension_id` itself so implementors can differentiate
58-
/// between different calls and parse data correctly.
59-
///
60-
/// The hooks are always called, even if there is no extension data
61-
/// for a specified id. This is useful for scenarios where additional
62-
/// extension data isn't required, but processing should still happen.
63-
///
64-
/// Most implementors will pick one extension for this list, causing the
65-
/// relevant hooks to fire once per object. An implementor that does not
66-
/// wish to receive any data but still wants hooks to be called can use
67-
/// an empty string `""` as the extension id, which is also the default
68-
/// value if the function is not implemented by an implementor. If the
69-
/// empty string is used, all extension data in hooks will be `None`.
70-
///
71-
/// Some implementors will choose to list multiple extensions here.
72-
/// This is an advanced use case and the alternative of having multiple
73-
/// independent handlers should be considered as an option first.
74-
/// If multiple extension ids are listed here, the hooks will fire once
75-
/// for each extension id, and each successive call will receive the data for
76-
/// a separate extension. The extension id is also included in hook arguments
77-
/// for this reason, so multiple extension id implementors can differentiate
78-
/// between the data received.
79-
fn extension_ids(&self) -> &'static [&'static str] {
80-
&[""]
81-
}
82-
8363
/// Called when the "global" data for an extension
8464
/// at the root of a glTF file is encountered.
8565
#[expect(
8666
unused,
8767
reason = "default trait implementations do not use the arguments because they are no-ops"
8868
)]
89-
fn on_root_data(&mut self, extension_id: &str, value: Option<&serde_json::Value>) {}
69+
fn on_root(&mut self, gltf: &gltf::Gltf) {}
9070

9171
#[cfg(feature = "bevy_animation")]
9272
#[expect(
9373
unused,
9474
reason = "default trait implementations do not use the arguments because they are no-ops"
9575
)]
9676
/// Called when an individual animation is processed
97-
fn on_animation(
98-
&mut self,
99-
extension_id: &str,
100-
extension_data: Option<&serde_json::Value>,
101-
gltf_animation: &gltf::Animation,
102-
name: Option<&str>,
103-
handle: Handle<AnimationClip>,
104-
) {
105-
}
77+
fn on_animation(&mut self, gltf_animation: &gltf::Animation, handle: Handle<AnimationClip>) {}
10678

10779
#[cfg(feature = "bevy_animation")]
10880
#[expect(
@@ -123,14 +95,15 @@ pub trait GltfExtensionHandler: Send + Sync {
12395
}
12496

12597
/// Called when an individual texture is processed
98+
/// Unlike other hooks, this hook does not receive its glTF
99+
/// object due to internal constraints.
126100
#[expect(
127101
unused,
128102
reason = "default trait implementations do not use the arguments because they are no-ops"
129103
)]
130104
fn on_texture(
131105
&mut self,
132-
extension_id: &str,
133-
extension_data: Option<&serde_json::Value>,
106+
extension_data: Option<&serde_json::Map<String, serde_json::Value>>,
134107
texture: Handle<bevy_image::Image>,
135108
) {
136109
}
@@ -142,11 +115,8 @@ pub trait GltfExtensionHandler: Send + Sync {
142115
)]
143116
fn on_material(
144117
&mut self,
145-
extension_id: &str,
146-
extension_data: Option<&serde_json::Value>,
147118
load_context: &mut LoadContext<'_>,
148119
gltf_material: &gltf::Material,
149-
name: Option<&str>,
150120
material: Handle<StandardMaterial>,
151121
) {
152122
}
@@ -158,11 +128,8 @@ pub trait GltfExtensionHandler: Send + Sync {
158128
)]
159129
fn on_gltf_mesh(
160130
&mut self,
161-
extension_id: &str,
162-
extension_data: Option<&serde_json::Value>,
163131
load_context: &mut LoadContext<'_>,
164132
gltf_mesh: &gltf::Mesh,
165-
name: Option<&str>,
166133
mesh: Handle<GltfMesh>,
167134
) {
168135
}
@@ -191,13 +158,10 @@ pub trait GltfExtensionHandler: Send + Sync {
191158
)]
192159
fn on_scene_completed(
193160
&mut self,
194-
extension_id: &str,
195-
extension_data: Option<&serde_json::Value>,
161+
load_context: &mut LoadContext<'_>,
196162
scene: &gltf::Scene,
197-
name: Option<&str>,
198163
world_root_id: Entity,
199164
scene_world: &mut World,
200-
load_context: &mut LoadContext<'_>,
201165
) {
202166
}
203167

@@ -208,8 +172,6 @@ pub trait GltfExtensionHandler: Send + Sync {
208172
)]
209173
fn on_gltf_node(
210174
&mut self,
211-
extension_id: &str,
212-
extension_data: Option<&serde_json::Value>,
213175
load_context: &mut LoadContext<'_>,
214176
gltf_node: &Node,
215177
entity: &mut EntityWorldMut,
@@ -225,8 +187,6 @@ pub trait GltfExtensionHandler: Send + Sync {
225187
)]
226188
fn on_spawn_light_directional(
227189
&mut self,
228-
extension_id: &str,
229-
extension_data: Option<&serde_json::Value>,
230190
load_context: &mut LoadContext<'_>,
231191
gltf_node: &Node,
232192
entity: &mut EntityWorldMut,
@@ -241,8 +201,6 @@ pub trait GltfExtensionHandler: Send + Sync {
241201
)]
242202
fn on_spawn_light_point(
243203
&mut self,
244-
extension_id: &str,
245-
extension_data: Option<&serde_json::Value>,
246204
load_context: &mut LoadContext<'_>,
247205
gltf_node: &Node,
248206
entity: &mut EntityWorldMut,
@@ -257,8 +215,6 @@ pub trait GltfExtensionHandler: Send + Sync {
257215
)]
258216
fn on_spawn_light_spot(
259217
&mut self,
260-
extension_id: &str,
261-
extension_data: Option<&serde_json::Value>,
262218
load_context: &mut LoadContext<'_>,
263219
gltf_node: &Node,
264220
entity: &mut EntityWorldMut,

‎crates/bevy_gltf/src/loader/mod.rs‎

Lines changed: 22 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -258,9 +258,7 @@ impl GltfLoader {
258258
// Let extensions process the root data for the extension ids
259259
// they've subscribed to.
260260
for extension in extensions.iter_mut() {
261-
for id in extension.extension_ids() {
262-
extension.on_root_data(id, gltf.extension_value(id));
263-
}
261+
extension.on_root(&gltf);
264262
}
265263

266264
let file_name = load_context
@@ -597,15 +595,7 @@ impl GltfLoader {
597595

598596
// let extensions handle extension data placed on animations
599597
for extension in extensions.iter_mut() {
600-
for id in extension.extension_ids() {
601-
extension.on_animation(
602-
id,
603-
animation.extension_value(id),
604-
&animation,
605-
animation.name(),
606-
handle.clone(),
607-
);
608-
}
598+
extension.on_animation(&animation, handle.clone());
609599
}
610600

611601
animations.push(handle);
@@ -654,13 +644,10 @@ impl GltfLoader {
654644
image.process_loaded_texture(load_context, &mut texture_handles);
655645
// let extensions handle texture data
656646
for extension in extensions.iter_mut() {
657-
for id in extension.extension_ids() {
658-
extension.on_texture(
659-
id,
660-
texture.extension_value(id),
661-
texture_handles.iter().last().unwrap().clone(),
662-
);
663-
}
647+
extension.on_texture(
648+
texture.extensions(),
649+
texture_handles.iter().last().unwrap().clone(),
650+
);
664651
}
665652
}
666653
} else {
@@ -695,13 +682,10 @@ impl GltfLoader {
695682
// We do this differently here because of the IoTaskPool vs
696683
// gltf::Texture lifetimes
697684
for extension in extensions.iter_mut() {
698-
for id in extension.extension_ids() {
699-
extension.on_texture(
700-
id,
701-
extension_data.as_ref().and_then(|map| map.get(*id)),
702-
texture_handles.iter().last().unwrap().clone(),
703-
);
704-
}
685+
extension.on_texture(
686+
extension_data.as_ref(),
687+
texture_handles.iter().last().unwrap().clone(),
688+
);
705689
}
706690
}
707691
Err(err) => {
@@ -723,16 +707,7 @@ impl GltfLoader {
723707

724708
// let extensions handle material data
725709
for extension in extensions.iter_mut() {
726-
for id in extension.extension_ids() {
727-
extension.on_material(
728-
id,
729-
material.extension_value(id),
730-
load_context,
731-
&material,
732-
material.name(),
733-
handle.clone(),
734-
);
735-
}
710+
extension.on_material(load_context, &material, handle.clone());
736711
}
737712

738713
materials.push(handle);
@@ -899,16 +874,7 @@ impl GltfLoader {
899874
named_meshes.insert(name.into(), handle.clone());
900875
}
901876
for extension in extensions.iter_mut() {
902-
for id in extension.extension_ids() {
903-
extension.on_gltf_mesh(
904-
id,
905-
gltf_mesh.extension_value(id),
906-
load_context,
907-
&gltf_mesh,
908-
gltf_mesh.name(),
909-
handle.clone(),
910-
);
911-
}
877+
extension.on_gltf_mesh(load_context, &gltf_mesh, handle.clone());
912878
}
913879

914880
meshes.push(handle);
@@ -1114,17 +1080,12 @@ impl GltfLoader {
11141080

11151081
// let extensions handle scene extension data
11161082
for extension in extensions.iter_mut() {
1117-
for id in extension.extension_ids() {
1118-
extension.on_scene_completed(
1119-
id,
1120-
scene.extension_value(id),
1121-
&scene,
1122-
scene.name(),
1123-
world_root_id,
1124-
&mut world,
1125-
&mut scene_load_context,
1126-
);
1127-
}
1083+
extension.on_scene_completed(
1084+
&mut scene_load_context,
1085+
&scene,
1086+
world_root_id,
1087+
&mut world,
1088+
);
11281089
}
11291090

11301091
let loaded_scene = scene_load_context.finish(Scene::new(world));
@@ -1755,15 +1716,7 @@ fn load_node(
17551716
});
17561717
}
17571718
for extension in extensions.iter_mut() {
1758-
for id in extension.extension_ids() {
1759-
extension.on_spawn_light_directional(
1760-
id,
1761-
gltf_node.extension_value(id),
1762-
load_context,
1763-
gltf_node,
1764-
&mut entity,
1765-
);
1766-
}
1719+
extension.on_spawn_light_directional(load_context, gltf_node, &mut entity);
17671720
}
17681721
}
17691722
gltf::khr_lights_punctual::Kind::Point => {
@@ -1786,15 +1739,7 @@ fn load_node(
17861739
});
17871740
}
17881741
for extension in extensions.iter_mut() {
1789-
for id in extension.extension_ids() {
1790-
extension.on_spawn_light_point(
1791-
id,
1792-
gltf_node.extension_value(id),
1793-
load_context,
1794-
gltf_node,
1795-
&mut entity,
1796-
);
1797-
}
1742+
extension.on_spawn_light_point(load_context, gltf_node, &mut entity);
17981743
}
17991744
}
18001745
gltf::khr_lights_punctual::Kind::Spot {
@@ -1822,15 +1767,7 @@ fn load_node(
18221767
});
18231768
}
18241769
for extension in extensions.iter_mut() {
1825-
for id in extension.extension_ids() {
1826-
extension.on_spawn_light_spot(
1827-
id,
1828-
gltf_node.extension_value(id),
1829-
load_context,
1830-
gltf_node,
1831-
&mut entity,
1832-
);
1833-
}
1770+
extension.on_spawn_light_spot(load_context, gltf_node, &mut entity);
18341771
}
18351772
}
18361773
}
@@ -1881,10 +1818,7 @@ fn load_node(
18811818
// accessing Mesh and Material extension data, which
18821819
// are merged onto the same entity in Bevy
18831820
for extension in extensions.iter_mut() {
1884-
for id in extension.extension_ids() {
1885-
let data = gltf_node.extension_value(id);
1886-
extension.on_gltf_node(id, data, load_context, gltf_node, &mut node);
1887-
}
1821+
extension.on_gltf_node(load_context, gltf_node, &mut node);
18881822
}
18891823

18901824
if let Some(err) = gltf_error {

‎examples/gltf/gltf_extension_animation_graph.rs‎

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -135,15 +135,8 @@ impl GltfExtensionHandler for GltfExtensionHandlerAnimation {
135135
}
136136

137137
#[cfg(feature = "bevy_animation")]
138-
fn on_animation(
139-
&mut self,
140-
_extension_id: &str,
141-
_value: Option<&serde_json::Value>,
142-
_gltf_animation: &gltf::Animation,
143-
name: Option<&str>,
144-
handle: Handle<AnimationClip>,
145-
) {
146-
if name.is_some_and(|v| v == "Walk") {
138+
fn on_animation(&mut self, gltf_animation: &gltf::Animation, handle: Handle<AnimationClip>) {
139+
if gltf_animation.name().is_some_and(|v| v == "Walk") {
147140
self.clip = Some(handle.clone());
148141
}
149142
}
@@ -160,8 +153,6 @@ impl GltfExtensionHandler for GltfExtensionHandlerAnimation {
160153

161154
fn on_gltf_node(
162155
&mut self,
163-
_extension_id: &str,
164-
_value: Option<&serde_json::Value>,
165156
_load_context: &mut LoadContext<'_>,
166157
gltf_node: &gltf::Node,
167158
entity: &mut EntityWorldMut,
@@ -174,13 +165,10 @@ impl GltfExtensionHandler for GltfExtensionHandlerAnimation {
174165
/// Called when an individual Scene is done processing
175166
fn on_scene_completed(
176167
&mut self,
177-
_extension_id: &str,
178-
_value: Option<&serde_json::Value>,
168+
load_context: &mut LoadContext<'_>,
179169
_scene: &gltf::Scene,
180-
_name: Option<&str>,
181170
_world_root_id: Entity,
182171
world: &mut World,
183-
load_context: &mut LoadContext<'_>,
184172
) {
185173
// Create an AnimationGraph from the desired clip
186174
let (graph, index) = AnimationGraph::from_clip(self.clip.clone().unwrap());

0 commit comments

Comments
 (0)