From d69b141dd0908deff58a9531d2a58cbf91736e38 Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Tue, 30 Jan 2024 21:33:29 +0900 Subject: [PATCH] Move error helpers to their own modules --- src/collada/effect.rs | 20 +++++++-- src/collada/error.rs | 40 ++++++++++++++++++ src/collada/geometry.rs | 16 +++++++- src/collada/mod.rs | 58 +-------------------------- src/collada/scene.rs | 18 ++++++++- src/error.rs | 4 +- src/obj/error.rs | 53 ++++++++++++++++++++++++ src/obj/mod.rs | 62 ++-------------------------- src/stl/error.rs | 84 ++++++++++++++++++++++++++++++++++++++ src/stl/mod.rs | 89 +---------------------------------------- src/utils/mod.rs | 18 +++++---- 11 files changed, 243 insertions(+), 219 deletions(-) create mode 100644 src/collada/error.rs create mode 100644 src/obj/error.rs create mode 100644 src/stl/error.rs diff --git a/src/collada/effect.rs b/src/collada/effect.rs index 67150bd..8a308f7 100644 --- a/src/collada/effect.rs +++ b/src/collada/effect.rs @@ -498,7 +498,14 @@ fn parse_effect_color<'a>( // texcoord, }; } - "param" => warn::unsupported_child_elem(child), + "param" => { + // warn!( + // "<{}> child element in <{}> element is unsupported ({})", + // child.tag_name().name(), + // child.parent_element().unwrap().tag_name().name(), + // child.node_location() + // ); + } _ => {} } } @@ -517,7 +524,14 @@ fn parse_effect_float(node: xml::Node<'_, '_>) -> io::Result> { .ok_or_else(|| format_err!("error while parsing a float"))?, ); } - "param" => warn::unsupported_child_elem(child), + "param" => { + // warn!( + // "<{}> child element in <{}> element is unsupported ({})", + // child.tag_name().name(), + // child.parent_element().unwrap().tag_name().name(), + // child.node_location() + // ); + } _ => return Err(error::unexpected_child_elem(child)), } } @@ -552,7 +566,7 @@ impl FromStr for Opaque { "A_ONE" => Self::A_ONE, "RGB_ZERO" => Self::RGB_ZERO, "RGB_ONE" => Self::RGB_ONE, - _ => bail!("unknown shade type {:?}", s), + _ => bail!("unknown opaque type {:?}", s), }) } } diff --git a/src/collada/error.rs b/src/collada/error.rs new file mode 100644 index 0000000..4799b18 --- /dev/null +++ b/src/collada/error.rs @@ -0,0 +1,40 @@ +use super::*; + +#[cold] +pub(super) fn one_or_more_elems(node: xml::Node<'_, '_>, name: &str) -> io::Error { + format_err!( + "<{}> element must be contain one or more <{}> elements ({})", + node.tag_name().name(), + name, + node.node_location() + ) +} + +#[cold] +pub(super) fn exactly_one_elem(node: xml::Node<'_, '_>, name: &str) -> io::Error { + format_err!( + "<{}> element must be contain exactly one <{}> element ({})", + node.tag_name().name(), + name, + node.node_location() + ) +} + +#[cold] +pub(super) fn multiple_elems(node: xml::Node<'_, '_>) -> io::Error { + format_err!( + "multiple <{}> elements ({})", + node.tag_name().name(), + node.node_location() + ) +} + +#[cold] +pub(super) fn unexpected_child_elem(child: xml::Node<'_, '_>) -> io::Error { + format_err!( + "unexpected child element <{}> in <{}> element ({})", + child.tag_name().name(), + child.parent_element().unwrap().tag_name().name(), + child.node_location() + ) +} diff --git a/src/collada/geometry.rs b/src/collada/geometry.rs index 445321c..f38374c 100644 --- a/src/collada/geometry.rs +++ b/src/collada/geometry.rs @@ -183,7 +183,12 @@ fn parse_geometry<'a>( mesh = Some(parse_mesh(cx, node)?); } "convex_mesh" | "spline" | "brep" => { - warn::unsupported_child_elem(node); + // warn!( + // "<{}> child element in <{}> element is unsupported ({})", + // child.tag_name().name(), + // child.parent_element().unwrap().tag_name().name(), + // child.node_location() + // ); return Ok(None); } "asset" | "extra" => { /* skip */ } @@ -479,7 +484,14 @@ fn parse_primitive<'a>(node: xml::Node<'a, '_>, ty: PrimitiveType) -> io::Result } } } - "ph" => warn::unsupported_child_elem(node), + "ph" => { + // warn!( + // "<{}> child element in <{}> element is unsupported ({})", + // child.tag_name().name(), + // child.parent_element().unwrap().tag_name().name(), + // child.node_location() + // ); + } "extra" => { /* skip */ } _ => return Err(error::unexpected_child_elem(node)), } diff --git a/src/collada/mod.rs b/src/collada/mod.rs index b448c51..8678969 100644 --- a/src/collada/mod.rs +++ b/src/collada/mod.rs @@ -5,6 +5,7 @@ #![allow(clippy::wildcard_imports)] // TODO mod effect; +mod error; mod geometry; mod image; mod instance; @@ -885,60 +886,3 @@ impl FromStr for InputSemantic { }) } } - -mod error { - use super::*; - - #[cold] - pub(super) fn one_or_more_elems(node: xml::Node<'_, '_>, name: &str) -> io::Error { - format_err!( - "<{}> element must be contain one or more <{}> elements ({})", - node.tag_name().name(), - name, - node.node_location() - ) - } - - #[cold] - pub(super) fn exactly_one_elem(node: xml::Node<'_, '_>, name: &str) -> io::Error { - format_err!( - "<{}> element must be contain exactly one <{}> element ({})", - node.tag_name().name(), - name, - node.node_location() - ) - } - - #[cold] - pub(super) fn multiple_elems(node: xml::Node<'_, '_>) -> io::Error { - format_err!( - "multiple <{}> elements ({})", - node.tag_name().name(), - node.node_location() - ) - } - - #[cold] - pub(super) fn unexpected_child_elem(child: xml::Node<'_, '_>) -> io::Error { - format_err!( - "unexpected child element <{}> in <{}> element ({})", - child.tag_name().name(), - child.parent_element().unwrap().tag_name().name(), - child.node_location() - ) - } -} - -mod warn { - use super::*; - - #[cold] - pub(super) fn unsupported_child_elem(_child: xml::Node<'_, '_>) { - // warn!( - // "<{}> child element in <{}> element is unsupported ({})", - // child.tag_name().name(), - // child.parent_element().unwrap().tag_name().name(), - // child.node_location() - // ); - } -} diff --git a/src/collada/scene.rs b/src/collada/scene.rs index 4f9f63e..07ba6e7 100644 --- a/src/collada/scene.rs +++ b/src/collada/scene.rs @@ -195,7 +195,14 @@ fn parse_visual_scene<'a>(node: xml::Node<'a, '_>, nodes: &mut Vec>) -> "node" => { scene_nodes.push(parse_node(child, nodes, this_index)?); } - "evaluate_scene" => warn::unsupported_child_elem(child), + "evaluate_scene" => { + // warn!( + // "<{}> child element in <{}> element is unsupported ({})", + // child.tag_name().name(), + // child.parent_element().unwrap().tag_name().name(), + // child.node_location() + // ); + } "asset" | "extra" => { /* skip */ } _ => return Err(error::unexpected_child_elem(child)), } @@ -404,7 +411,14 @@ fn parse_instance_material<'a>(node: xml::Node<'a, '_>) -> io::Result warn::unsupported_child_elem(child), + "bind" => { + // warn!( + // "<{}> child element in <{}> element is unsupported ({})", + // child.tag_name().name(), + // child.parent_element().unwrap().tag_name().name(), + // child.node_location() + // ); + } "extra" => { /* skip */ } _ => return Err(error::unexpected_child_elem(child)), } diff --git a/src/error.rs b/src/error.rs index a0165cf..d357165 100644 --- a/src/error.rs +++ b/src/error.rs @@ -5,7 +5,7 @@ use std::{fmt, path::Path}; #[cfg(any(feature = "obj", feature = "stl"))] use crate::utils::bytes::{bytecount_naive, memrchr_naive}; -#[cfg(any(feature = "collada", feature = "obj"))] +#[cfg(feature = "collada")] macro_rules! format_err { ($msg:expr $(,)?) => { crate::error::invalid_data($msg) @@ -15,7 +15,7 @@ macro_rules! format_err { }; } -#[cfg(any(feature = "collada", feature = "obj"))] +#[cfg(feature = "collada")] macro_rules! bail { ($($tt:tt)*) => { return Err(format_err!($($tt)*)) diff --git a/src/obj/error.rs b/src/obj/error.rs new file mode 100644 index 0000000..fb74d37 --- /dev/null +++ b/src/obj/error.rs @@ -0,0 +1,53 @@ +use std::{fmt, io, path::Path, str}; + +#[cfg_attr(test, derive(Debug))] +pub(super) enum ErrorKind { + ExpectedSpace(&'static str, usize), + ExpectedNewline(&'static str, usize), + Expected(&'static str, usize), + Float(usize), + Int(usize), + InvalidW(usize), + InvalidFaceIndex(usize), + Oob(usize, usize), + Io(io::Error), +} + +impl ErrorKind { + #[cold] + #[inline(never)] + pub(super) fn into_io_error(self, start: &[u8], path: Option<&Path>) -> io::Error { + let remaining = match self { + Self::Expected(.., n) + | Self::ExpectedNewline(.., n) + | Self::ExpectedSpace(.., n) + | Self::Float(n) + | Self::Int(n) + | Self::InvalidW(n) + | Self::InvalidFaceIndex(n) + | Self::Oob(.., n) => n, + Self::Io(e) => return e, + }; + crate::error::with_location( + &crate::error::invalid_data(self.to_string()), + &crate::error::Location::find(remaining, start, path), + ) + } +} + +impl fmt::Display for ErrorKind { + #[cold] + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match *self { + Self::ExpectedSpace(msg, ..) => write!(f, "expected space after {msg}"), + Self::ExpectedNewline(msg, ..) => write!(f, "expected newline after {msg}"), + Self::Expected(msg, ..) => write!(f, "expected {msg}"), + Self::InvalidW(..) => write!(f, "w in homogeneous vector must not zero"), + Self::InvalidFaceIndex(..) => write!(f, "invalid face index"), + Self::Float(..) => write!(f, "error while parsing a float"), + Self::Int(..) => write!(f, "error while parsing an integer"), + Self::Oob(i, ..) => write!(f, "face index out of bounds ({i})"), + Self::Io(ref e) => write!(f, "{e}"), + } + } +} diff --git a/src/obj/mod.rs b/src/obj/mod.rs index 0ab463f..eae508a 100644 --- a/src/obj/mod.rs +++ b/src/obj/mod.rs @@ -4,6 +4,8 @@ #![allow(clippy::collapsible_if, clippy::many_single_char_names)] +mod error; + use std::{ collections::HashMap, io, mem, @@ -878,7 +880,7 @@ fn read_mtl_internal( // clearcoat_factor: mat.clearcoat_thickness, // clearcoat_roughness_factor: mat.clearcoat_roughness, // anisotropy_factor: mat.anisotropy, - color: crate::Colors { + color: common::Colors { ambient: color4(mat.ambient), diffuse: color4(mat.diffuse), specular: color4(mat.specular), @@ -886,7 +888,7 @@ fn read_mtl_internal( transparent: color4(mat.transparent), reflective: None, }, - texture: crate::Textures { + texture: common::Textures { diffuse: texture_path(mat.diffuse_texture, mtl_dir), ambient: texture_path(mat.ambient_texture, mtl_dir), emissive: texture_path(mat.emissive_texture, mtl_dir), @@ -1320,59 +1322,3 @@ fn name(s: &[u8]) -> (&[u8], &[u8]) { } (name, s_next) } - -mod error { - use std::{fmt, io, path::Path, str}; - - #[cfg_attr(test, derive(Debug))] - pub(super) enum ErrorKind { - ExpectedSpace(&'static str, usize), - ExpectedNewline(&'static str, usize), - Expected(&'static str, usize), - Float(usize), - Int(usize), - InvalidW(usize), - InvalidFaceIndex(usize), - Oob(usize, usize), - Io(io::Error), - } - - impl ErrorKind { - #[cold] - #[inline(never)] - pub(super) fn into_io_error(self, start: &[u8], path: Option<&Path>) -> io::Error { - let remaining = match self { - Self::Expected(.., n) - | Self::ExpectedNewline(.., n) - | Self::ExpectedSpace(.., n) - | Self::Float(n) - | Self::Int(n) - | Self::InvalidW(n) - | Self::InvalidFaceIndex(n) - | Self::Oob(.., n) => n, - Self::Io(e) => return e, - }; - crate::error::with_location( - &crate::error::invalid_data(self.to_string()), - &crate::error::Location::find(remaining, start, path), - ) - } - } - - impl fmt::Display for ErrorKind { - #[cold] - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match *self { - Self::ExpectedSpace(msg, ..) => write!(f, "expected space after {msg}"), - Self::ExpectedNewline(msg, ..) => write!(f, "expected newline after {msg}"), - Self::Expected(msg, ..) => write!(f, "expected {msg}"), - Self::InvalidW(..) => write!(f, "w in homogeneous vector must not zero"), - Self::InvalidFaceIndex(..) => write!(f, "invalid face index"), - Self::Float(..) => write!(f, "error while parsing a float"), - Self::Int(..) => write!(f, "error while parsing an integer"), - Self::Oob(i, ..) => write!(f, "face index out of bounds ({i})"), - Self::Io(ref e) => write!(f, "{e}"), - } - } - } -} diff --git a/src/stl/error.rs b/src/stl/error.rs new file mode 100644 index 0000000..a05447f --- /dev/null +++ b/src/stl/error.rs @@ -0,0 +1,84 @@ +use std::{fmt, io, path::Path}; + +#[cfg_attr(test, derive(Debug))] +pub(super) enum ErrorKind { + // ASCII STL error + ExpectedSpace(&'static str, usize), + ExpectedNewline(&'static str, usize), + Expected(&'static str, usize), + Float(usize), + NotAscii(&'static str, usize), + // binary STL error + TooSmall, + InvalidSize, + TooManyTriangles, +} + +impl ErrorKind { + #[cold] + #[inline(never)] + pub(super) fn into_io_error(self, start: &[u8], path: Option<&Path>) -> io::Error { + let remaining = match self { + // ASCII STL error + Self::Expected(.., n) + | Self::ExpectedNewline(.., n) + | Self::ExpectedSpace(.., n) + | Self::Float(n) + | Self::NotAscii(.., n) => n, + // binary STL error (always points file:1:1, as error occurs only during reading the header) + _ => start.len(), + }; + crate::error::with_location( + &crate::error::invalid_data(self.to_string()), + &crate::error::Location::find(remaining, start, path), + ) + } +} + +impl fmt::Display for ErrorKind { + #[cold] + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match *self { + // ASCII STL error + Self::ExpectedSpace(msg, ..) => { + if msg == "normal" || msg == "vertex" { + write!(f, "expected space before floats") + } else { + write!(f, "expected space after {msg}") + } + } + Self::ExpectedNewline(msg, ..) => { + if msg == "solid" { + write!(f, "expected newline after solid name") + } else if msg == "normal" || msg == "vertex" { + write!(f, "expected newline after floats") + } else { + write!(f, "expected newline after {msg}") + } + } + Self::Expected(msg, remaining) => { + if msg == "solid" && remaining != 0 { + write!(f, "expected solid or eof") + } else if msg == "endsolid" { + write!(f, "expected facet normal or endsolid") + } else { + write!(f, "expected {msg}") + } + } + Self::Float(..) => write!(f, "error while parsing a float"), + Self::NotAscii(..) => write!(f, "invalid ASCII"), + // binary STL error + Self::TooSmall => write!( + f, + "failed to determine STL storage representation: \ + not valid ASCII STL and size is too small as binary STL" + ), + Self::InvalidSize => write!( + f, + "failed to determine STL storage representation: \ + not valid ASCII STL and size is invalid as binary STL" + ), + Self::TooManyTriangles => write!(f, "too many triangles"), + } + } +} diff --git a/src/stl/mod.rs b/src/stl/mod.rs index 77e24f8..0bdfe34 100644 --- a/src/stl/mod.rs +++ b/src/stl/mod.rs @@ -2,6 +2,8 @@ //! //! [STL]: https://en.wikipedia.org/wiki/STL_(file_format) +mod error; + use std::{io, path::Path, str}; use self::error::ErrorKind; @@ -602,90 +604,3 @@ impl FromStl for Mesh { mesh.name = name.to_owned(); } } - -mod error { - use std::{fmt, io, path::Path}; - - #[cfg_attr(test, derive(Debug))] - pub(super) enum ErrorKind { - // ASCII STL error - ExpectedSpace(&'static str, usize), - ExpectedNewline(&'static str, usize), - Expected(&'static str, usize), - Float(usize), - NotAscii(&'static str, usize), - // binary STL error - TooSmall, - InvalidSize, - TooManyTriangles, - } - - impl ErrorKind { - #[cold] - #[inline(never)] - pub(super) fn into_io_error(self, start: &[u8], path: Option<&Path>) -> io::Error { - let remaining = match self { - // ASCII STL error - Self::Expected(.., n) - | Self::ExpectedNewline(.., n) - | Self::ExpectedSpace(.., n) - | Self::Float(n) - | Self::NotAscii(.., n) => n, - // binary STL error (always points file:1:1, as error occurs only during reading the header) - _ => start.len(), - }; - crate::error::with_location( - &crate::error::invalid_data(self.to_string()), - &crate::error::Location::find(remaining, start, path), - ) - } - } - - impl fmt::Display for ErrorKind { - #[cold] - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match *self { - // ASCII STL error - Self::ExpectedSpace(msg, ..) => { - if msg == "normal" || msg == "vertex" { - write!(f, "expected space before floats") - } else { - write!(f, "expected space after {msg}") - } - } - Self::ExpectedNewline(msg, ..) => { - if msg == "solid" { - write!(f, "expected newline after solid name") - } else if msg == "normal" || msg == "vertex" { - write!(f, "expected newline after floats") - } else { - write!(f, "expected newline after {msg}") - } - } - Self::Expected(msg, remaining) => { - if msg == "solid" && remaining != 0 { - write!(f, "expected solid or eof") - } else if msg == "endsolid" { - write!(f, "expected facet normal or endsolid") - } else { - write!(f, "expected {msg}") - } - } - Self::Float(..) => write!(f, "error while parsing a float"), - Self::NotAscii(..) => write!(f, "invalid ASCII"), - // binary STL error - Self::TooSmall => write!( - f, - "failed to determine STL storage representation: \ - not valid ASCII STL and size is too small as binary STL" - ), - Self::InvalidSize => write!( - f, - "failed to determine STL storage representation: \ - not valid ASCII STL and size is invalid as binary STL" - ), - Self::TooManyTriangles => write!(f, "too many triangles"), - } - } - } -} diff --git a/src/utils/mod.rs b/src/utils/mod.rs index d723401..73b8dc6 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -12,6 +12,8 @@ pub(crate) mod xml; pub(crate) mod utf16 { use std::{borrow::Cow, io}; + use crate::error; + const UTF32BE_BOM: &[u8] = &[0xFF, 0xFE, 00, 00]; const UTF32LE_BOM: &[u8] = &[00, 00, 0xFE, 0xFF]; const UTF16BE_BOM: &[u8] = &[0xFE, 0xFF]; @@ -24,9 +26,9 @@ pub(crate) mod utf16 { if bytes.starts_with(UTF8_BOM) { std::str::from_utf8(&bytes[UTF8_BOM.len()..]) .map(Cow::Borrowed) - .map_err(crate::error::invalid_data) + .map_err(error::invalid_data) } else if bytes.starts_with(UTF32BE_BOM) || bytes.starts_with(UTF32LE_BOM) { - bail!("utf-32 is not supported") + return Err(error::invalid_data("utf-32 is not supported")); } else if bytes.starts_with(UTF16BE_BOM) { from_utf16be(&bytes[UTF16BE_BOM.len()..]).map(Into::into) } else if bytes.starts_with(UTF16LE_BOM) { @@ -35,7 +37,7 @@ pub(crate) mod utf16 { // UTF-16/UTF-32 without BOM will get an error here. std::str::from_utf8(bytes) .map(Cow::Borrowed) - .map_err(crate::error::invalid_data) + .map_err(error::invalid_data) } } @@ -47,7 +49,7 @@ pub(crate) mod utf16 { if bytes.starts_with(UTF8_BOM) { Ok(Cow::Borrowed(&bytes[UTF8_BOM.len()..])) } else if bytes.starts_with(UTF32BE_BOM) || bytes.starts_with(UTF32LE_BOM) { - bail!("utf-32 is not supported") + return Err(error::invalid_data("utf-32 is not supported")); } else if bytes.starts_with(UTF16BE_BOM) { from_utf16be(&bytes[UTF16BE_BOM.len()..]) .map(String::into_bytes) @@ -65,7 +67,7 @@ pub(crate) mod utf16 { #[inline(never)] fn from_utf16be(bytes: &[u8]) -> io::Result { if bytes.len() % 2 != 0 { - bail!("invalid utf-16: lone surrogate found"); + return Err(error::invalid_data("invalid utf-16: lone surrogate found")); } char::decode_utf16( bytes @@ -73,14 +75,14 @@ pub(crate) mod utf16 { .map(|b| u16::from_be_bytes(b.try_into().unwrap())), ) .collect::>() - .map_err(crate::error::invalid_data) + .map_err(error::invalid_data) } #[cold] #[inline(never)] fn from_utf16le(bytes: &[u8]) -> io::Result { if bytes.len() % 2 != 0 { - bail!("invalid utf-16: lone surrogate found"); + return Err(error::invalid_data("invalid utf-16: lone surrogate found")); } char::decode_utf16( bytes @@ -88,6 +90,6 @@ pub(crate) mod utf16 { .map(|b| u16::from_le_bytes(b.try_into().unwrap())), ) .collect::>() - .map_err(crate::error::invalid_data) + .map_err(error::invalid_data) } }