From 33ce6a1c2c591528699ceeb80f98b939e9186634 Mon Sep 17 00:00:00 2001 From: David Harvey-Macaulay Date: Mon, 22 Apr 2024 15:58:27 +0100 Subject: [PATCH] Use new validate_hook everywhere except animation --- gltf-json/src/accessor.rs | 44 +++++----------- gltf-json/src/camera.rs | 37 +++++--------- gltf-json/src/extensions/animation.rs | 4 +- gltf-json/src/extensions/scene.rs | 29 ++++------- gltf-json/src/mesh.rs | 73 +++++++++++---------------- gltf-json/src/root.rs | 40 +++++++-------- gltf-json/src/texture.rs | 30 +++++------ 7 files changed, 99 insertions(+), 158 deletions(-) diff --git a/gltf-json/src/accessor.rs b/gltf-json/src/accessor.rs index d1663e49..b8c14cb1 100644 --- a/gltf-json/src/accessor.rs +++ b/gltf-json/src/accessor.rs @@ -1,4 +1,4 @@ -use crate::validation::{Checked, Error, USize64, Validate}; +use crate::validation::{Checked, Error, USize64}; use crate::{buffer, extensions, Extras, Index, Path, Root}; use gltf_derive::Validate; use serde::{de, ser}; @@ -169,7 +169,8 @@ pub mod sparse { } /// A typed view into a buffer view. -#[derive(Clone, Debug, Deserialize, Serialize)] +#[derive(Clone, Debug, Deserialize, Serialize, Validate)] +#[gltf(validate_hook = "accessor_validate_hook")] pub struct Accessor { /// The parent buffer view this accessor reads from. /// @@ -231,36 +232,15 @@ pub struct Accessor { pub sparse: Option, } -impl Validate for Accessor { - fn validate(&self, root: &Root, path: P, report: &mut R) - where - P: Fn() -> Path, - R: FnMut(&dyn Fn() -> Path, Error), - { - if self.sparse.is_none() && self.buffer_view.is_none() { - // If sparse is missing, then bufferView must be present. Report that bufferView is - // missing since it is the more common one to require. - report(&|| path().field("bufferView"), Error::Missing); - } - - self.buffer_view - .validate(root, || path().field("bufferView"), report); - self.byte_offset - .validate(root, || path().field("byteOffset"), report); - self.count.validate(root, || path().field("count"), report); - self.component_type - .validate(root, || path().field("componentType"), report); - self.extensions - .validate(root, || path().field("extensions"), report); - self.extras - .validate(root, || path().field("extras"), report); - self.type_.validate(root, || path().field("type"), report); - self.min.validate(root, || path().field("min"), report); - self.max.validate(root, || path().field("max"), report); - self.normalized - .validate(root, || path().field("normalized"), report); - self.sparse - .validate(root, || path().field("sparse"), report); +fn accessor_validate_hook(accessor: &Accessor, _root: &Root, path: P, report: &mut R) +where + P: Fn() -> Path, + R: FnMut(&dyn Fn() -> Path, Error), +{ + if accessor.sparse.is_none() && accessor.buffer_view.is_none() { + // If sparse is missing, then bufferView must be present. Report that bufferView is + // missing since it is the more common one to require. + report(&|| path().field("bufferView"), Error::Missing); } } diff --git a/gltf-json/src/camera.rs b/gltf-json/src/camera.rs index 3b416c30..e7c23d3b 100644 --- a/gltf-json/src/camera.rs +++ b/gltf-json/src/camera.rs @@ -1,4 +1,4 @@ -use crate::validation::{Checked, Error, Validate}; +use crate::validation::{Checked, Error}; use crate::{extensions, Extras, Path, Root}; use gltf_derive::Validate; use serde::{de, ser}; @@ -22,7 +22,8 @@ pub enum Type { /// /// A node can reference a camera to apply a transform to place the camera in the /// scene. -#[derive(Clone, Debug, Deserialize, Serialize)] +#[derive(Clone, Debug, Deserialize, Serialize, Validate)] +#[gltf(validate_hook = "camera_validate_hook")] pub struct Camera { /// Optional user-defined name for this object. #[cfg(feature = "names")] @@ -54,6 +55,16 @@ pub struct Camera { pub extras: Extras, } +fn camera_validate_hook(camera: &Camera, _root: &Root, path: P, report: &mut R) +where + P: Fn() -> Path, + R: FnMut(&dyn Fn() -> Path, Error), +{ + if camera.orthographic.is_none() && camera.perspective.is_none() { + report(&path, Error::Missing); + } +} + /// Values for an orthographic camera. #[derive(Clone, Debug, Deserialize, Serialize, Validate)] pub struct Orthographic { @@ -109,28 +120,6 @@ pub struct Perspective { pub extras: Extras, } -impl Validate for Camera { - fn validate(&self, root: &Root, path: P, report: &mut R) - where - P: Fn() -> Path, - R: FnMut(&dyn Fn() -> Path, Error), - { - if self.orthographic.is_none() && self.perspective.is_none() { - report(&path, Error::Missing); - } - - self.orthographic - .validate(root, || path().field("orthographic"), report); - self.perspective - .validate(root, || path().field("perspective"), report); - self.type_.validate(root, || path().field("type"), report); - self.extensions - .validate(root, || path().field("extensions"), report); - self.extras - .validate(root, || path().field("extras"), report); - } -} - impl<'de> de::Deserialize<'de> for Checked { fn deserialize(deserializer: D) -> Result where diff --git a/gltf-json/src/extensions/animation.rs b/gltf-json/src/extensions/animation.rs index ebe671e3..f12da808 100644 --- a/gltf-json/src/extensions/animation.rs +++ b/gltf-json/src/extensions/animation.rs @@ -4,7 +4,7 @@ use serde_derive::{Deserialize, Serialize}; use serde_json::{Map, Value}; /// A keyframe animation. -#[derive(Clone, Debug, Default, Deserialize, Serialize)] +#[derive(Clone, Debug, Default, Deserialize, Serialize, Validate)] pub struct Animation { #[cfg(feature = "extensions")] #[serde(default, flatten)] @@ -12,7 +12,7 @@ pub struct Animation { } /// Targets an animation's sampler at a node's property. -#[derive(Clone, Debug, Default, Deserialize, Serialize)] +#[derive(Clone, Debug, Default, Deserialize, Serialize, Validate)] pub struct Channel {} /// The index of the node and TRS property that an animation channel targets. diff --git a/gltf-json/src/extensions/scene.rs b/gltf-json/src/extensions/scene.rs index 77139836..68a212d3 100644 --- a/gltf-json/src/extensions/scene.rs +++ b/gltf-json/src/extensions/scene.rs @@ -30,7 +30,7 @@ pub struct Node { #[cfg(feature = "KHR_lights_punctual")] pub mod khr_lights_punctual { - use crate::validation::{Checked, Error, Validate}; + use crate::validation::{Checked, Error}; use crate::{Extras, Index, Path, Root}; use gltf_derive::Validate; use serde::{de, ser}; @@ -76,7 +76,8 @@ pub mod khr_lights_punctual { Spot, } - #[derive(Clone, Debug, Deserialize, Serialize)] + #[derive(Clone, Debug, Deserialize, Serialize, Validate)] + #[gltf(validate_hook = "light_validate_hook")] pub struct Light { /// Color of the light source. #[serde(default = "color_default")] @@ -116,23 +117,15 @@ pub mod khr_lights_punctual { pub type_: Checked, } - impl Validate for Light { - fn validate(&self, root: &Root, path: P, report: &mut R) - where - P: Fn() -> Path, - R: FnMut(&dyn Fn() -> Path, Error), - { - if let Checked::Valid(ty) = self.type_.as_ref() { - if *ty == Type::Spot && self.spot.is_none() { - report(&|| path().field("spot"), Error::Missing); - } + fn light_validate_hook(light: &Light, _root: &Root, path: P, report: &mut R) + where + P: Fn() -> Path, + R: FnMut(&dyn Fn() -> Path, Error), + { + if let Checked::Valid(ty) = light.type_.as_ref() { + if *ty == Type::Spot && light.spot.is_none() { + report(&|| path().field("spot"), Error::Missing); } - - self.type_.validate(root, || path().field("type"), report); - self.extensions - .validate(root, || path().field("extensions"), report); - self.extras - .validate(root, || path().field("extras"), report); } } diff --git a/gltf-json/src/mesh.rs b/gltf-json/src/mesh.rs index 85b2e6c3..14aeb44f 100644 --- a/gltf-json/src/mesh.rs +++ b/gltf-json/src/mesh.rs @@ -1,4 +1,4 @@ -use crate::validation::{Checked, Error, Validate}; +use crate::validation::{Checked, Error}; use crate::{accessor, extensions, material, Extras, Index}; use gltf_derive::Validate; use serde::{de, ser}; @@ -97,7 +97,8 @@ pub struct Mesh { } /// Geometry to be rendered with the given material. -#[derive(Clone, Debug, Deserialize, Serialize)] +#[derive(Clone, Debug, Deserialize, Serialize, Validate)] +#[gltf(validate_hook = "primitive_validate_hook")] pub struct Primitive { /// Maps attribute semantic names to the `Accessor`s containing the /// corresponding attribute data. @@ -136,54 +137,38 @@ fn is_primitive_mode_default(mode: &Checked) -> bool { *mode == Checked::Valid(Mode::Triangles) } -impl Validate for Primitive { - fn validate(&self, root: &crate::Root, path: P, report: &mut R) - where - P: Fn() -> crate::Path, - R: FnMut(&dyn Fn() -> crate::Path, crate::validation::Error), +fn primitive_validate_hook(primitive: &Primitive, root: &crate::Root, path: P, report: &mut R) +where + P: Fn() -> crate::Path, + R: FnMut(&dyn Fn() -> crate::Path, crate::validation::Error), +{ + let position_path = &|| path().field("attributes").key("POSITION"); + if let Some(pos_accessor_index) = primitive + .attributes + .get(&Checked::Valid(Semantic::Positions)) { - // Generated part - self.attributes - .validate(root, || path().field("attributes"), report); - self.extensions - .validate(root, || path().field("extensions"), report); - self.extras - .validate(root, || path().field("extras"), report); - self.indices - .validate(root, || path().field("indices"), report); - self.material - .validate(root, || path().field("material"), report); - self.mode.validate(root, || path().field("mode"), report); - self.targets - .validate(root, || path().field("targets"), report); - - // Custom part - let position_path = &|| path().field("attributes").key("POSITION"); - if let Some(pos_accessor_index) = self.attributes.get(&Checked::Valid(Semantic::Positions)) - { - // spec: POSITION accessor **must** have `min` and `max` properties defined. - let pos_accessor = &root.accessors[pos_accessor_index.value()]; - - let min_path = &|| position_path().field("min"); - if let Some(ref min) = pos_accessor.min { - if from_value::<[f32; 3]>(min.clone()).is_err() { - report(min_path, Error::Invalid); - } - } else { - report(min_path, Error::Missing); + // spec: POSITION accessor **must** have `min` and `max` properties defined. + let pos_accessor = &root.accessors[pos_accessor_index.value()]; + + let min_path = &|| position_path().field("min"); + if let Some(ref min) = pos_accessor.min { + if from_value::<[f32; 3]>(min.clone()).is_err() { + report(min_path, Error::Invalid); } + } else { + report(min_path, Error::Missing); + } - let max_path = &|| position_path().field("max"); - if let Some(ref max) = pos_accessor.max { - if from_value::<[f32; 3]>(max.clone()).is_err() { - report(max_path, Error::Invalid); - } - } else { - report(max_path, Error::Missing); + let max_path = &|| position_path().field("max"); + if let Some(ref max) = pos_accessor.max { + if from_value::<[f32; 3]>(max.clone()).is_err() { + report(max_path, Error::Invalid); } } else { - report(position_path, Error::Missing); + report(max_path, Error::Missing); } + } else { + report(position_path, Error::Missing); } } diff --git a/gltf-json/src/root.rs b/gltf-json/src/root.rs index 472da3d9..493271de 100644 --- a/gltf-json/src/root.rs +++ b/gltf-json/src/root.rs @@ -55,26 +55,6 @@ impl Index { } } -fn root_validate_hook(root: &Root, _also_root: &Root, path: P, report: &mut R) -where - P: Fn() -> Path, - R: FnMut(&dyn Fn() -> Path, crate::validation::Error), -{ - for (i, ext) in root.extensions_required.iter().enumerate() { - if !crate::extensions::ENABLED_EXTENSIONS.contains(&ext.as_str()) { - report( - &|| { - path() - .field("extensionsRequired") - .index(i) - .value_str(ext.as_str()) - }, - crate::validation::Error::Unsupported, - ); - } - } -} - /// The root object of a glTF 2.0 asset. #[derive(Clone, Debug, Default, Deserialize, Serialize, Validate)] #[gltf(validate_hook = "root_validate_hook")] @@ -172,6 +152,26 @@ pub struct Root { pub textures: Vec, } +fn root_validate_hook(root: &Root, _also_root: &Root, path: P, report: &mut R) +where + P: Fn() -> Path, + R: FnMut(&dyn Fn() -> Path, crate::validation::Error), +{ + for (i, ext) in root.extensions_required.iter().enumerate() { + if !crate::extensions::ENABLED_EXTENSIONS.contains(&ext.as_str()) { + report( + &|| { + path() + .field("extensionsRequired") + .index(i) + .value_str(ext.as_str()) + }, + crate::validation::Error::Unsupported, + ); + } + } +} + impl Root { /// Returns a single item from the root object. pub fn get(&self, index: Index) -> Option<&T> diff --git a/gltf-json/src/texture.rs b/gltf-json/src/texture.rs index d00cb19d..4ad3236a 100644 --- a/gltf-json/src/texture.rs +++ b/gltf-json/src/texture.rs @@ -167,7 +167,8 @@ pub struct Sampler { } /// A texture and its sampler. -#[derive(Clone, Debug, Deserialize, Serialize)] +#[derive(Clone, Debug, Deserialize, Serialize, Validate)] +#[gltf(validate_hook = "texture_validate_hook")] pub struct Texture { /// Optional user-defined name for this object. #[cfg(feature = "names")] @@ -193,23 +194,16 @@ pub struct Texture { pub extras: Extras, } -impl Validate for Texture { - fn validate(&self, root: &Root, path: P, report: &mut R) - where - P: Fn() -> Path, - R: FnMut(&dyn Fn() -> Path, Error), - { - self.sampler - .validate(root, || path().field("sampler"), report); - - { - let source_path = || path().field("source"); - if let Some(index) = self.source.as_ref() { - index.validate(root, source_path, report); - } else { - report(&source_path, Error::Missing); - } - } +fn texture_validate_hook(texture: &Texture, root: &Root, path: P, report: &mut R) +where + P: Fn() -> Path, + R: FnMut(&dyn Fn() -> Path, Error), +{ + let source_path = || path().field("source"); + if let Some(index) = texture.source.as_ref() { + index.validate(root, source_path, report); + } else { + report(&source_path, Error::Missing); } }