Skip to content

Commit cb4fe4e

Browse files
authored
Make gLTF node children Handle instead of objects (#13707)
Part of #13681 # Objective gLTF Assets shouldn't be duplicated between Assets resource and node children. Also changed `asset_label` to be a method as [per previous PR comment](#13558). ## Solution - Made GltfNode children be Handles instead of asset copies. ## Testing - Added tests that actually test loading and hierarchy as previous ones unit tested only one function and that makes little sense. - Made circular nodes an actual loading failure instead of a warning no-op. You [_MUST NOT_ have cycles in gLTF](https://registry.khronos.org/glTF/specs/2.0/glTF-2.0.html#nodes-and-hierarchy) according to the spec. - IMO this is a bugfix, not a breaking change. But in an extremely unlikely event in which you relied on invalid behavior for loading gLTF with cyclic children, you will not be able to do that anymore. You should fix your gLTF file as it's not valid according to gLTF spec. For it to for work someone, it had to be bevy with bevy_animation flag off. --- ## Changelog ### Changed - `GltfNode.children` are now `Vec<Handle<GltfNode>>` instead of `Vec<GltfNode>` - Having children cycles between gLTF nodes in a gLTF document is now an explicit asset loading failure. ## Migration Guide If accessing children, use `Assets<GltfNode>` resource to get the actual child object. #### Before ```rs fn gltf_print_first_node_children_system(gltf_component_query: Query<Handle<Gltf>>, gltf_assets: Res<Assets<Gltf>>, gltf_nodes: Res<Assets<GltfNode>>) { for gltf_handle in gltf_component_query.iter() { let gltf_root = gltf_assets.get(gltf_handle).unwrap(); let first_node_handle = gltf_root.nodes.get(0).unwrap(); let first_node = gltf_nodes.get(first_node_handle).unwrap(); let first_child = first_node.children.get(0).unwrap(); println!("First nodes child node name is {:?)", first_child.name); } } ``` #### After ```rs fn gltf_print_first_node_children_system(gltf_component_query: Query<Handle<Gltf>>, gltf_assets: Res<Assets<Gltf>>, gltf_nodes: Res<Assets<GltfNode>>) { for gltf_handle in gltf_component_query.iter() { let gltf_root = gltf_assets.get(gltf_handle).unwrap(); let first_node_handle = gltf_root.nodes.get(0).unwrap(); let first_node = gltf_nodes.get(first_node_handle).unwrap(); let first_child_handle = first_node.children.get(0).unwrap(); let first_child = gltf_nodes.get(first_child_handle).unwrap(); println!("First nodes child node name is {:?)", first_child.name); } } ```
1 parent 64bc811 commit cb4fe4e

File tree

3 files changed

+356
-109
lines changed

3 files changed

+356
-109
lines changed

crates/bevy_gltf/Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,9 @@ serde = { version = "1.0", features = ["derive"] }
5757
serde_json = "1"
5858
smallvec = "1.11"
5959

60+
[dev-dependencies]
61+
bevy_log = { path = "../bevy_log", version = "0.14.0-dev" }
62+
6063
[lints]
6164
workspace = true
6265

crates/bevy_gltf/src/lib.rs

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -209,10 +209,8 @@ pub struct GltfNode {
209209
pub index: usize,
210210
/// Computed name for a node - either a user defined node name from gLTF or a generated name from index
211211
pub name: String,
212-
/// Subasset label for this node within the gLTF parent asset.
213-
pub asset_label: GltfAssetLabel,
214212
/// Direct children of the node.
215-
pub children: Vec<GltfNode>,
213+
pub children: Vec<Handle<GltfNode>>,
216214
/// Mesh of the node.
217215
pub mesh: Option<Handle<GltfMesh>>,
218216
/// Local transform.
@@ -225,14 +223,13 @@ impl GltfNode {
225223
/// Create a node extracting name and index from glTF def
226224
pub fn new(
227225
node: &gltf::Node,
228-
children: Vec<GltfNode>,
226+
children: Vec<Handle<GltfNode>>,
229227
mesh: Option<Handle<GltfMesh>>,
230228
transform: bevy_transform::prelude::Transform,
231229
extras: Option<GltfExtras>,
232230
) -> Self {
233231
Self {
234232
index: node.index(),
235-
asset_label: GltfAssetLabel::Node(node.index()),
236233
name: if let Some(name) = node.name() {
237234
name.to_string()
238235
} else {
@@ -244,6 +241,11 @@ impl GltfNode {
244241
extras,
245242
}
246243
}
244+
245+
/// Subasset label for this node within the gLTF parent asset.
246+
pub fn asset_label(&self) -> GltfAssetLabel {
247+
GltfAssetLabel::Node(self.index)
248+
}
247249
}
248250

249251
/// A glTF mesh, which may consist of multiple [`GltfPrimitives`](GltfPrimitive)
@@ -256,8 +258,6 @@ pub struct GltfMesh {
256258
pub index: usize,
257259
/// Computed name for a mesh - either a user defined mesh name from gLTF or a generated name from index
258260
pub name: String,
259-
/// Subasset label for this mesh within the gLTF parent asset.
260-
pub asset_label: GltfAssetLabel,
261261
/// Primitives of the glTF mesh.
262262
pub primitives: Vec<GltfPrimitive>,
263263
/// Additional data.
@@ -273,7 +273,6 @@ impl GltfMesh {
273273
) -> Self {
274274
Self {
275275
index: mesh.index(),
276-
asset_label: GltfAssetLabel::Mesh(mesh.index()),
277276
name: if let Some(name) = mesh.name() {
278277
name.to_string()
279278
} else {
@@ -283,6 +282,11 @@ impl GltfMesh {
283282
extras,
284283
}
285284
}
285+
286+
/// Subasset label for this mesh within the gLTF parent asset.
287+
pub fn asset_label(&self) -> GltfAssetLabel {
288+
GltfAssetLabel::Mesh(self.index)
289+
}
286290
}
287291

288292
/// Part of a [`GltfMesh`] that consists of a [`Mesh`], an optional [`StandardMaterial`] and [`GltfExtras`].
@@ -292,10 +296,10 @@ impl GltfMesh {
292296
pub struct GltfPrimitive {
293297
/// Index of the primitive inside the mesh
294298
pub index: usize,
299+
/// Index of the parent [`GltfMesh`] of this primitive
300+
pub parent_mesh_index: usize,
295301
/// Computed name for a primitive - either a user defined primitive name from gLTF or a generated name from index
296302
pub name: String,
297-
/// Subasset label for this mesh within the gLTF parent asset.
298-
pub asset_label: GltfAssetLabel,
299303
/// Topology to be rendered.
300304
pub mesh: Handle<Mesh>,
301305
/// Material to apply to the `mesh`.
@@ -318,6 +322,7 @@ impl GltfPrimitive {
318322
) -> Self {
319323
GltfPrimitive {
320324
index: gltf_primitive.index(),
325+
parent_mesh_index: gltf_mesh.index(),
321326
name: {
322327
let mesh_name = gltf_mesh.name().unwrap_or("Mesh");
323328
if gltf_mesh.primitives().len() > 1 {
@@ -326,16 +331,20 @@ impl GltfPrimitive {
326331
mesh_name.to_string()
327332
}
328333
},
329-
asset_label: GltfAssetLabel::Primitive {
330-
mesh: gltf_mesh.index(),
331-
primitive: gltf_primitive.index(),
332-
},
333334
mesh,
334335
material,
335336
extras,
336337
material_extras,
337338
}
338339
}
340+
341+
/// Subasset label for this primitive within its parent [`GltfMesh`] within the gLTF parent asset.
342+
pub fn asset_label(&self) -> GltfAssetLabel {
343+
GltfAssetLabel::Primitive {
344+
mesh: self.parent_mesh_index,
345+
primitive: self.index,
346+
}
347+
}
339348
}
340349

341350
/// Additional untyped data that can be present on most glTF types at the primitive level.
@@ -405,7 +414,7 @@ pub struct GltfMaterialExtras {
405414
/// let gltf_scene: Handle<Scene> = asset_server.load(format!("models/FlightHelmet/FlightHelmet.gltf#{}", GltfAssetLabel::Scene(0)));
406415
/// }
407416
/// ```
408-
#[derive(Debug, Clone, Copy)]
417+
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
409418
pub enum GltfAssetLabel {
410419
/// `Scene{}`: glTF Scene as a Bevy `Scene`
411420
Scene(usize),

0 commit comments

Comments
 (0)