Merge pull request #445 from alteous/allow-empty-animation-target-node

Allow animation target node to be empty

.github/workflows/test.yml

@@ -21,10 +21,10 @@ jobs:
           path: glTF-Sample-Assets
 
       - name: Tests (minimal features)
-        run: cargo test --all --release --features allow_empty_texture
+        run: cargo test --all --release --features allow_empty_texture,allow_empty_animation_target_node -- --nocapture
 
       - name: Tests (all features)
-        run: cargo test --all --all-features --release
+        run: cargo test --all --all-features --release -- --nocapture
 
       - name: Formatting
         run: cargo fmt --all -- --check

CHANGELOG.md

@@ -8,6 +8,10 @@ The top-level `gltf` crate adheres to [Semantic Versioning](http://semver.org/sp
 
 ## Unreleased
 
+### Added
+
+- New feature flag `allow_empty_animation_target_node` to be able to parse newer animated assets.
+
 ## [1.4.1] - 2024-05-09
 
 ### Added

Cargo.toml

@@ -38,6 +38,7 @@ version = "0.25"
 
 [features]
 default = ["import", "utils", "names"]
+allow_empty_animation_target_node = ["gltf-json/allow_empty_animation_target_node"]
 allow_empty_texture = ["gltf-json/allow_empty_texture"]
 extensions = ["gltf-json/extensions"]
 extras = ["gltf-json/extras"]

gltf-json/Cargo.toml

@@ -16,6 +16,7 @@ serde_json = { features = ["raw_value"], version = "1.0" }
 
 [features]
 default = []
+allow_empty_animation_target_node = []
 allow_empty_texture = []
 names = []
 extensions = []

gltf-json/src/animation.rs

@@ -104,8 +104,32 @@ pub struct Channel {
     pub extras: Extras,
 }
 
-/// The index of the node and TRS property that an animation channel targets.
-#[derive(Clone, Debug, Deserialize, Serialize, Validate)]
+fn node_default() -> Index<scene::Node> {
+    Index::new(u32::MAX)
+}
+
+fn node_is_empty(node: &Index<scene::Node>) -> bool {
+    node.value() == u32::MAX as usize
+}
+
+fn node_validate<P, R>(node: &Index<scene::Node>, root: &Root, path: P, report: &mut R)
+where
+    P: Fn() -> Path,
+    R: FnMut(&dyn Fn() -> Path, Error),
+{
+    if cfg!(feature = "allow_empty_animation_target_node") {
+        if !node_is_empty(node) {
+            node.validate(root, path, report);
+        }
+    } else if node_is_empty(node) {
+        report(&path, Error::Missing);
+    } else {
+        node.validate(root, &path, report);
+    }
+}
+
+/// Selects the target of an animation channel.
+#[derive(Clone, Debug, Deserialize, Serialize)]
 pub struct Target {
     /// Extension specific data.
     #[serde(default, skip_serializing_if = "Option::is_none")]
@@ -118,13 +142,27 @@ pub struct Target {
     pub extras: Extras,
 
     /// The index of the node to target.
+    #[serde(default = "node_default", skip_serializing_if = "node_is_empty")]
     pub node: Index<scene::Node>,
 
     /// The name of the node's property to modify or the 'weights' of the
     /// morph targets it instantiates.
     pub path: Checked<Property>,
 }
 
+impl Validate for Target {
+    fn validate<P, R>(&self, root: &Root, path: P, report: &mut R)
+    where
+        P: Fn() -> Path,
+        R: FnMut(&dyn Fn() -> Path, Error),
+    {
+        self.extensions
+            .validate(root, || path().field("extensions"), report);
+        node_validate(&self.node, root, || path().field("node"), report);
+        self.path.validate(root, || path().field("path"), report);
+    }
+}
+
 /// Defines a keyframe graph but not its target.
 #[derive(Clone, Debug, Deserialize, Serialize, Validate)]
 pub struct Sampler {

gltf-json/src/texture.rs

@@ -1,4 +1,3 @@
-use crate::extensions::texture;
 use crate::validation::{Checked, Validate};
 use crate::{extensions, image, Extras, Index};
 use gltf_derive::Validate;

tests/import_sample_models.rs

@@ -13,9 +13,13 @@ fn check_import_result(
     use gltf::json::validation::Error;
     match result {
         Err(gltf::Error::Validation(errors)) => {
-            assert!(errors
+            let all_unsupported = errors
                 .iter()
-                .all(|(_path, error)| *error == Error::Unsupported));
+                .all(|(_path, error)| *error == Error::Unsupported);
+            if !all_unsupported {
+                println!("{errors:#?}");
+            }
+            assert!(all_unsupported);
             println!("skipped");
         }
         Err(otherwise) => {