From d24d8f97085917176dba0c2d3ab0da88ec38e238 Mon Sep 17 00:00:00 2001 From: Orual Date: Sun, 2 Aug 2026 19:34:36 -0400 Subject: [PATCH] PM-78: fix H4-H5 and M2-M4/M7 typed semantic validation Epic: PM-86 Task: PM-78 --- crates/polymodel-ldraw-core/src/bfc.rs | 14 ++ crates/polymodel-ldraw-core/src/geom.rs | 165 ++++++++++++------- crates/polymodel-ldraw-core/src/lib.rs | 13 +- crates/polymodel-ldraw-core/src/model.rs | 122 ++++++-------- crates/polymodel-ldraw-core/src/mpd.rs | 2 +- crates/polymodel-ldraw-core/src/parser.rs | 43 +++-- crates/polymodel-ldraw-core/src/scanner.rs | 32 ++-- crates/polymodel-ldraw-core/src/texmap.rs | 14 -- crates/polymodel-ldraw-core/src/traversal.rs | 41 +++-- crates/polymodel-ldraw-core/src/types.rs | 12 +- crates/polymodel-ldraw-core/src/util.rs | 17 -- 11 files changed, 263 insertions(+), 212 deletions(-) diff --git a/crates/polymodel-ldraw-core/src/bfc.rs b/crates/polymodel-ldraw-core/src/bfc.rs index f82c71a..7557b88 100644 --- a/crates/polymodel-ldraw-core/src/bfc.rs +++ b/crates/polymodel-ldraw-core/src/bfc.rs @@ -112,6 +112,7 @@ pub(crate) fn bfc( } model.bfc.state = BfcState::Uncertified; model.bfc.clipping = false; + model.bfc.winding = Winding::Ccw; model.bfc.invert_next = false; } TokenKind::InvertNext => { @@ -142,6 +143,19 @@ pub(crate) fn bfc( } } TokenKind::Clip | TokenKind::NoClip | TokenKind::Orientation => { + if line.tokens.len() != 3 { + model.bfc.invert_next = false; + return add_diag( + profile, + diagnostics, + counters, + limits, + DiagnosticCode::BfcInvalidDirective, + line.span, + "CLIP, NOCLIP, CW, and CCW take no arguments", + true, + ); + } if model.bfc.state != BfcState::Certified { return add_diag( profile, diff --git a/crates/polymodel-ldraw-core/src/geom.rs b/crates/polymodel-ldraw-core/src/geom.rs index de6ee89..0a0525b 100644 --- a/crates/polymodel-ldraw-core/src/geom.rs +++ b/crates/polymodel-ldraw-core/src/geom.rs @@ -8,6 +8,16 @@ use crate::types::{ use serde::{Deserialize, Deserializer, Serialize}; use std::borrow::Cow; +fn finite_product(left: f64, right: f64) -> Option { + let value = left * right; + value.is_finite().then_some(value) +} + +fn finite_sum(values: [f64; 3]) -> Option { + let value = values[0] + values[1] + values[2]; + value.is_finite().then_some(value) +} + #[derive(Clone, Copy, Debug, PartialEq, Serialize, Deserialize)] pub struct Point3 { pub x: f64, @@ -37,30 +47,36 @@ impl Matrix3 { } } - fn multiply(self, other: Self) -> Self { - let dot = |a: Point3, b: Point3| a.x * b.x + a.y * b.y + a.z * b.z; + fn multiply(self, other: Self) -> Option { + let dot = |a: Point3, b: Point3| { + finite_sum([ + finite_product(a.x, b.x)?, + finite_product(a.y, b.y)?, + finite_product(a.z, b.z)?, + ]) + }; let columns = [ Point3::new(other.row_x.x, other.row_y.x, other.row_z.x), Point3::new(other.row_x.y, other.row_y.y, other.row_z.y), Point3::new(other.row_x.z, other.row_y.z, other.row_z.z), ]; - Self { + Some(Self { row_x: Point3::new( - dot(self.row_x, columns[0]), - dot(self.row_x, columns[1]), - dot(self.row_x, columns[2]), + dot(self.row_x, columns[0])?, + dot(self.row_x, columns[1])?, + dot(self.row_x, columns[2])?, ), row_y: Point3::new( - dot(self.row_y, columns[0]), - dot(self.row_y, columns[1]), - dot(self.row_y, columns[2]), + dot(self.row_y, columns[0])?, + dot(self.row_y, columns[1])?, + dot(self.row_y, columns[2])?, ), row_z: Point3::new( - dot(self.row_z, columns[0]), - dot(self.row_z, columns[1]), - dot(self.row_z, columns[2]), + dot(self.row_z, columns[0])?, + dot(self.row_z, columns[1])?, + dot(self.row_z, columns[2])?, ), - } + }) } } @@ -101,21 +117,28 @@ impl Transform { pub fn reflection(&self) -> bool { self.determinant() < 0.0 } - pub fn apply(&self, p: Point3) -> Point3 { + pub fn apply(&self, p: Point3) -> Option { let x = self.matrix.row_x; let y = self.matrix.row_y; let z = self.matrix.row_z; - Point3::new( - self.translation.x + x.x * p.x + x.y * p.y + x.z * p.z, - self.translation.y + y.x * p.x + y.y * p.y + y.z * p.z, - self.translation.z + z.x * p.x + z.y * p.y + z.z * p.z, - ) + let dot = |row: Point3| { + finite_sum([ + finite_product(row.x, p.x)?, + finite_product(row.y, p.y)?, + finite_product(row.z, p.z)?, + ]) + }; + Some(Point3::new( + finite_sum([self.translation.x, dot(x)?, 0.0])?, + finite_sum([self.translation.y, dot(y)?, 0.0])?, + finite_sum([self.translation.z, dot(z)?, 0.0])?, + )) } - pub fn compose(&self, child: &Self) -> Self { - Self { - translation: self.apply(child.translation), - matrix: self.matrix.multiply(child.matrix), - } + pub fn compose(&self, child: &Self) -> Option { + Some(Self { + translation: self.apply(child.translation)?, + matrix: self.matrix.multiply(child.matrix)?, + }) } } @@ -124,15 +147,11 @@ pub struct Bounds3 { pub min: Point3, pub max: Point3, } -impl Default for Bounds3 { - fn default() -> Self { - Self { - min: Point3::new(f64::INFINITY, f64::INFINITY, f64::INFINITY), - max: Point3::new(f64::NEG_INFINITY, f64::NEG_INFINITY, f64::NEG_INFINITY), - } - } -} impl Bounds3 { + pub(crate) fn from_point(p: Point3) -> Self { + Self { min: p, max: p } + } + pub(crate) fn add(&mut self, p: Point3) { self.min.x = self.min.x.min(p.x); self.min.y = self.min.y.min(p.y); @@ -143,7 +162,9 @@ impl Bounds3 { } } -pub(crate) fn deserialize_bounds3<'de, D>(deserializer: D) -> Result +pub(crate) fn deserialize_optional_bounds3<'de, D>( + deserializer: D, +) -> Result, D::Error> where D: Deserializer<'de>, { @@ -154,31 +175,38 @@ where Legacy([String; 6]), } - match Representation::deserialize(deserializer)? { - Representation::Typed(bounds) => Ok(bounds), - Representation::Legacy(values) => { + let value = Option::::deserialize(deserializer)?; + match value { + None => Ok(None), + Some(Representation::Typed(bounds)) => Ok(Some(bounds)), + Some(Representation::Legacy(values)) => { let values = values.map(|value| value.parse::().map_err(serde::de::Error::custom)); let [min_x, min_y, min_z, max_x, max_y, max_z] = values .into_iter() .collect::, _>>()? .try_into() .map_err(|_| serde::de::Error::custom("six legacy bounds values required"))?; - Ok(Bounds3 { + Ok(Some(Bounds3 { min: Point3::new(min_x, min_y, min_z), max: Point3::new(max_x, max_y, max_z), - }) + })) } } } fn parse_number(token: Option<&Token>) -> Option { - token?.text.parse::().ok().filter(|n| n.is_finite()) + token?.number +} + +fn parse_geometry_colour(token: Option<&Token>) -> Option { + let value = token?.text.parse::().ok()?; + (value <= 511 || value & 0xff00_0000 == 0x0200_0000).then_some(ColourCode(value)) } fn parse_points(tokens: &[Token], count: usize) -> Option> { let coordinate_count = count.checked_mul(3)?; let expected = coordinate_count.checked_add(2)?; - if tokens.len() != expected || parse_number(tokens.get(1)).is_none() { + if tokens.len() != expected || tokens.get(1).is_none() { return None; } let mut values = Vec::with_capacity(coordinate_count); @@ -198,26 +226,39 @@ pub(crate) enum ParsedPrimitive { Points(Vec), } -#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)] -pub struct ColourCode(pub u16); +#[derive(Clone, Copy, Debug, PartialEq, Eq, Ord, PartialOrd, Hash, Serialize, Deserialize)] +pub struct ColourCode(pub u32); -impl From for u16 { - fn from(c: ColourCode) -> u16 { +impl From for u32 { + fn from(c: ColourCode) -> u32 { c.0 } } +impl From for ColourCode { + fn from(value: u16) -> Self { + Self(u32::from(value)) + } +} + +impl From for ColourCode { + fn from(value: u32) -> Self { + Self(value) + } +} + #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] pub struct GeometryRecord { - pub line_type: u8, + pub line_type: LineType, pub colour: ColourCode, + /// Type-5 vertices are ordered as endpoint1, endpoint2, control1, control2. pub vertices: Vec, } pub(crate) fn parse_primitive(line: &ScannedLine) -> Option { match line.line_type { Some(LineType::One) => { - if line.tokens.len() < 15 || parse_number(line.tokens.get(1)).is_none() { + if line.tokens.len() < 15 || line.tokens.get(1).is_none() { return None; } let values = (2..14) @@ -246,6 +287,7 @@ pub(crate) fn process_geometry<'src>( match line.line_type { Some(LineType::One) => { let Some(ParsedPrimitive::Include(transform)) = primitive else { + model.bfc.invert_next = false; return add_diag( profile, diagnostics, @@ -273,7 +315,6 @@ pub(crate) fn process_geometry<'src>( name, transform, span: line.span, - valid: true, inverted, }); if model.bfc.state == BfcState::Unknown { @@ -313,17 +354,29 @@ pub(crate) fn process_geometry<'src>( counters .add(line_kind, line_delta, limits) .map_err(|name| limit_error(name, Some(line.span)))?; - for point in &points { - model.bounds.add(*point); + for point in points.iter().copied() { + if let Some(bounds) = model.bounds.as_mut() { + bounds.add(point); + } else { + model.bounds = Some(Bounds3::from_point(point)); + } } - let colour = line - .tokens - .get(1) - .and_then(|t| t.text.parse::().ok()) - .unwrap_or(16); + let Some(colour) = parse_geometry_colour(line.tokens.get(1)) else { + model.bfc.invert_next = false; + return add_diag( + profile, + diagnostics, + counters, + limits, + DiagnosticCode::InvalidColour, + line.span, + "invalid geometry colour code", + true, + ); + }; model.geometry.push(GeometryRecord { - line_type: u8::from(typ), - colour: ColourCode(colour), + line_type: typ, + colour, vertices: points, }); match typ { diff --git a/crates/polymodel-ldraw-core/src/lib.rs b/crates/polymodel-ldraw-core/src/lib.rs index 46513b0..c7ba3a1 100644 --- a/crates/polymodel-ldraw-core/src/lib.rs +++ b/crates/polymodel-ldraw-core/src/lib.rs @@ -218,10 +218,11 @@ mod tests { for (index, (record, (line_type, colour, vertices))) in geometry.iter().zip(expected).enumerate() { - assert_eq!(record.line_type, *line_type, "geometry[{index}] line type"); + let line_type = LineType::from(*line_type); + assert_eq!(record.line_type, line_type, "geometry[{index}] line type"); assert_eq!( record.colour, - ColourCode(*colour), + ColourCode::from(*colour), "geometry[{index}] colour" ); assert_eq!( @@ -513,15 +514,15 @@ mod tests { .parse_bytes(bytes, options(&ledger), None) .expect("colour definitions fixture should parse"); - let bright_red = result.colours.get(300).expect("colour code 300"); + let bright_red = result.colours.get(300u16).expect("colour code 300"); assert_eq!(bright_red.data.r, 0xC9); assert_eq!(bright_red.data.g, 0x1A); assert_eq!(bright_red.data.b, 0x09); assert_eq!(bright_red.data.alpha, Some(128)); assert_eq!(bright_red.data.luminance, Some(50)); - assert_eq!(bright_red.edge, ColourEdge::Code(0)); + assert_eq!(bright_red.edge, ColourEdge::Code(ColourCode(0))); - let edge_data = result.colours.get(301).expect("colour code 301"); + let edge_data = result.colours.get(301u16).expect("colour code 301"); assert_eq!(edge_data.data.r, 0x12); assert_eq!(edge_data.data.g, 0x34); assert_eq!(edge_data.data.b, 0x56); @@ -537,7 +538,7 @@ mod tests { luminance: None, }) ); - assert_eq!(result.semantic.colour_state, 301); + assert_eq!(result.semantic.colour_state, crate::ColourCode(301)); assert_scene_counts(&result, 2, 0, 0, 0); assert_geometry( &result.models[0].geometry, diff --git a/crates/polymodel-ldraw-core/src/model.rs b/crates/polymodel-ldraw-core/src/model.rs index 7c3d11a..78d8987 100644 --- a/crates/polymodel-ldraw-core/src/model.rs +++ b/crates/polymodel-ldraw-core/src/model.rs @@ -1,7 +1,6 @@ use crate::bfc::{BfcFrame, BfcState}; use crate::cache::{CacheKey, NormalizedPath}; -use crate::geom::GeometryRecord; -use crate::geom::{Bounds3, Transform, deserialize_bounds3}; +use crate::geom::{Bounds3, ColourCode, GeometryRecord, Transform, deserialize_optional_bounds3}; use crate::mpd::OwnedVirtualFile; use crate::scanner::{LineType, Token, TokenKind}; use crate::texmap::{TexmapEvent, TexmapState, TextureDescriptor}; @@ -21,13 +20,13 @@ pub struct ColourData { #[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] pub enum ColourEdge { - Code(u16), + Code(ColourCode), Data(ColourData), } #[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] pub struct Colour<'src> { - pub code: u16, + pub code: ColourCode, #[serde(borrow)] pub name: Option>, pub data: ColourData, @@ -37,10 +36,7 @@ pub struct Colour<'src> { #[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] pub struct ColourTable<'src> { #[serde(borrow)] - entries: BTreeMap>, - #[serde(borrow)] - local_slots: BTreeMap, u16>, - next_local: u16, + entries: BTreeMap>, } impl<'src> Default for ColourTable<'src> { @@ -53,8 +49,6 @@ impl<'src> ColourTable<'src> { pub fn new() -> Self { Self { entries: BTreeMap::new(), - local_slots: BTreeMap::new(), - next_local: 0, } } @@ -62,11 +56,12 @@ impl<'src> ColourTable<'src> { self.entries.insert(colour.code, colour); } - pub fn get(&self, code: u16) -> Option<&Colour<'src>> { - self.entries.get(&code) + pub fn get(&self, code: impl Into) -> Option<&Colour<'src>> { + self.entries.get(&code.into()) } - pub fn find(&self, code: u16, find_edge: bool) -> Option<&ColourData> { + pub fn find(&self, code: impl Into, find_edge: bool) -> Option<&ColourData> { + let code = code.into(); let colour = self.entries.get(&code)?; if !find_edge { return Some(&colour.data); @@ -85,23 +80,6 @@ impl<'src> ColourTable<'src> { self.entries.is_empty() } - pub(crate) fn local_len(&self) -> usize { - self.local_slots.len() - } - - pub(crate) fn register_local(&mut self, name: &'src str) -> Option { - if let Some(slot) = self.local_slots.get(name) { - return Some(*slot); - } - if self.next_local >= 512 { - return None; - } - let slot = self.next_local; - self.next_local = self.next_local.checked_add(1)?; - self.local_slots.insert(Cow::Borrowed(name), slot); - Some(slot) - } - pub(crate) fn into_owned(self) -> ColourTable<'static> { ColourTable { entries: self @@ -119,12 +97,6 @@ impl<'src> ColourTable<'src> { ) }) .collect(), - local_slots: self - .local_slots - .into_iter() - .map(|(name, slot)| (Cow::Owned(name.into_owned()), slot)) - .collect(), - next_local: self.next_local, } } } @@ -133,40 +105,50 @@ pub(crate) fn parse_colour<'src>(tokens: &[Token<'src>]) -> Option> if tokens.len() < 5 || tokens.get(2)?.kind != TokenKind::Identifier { return None; } + if tokens[2].text.ends_with(';') { + return None; + } + let fields = &tokens[3..]; + if fields.len() % 2 != 0 { + return None; + } let mut code = None; - let mut value: Option<(u8, u8, u8)> = None; + let mut value = None; let mut edge = None; let mut alpha = None; let mut luminance = None; - let mut index = 3; - while index < tokens.len() { - let value_token = tokens.get(index + 1); - match tokens[index].kind { - TokenKind::Code => code = value_token.and_then(|token| token.text.parse().ok()), - TokenKind::Value => value = value_token.and_then(|token| parse_rgb(token.text)), - TokenKind::Edge => { - edge = value_token.and_then(|token| { - parse_rgb(token.text) - .map(|(r, g, b)| { - ColourEdge::Data(ColourData { - r, - g, - b, - alpha: None, - luminance: None, - }) + for pair in fields.chunks_exact(2) { + let key = pair[0].kind; + let value_token = &pair[1]; + match key { + TokenKind::Code if code.is_none() => code = parse_colour_code(value_token.text), + TokenKind::Value if value.is_none() => value = parse_rgb(value_token.text), + TokenKind::Edge if edge.is_none() => { + edge = parse_rgb(value_token.text) + .map(|(r, g, b)| { + ColourEdge::Data(ColourData { + r, + g, + b, + alpha: None, + luminance: None, }) - .or_else(|| token.text.parse().ok().map(ColourEdge::Code)) - }); + }) + .or_else(|| parse_colour_code(value_token.text).map(ColourEdge::Code)); } - TokenKind::Alpha => alpha = value_token.and_then(|token| token.text.parse().ok()), - TokenKind::Luminance => { - luminance = value_token.and_then(|token| token.text.parse().ok()) + TokenKind::Alpha if alpha.is_none() => alpha = value_token.text.parse().ok(), + TokenKind::Luminance if luminance.is_none() => { + luminance = value_token.text.parse().ok() } - _ => {} + TokenKind::Chrome + | TokenKind::Pearlescent + | TokenKind::Rubber + | TokenKind::MatteMetallic + | TokenKind::Metal + | TokenKind::Material => return None, + _ => return None, } - index += 1; } Some(Colour { @@ -183,6 +165,11 @@ pub(crate) fn parse_colour<'src>(tokens: &[Token<'src>]) -> Option> }) } +fn parse_colour_code(text: &str) -> Option { + let value = text.parse::().ok()?; + Some(ColourCode(value)) +} + fn parse_rgb(text: &str) -> Option<(u8, u8, u8)> { let hex = text.strip_prefix('#')?; if hex.len() != 6 { @@ -217,7 +204,7 @@ pub struct SemanticRecord<'src> { #[serde(borrow)] pub cache_identity: Option>, pub bfc_state: BfcState, - pub colour_state: u16, + pub colour_state: ColourCode, pub steps: Vec, #[serde(borrow)] pub limits: Vec>, @@ -242,8 +229,8 @@ pub struct SceneRecord { pub quads: u64, pub lines: u64, pub conditional_lines: u64, - #[serde(deserialize_with = "deserialize_bounds3")] - pub bounds: Bounds3, + #[serde(deserialize_with = "deserialize_optional_bounds3")] + pub bounds: Option, pub reflection: bool, } #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] @@ -262,7 +249,6 @@ pub(crate) struct Include<'src> { pub(crate) name: Cow<'src, str>, pub transform: Transform, pub(crate) span: Span, - pub(crate) valid: bool, pub(crate) inverted: bool, } #[derive(Clone, Debug)] @@ -271,8 +257,7 @@ pub(crate) struct ModelData<'src> { pub(crate) path: NormalizedPath, pub(crate) key: CacheKey, pub(crate) bfc: BfcFrame, - pub(crate) colour: u16, - pub(crate) colours: ColourTable<'src>, + pub(crate) colour: ColourCode, pub(crate) steps: Vec>, pub(crate) texmap_state: TexmapState, pub(crate) texmap_descriptor: Option>, @@ -287,8 +272,7 @@ pub(crate) struct ModelData<'src> { pub(crate) lines: u64, pub(crate) conditional_lines: u64, pub(crate) geometry: Vec, - pub(crate) bounds: Bounds3, - pub(crate) reflection: bool, + pub(crate) bounds: Option, } pub struct ParseResult<'src> { diff --git a/crates/polymodel-ldraw-core/src/mpd.rs b/crates/polymodel-ldraw-core/src/mpd.rs index 380442e..d6b164d 100644 --- a/crates/polymodel-ldraw-core/src/mpd.rs +++ b/crates/polymodel-ldraw-core/src/mpd.rs @@ -5,7 +5,7 @@ use crate::types::{ use serde::{Deserialize, Serialize}; use std::borrow::Cow; -#[derive(Clone, Debug, Eq, PartialEq)] +#[derive(Clone, Debug, PartialEq)] pub struct VirtualFile<'src> { pub id: u32, pub name: Cow<'src, str>, diff --git a/crates/polymodel-ldraw-core/src/parser.rs b/crates/polymodel-ldraw-core/src/parser.rs index 19757db..dc6c743 100644 --- a/crates/polymodel-ldraw-core/src/parser.rs +++ b/crates/polymodel-ldraw-core/src/parser.rs @@ -1,6 +1,7 @@ -use crate::bfc::{BfcFrame, BfcState, bfc}; +use crate::BfcFrame; +use crate::bfc::{BfcState, bfc}; use crate::cache::{CacheKey, NormalizedPath}; -use crate::geom::{Bounds3, parse_primitive, process_geometry}; +use crate::geom::{parse_primitive, process_geometry}; use crate::model::{ ColourTable, ModelData, ParseResult, SemanticRecord, SyntaxRecord, parse_colour, }; @@ -106,11 +107,13 @@ impl LdrawParser { .map(|include| include.name.to_string()) .collect::>(); for include_name in include_names { - let normalized_name = include_name.replace('\\', "/"); - if models - .iter() - .any(|model| model.path.as_str().eq_ignore_ascii_case(&normalized_name)) - || !visited_names.insert(normalized_name.to_ascii_lowercase()) + let normalized_path = match NormalizedPath::new(&include_name) { + Ok(path) => path, + Err(_) => continue, + }; + let normalized_name = normalized_path.to_string(); + if models.iter().any(|model| model.path == normalized_path) + || !visited_names.insert(normalized_name.clone()) { continue; } @@ -222,7 +225,6 @@ fn parse_model<'src>( colours: &mut ColourTable<'src>, texture_ids: &mut BTreeSet>, ) -> Result, ParseError> { - let colours_before = colours.local_len(); if options.cancellation.cancelled { return Err(ParseError::Cancelled); } @@ -244,7 +246,6 @@ fn parse_model<'src>( key, bfc: BfcFrame::default(), colour: DEFAULT_COLOUR, - colours: colours.clone(), steps: Vec::new(), texmap_state: TexmapState::Inactive, texmap_descriptor: None, @@ -259,8 +260,7 @@ fn parse_model<'src>( lines: 0, conditional_lines: 0, geometry: Vec::new(), - bounds: Bounds3::default(), - reflection: false, + bounds: None, }; for line in &file.lines { if line.blank { @@ -320,11 +320,7 @@ fn parse_model<'src>( } let include_count = u64::try_from(model.includes.len()) .map_err(|_| ParseError::Overflow("include count conversion"))?; - let colour_entries = colours - .local_len() - .checked_sub(colours_before) - .ok_or(ParseError::Overflow("colour entry delta"))?; - let colour_entries = u64::try_from(colour_entries) + let colour_entries = u64::try_from(colours.len()) .map_err(|_| ParseError::Overflow("colour entry conversion"))?; let graph_entries = include_count .checked_add(1) @@ -507,10 +503,25 @@ fn process_meta<'src>( model.texmap_fallback_seen = false; } TokenKind::Colour => { + if tokens.get(2).is_some_and(|token| token.text.ends_with(';')) { + model.bfc.invert_next = false; + return Ok(()); + } if let Some(colour) = parse_colour(tokens) { let code = colour.code; colours.record(colour); model.colour = code; + } else { + add_diag( + profile, + diagnostics, + counters, + limits, + DiagnosticCode::InvalidColour, + line.span, + "malformed !COLOUR directive", + true, + )?; } model.bfc.invert_next = false; } diff --git a/crates/polymodel-ldraw-core/src/scanner.rs b/crates/polymodel-ldraw-core/src/scanner.rs index 7a151a0..44d6b8e 100644 --- a/crates/polymodel-ldraw-core/src/scanner.rs +++ b/crates/polymodel-ldraw-core/src/scanner.rs @@ -47,14 +47,15 @@ pub enum TokenKind { Garbage, } -#[derive(Clone, Copy, Debug, Eq, PartialEq)] +#[derive(Clone, Copy, Debug, PartialEq)] pub struct Token<'src> { pub kind: TokenKind, pub text: &'src str, + pub number: Option, pub span: Span, } -#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize, Eq, Hash)] pub enum LineEnding { Lf, CrLf, @@ -95,6 +96,20 @@ impl From for u8 { } } +impl From for LineType { + fn from(v: u8) -> Self { + match v { + 0 => LineType::Zero, + 1 => LineType::One, + 2 => LineType::Two, + 3 => LineType::Three, + 4 => LineType::Four, + 5 => LineType::Five, + _ => panic!("invalid line type: {v}"), + } + } +} + impl serde::Serialize for LineType { fn serialize(&self, s: S) -> Result { s.serialize_u8((*self).into()) @@ -116,7 +131,7 @@ impl<'de> serde::Deserialize<'de> for LineType { } } -#[derive(Clone, Debug, Eq, PartialEq)] +#[derive(Clone, Debug, PartialEq)] pub struct ScannedLine<'src> { pub line_type: Option, pub raw: &'src [u8], @@ -290,6 +305,7 @@ fn scan_one<'src>( tokens.push(Token { kind: TokenKind::QuotedIdentifier, text: value, + number: None, span, }); continue; @@ -299,14 +315,9 @@ fn scan_one<'src>( } &text[begin..i] }; + let number = value.parse::().ok().filter(|n| n.is_finite()); let kind = keyword(value) - .or_else(|| { - value - .parse::() - .ok() - .filter(|n| n.is_finite()) - .map(|_| TokenKind::Number) - }) + .or_else(|| number.map(|_| TokenKind::Number)) .unwrap_or(TokenKind::Identifier); let begin_u32 = u32::try_from(begin).map_err(|_| ParseError::Overflow("token span start"))?; @@ -317,6 +328,7 @@ fn scan_one<'src>( tokens.push(Token { kind, text: value, + number, span: Span { start: start .checked_add(begin_u32) diff --git a/crates/polymodel-ldraw-core/src/texmap.rs b/crates/polymodel-ldraw-core/src/texmap.rs index 8bab450..eeb4a62 100644 --- a/crates/polymodel-ldraw-core/src/texmap.rs +++ b/crates/polymodel-ldraw-core/src/texmap.rs @@ -47,20 +47,6 @@ pub struct TextureDescriptor<'src> { #[serde(borrow)] pub glossmap: Option>, } -impl<'src> TextureDescriptor<'src> { - pub(crate) fn into_owned(self) -> TextureDescriptor<'static> { - TextureDescriptor { - mode: self.mode, - parameters: self - .parameters - .into_iter() - .map(|value| Cow::Owned(value.into_owned())) - .collect(), - pngfile: Cow::Owned(self.pngfile.into_owned()), - glossmap: self.glossmap.map(|value| Cow::Owned(value.into_owned())), - } - } -} #[derive(Clone, Copy, Debug, Eq, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] pub enum TexmapKind { diff --git a/crates/polymodel-ldraw-core/src/traversal.rs b/crates/polymodel-ldraw-core/src/traversal.rs index 16c7854..b98f025 100644 --- a/crates/polymodel-ldraw-core/src/traversal.rs +++ b/crates/polymodel-ldraw-core/src/traversal.rs @@ -23,7 +23,7 @@ pub(crate) fn traverse( quads: 0, lines: 0, conditional_lines: 0, - bounds: Bounds3::default(), + bounds: None, reflection: false, }, Vec::new(), @@ -54,7 +54,7 @@ pub(crate) fn traverse( let mut summaries = Vec::new(); let mut instance_ids = Vec::new(); let mut instances = Vec::new(); - let mut scene_bounds = Bounds3::default(); + let mut scene_bounds: Option = None; let mut scene_triangles = 0u64; let mut scene_quads = 0u64; let mut scene_lines = 0u64; @@ -114,18 +114,27 @@ pub(crate) fn traverse( }); } - let corners = [ - Point3::new(model.bounds.min.x, model.bounds.min.y, model.bounds.min.z), - Point3::new(model.bounds.min.x, model.bounds.min.y, model.bounds.max.z), - Point3::new(model.bounds.min.x, model.bounds.max.y, model.bounds.min.z), - Point3::new(model.bounds.min.x, model.bounds.max.y, model.bounds.max.z), - Point3::new(model.bounds.max.x, model.bounds.min.y, model.bounds.min.z), - Point3::new(model.bounds.max.x, model.bounds.min.y, model.bounds.max.z), - Point3::new(model.bounds.max.x, model.bounds.max.y, model.bounds.min.z), - Point3::new(model.bounds.max.x, model.bounds.max.y, model.bounds.max.z), - ]; - for corner in corners { - scene_bounds.add(transform.apply(corner)); + if let Some(model_bounds) = model.bounds { + let corners = [ + Point3::new(model_bounds.min.x, model_bounds.min.y, model_bounds.min.z), + Point3::new(model_bounds.min.x, model_bounds.min.y, model_bounds.max.z), + Point3::new(model_bounds.min.x, model_bounds.max.y, model_bounds.min.z), + Point3::new(model_bounds.min.x, model_bounds.max.y, model_bounds.max.z), + Point3::new(model_bounds.max.x, model_bounds.min.y, model_bounds.min.z), + Point3::new(model_bounds.max.x, model_bounds.min.y, model_bounds.max.z), + Point3::new(model_bounds.max.x, model_bounds.max.y, model_bounds.min.z), + Point3::new(model_bounds.max.x, model_bounds.max.y, model_bounds.max.z), + ]; + for corner in corners { + let transformed = transform + .apply(corner) + .ok_or(ParseError::Overflow("scene transform arithmetic"))?; + if let Some(bounds) = scene_bounds.as_mut() { + bounds.add(transformed); + } else { + scene_bounds = Some(Bounds3::from_point(transformed)); + } + } } scene_triangles = scene_triangles .checked_add(model.triangles) @@ -165,7 +174,9 @@ pub(crate) fn traverse( child_name, include_position + 1 ); - let composed_transform = transform.compose(&include.transform); + let composed_transform = transform + .compose(&include.transform) + .ok_or(ParseError::Overflow("include transform arithmetic"))?; let inverted = reflected ^ include.inverted ^ include.transform.reflection(); instance_ids.push(instance_id.clone()); instances.push(InstanceRecord { diff --git a/crates/polymodel-ldraw-core/src/types.rs b/crates/polymodel-ldraw-core/src/types.rs index eb4a7bd..c2055fe 100644 --- a/crates/polymodel-ldraw-core/src/types.rs +++ b/crates/polymodel-ldraw-core/src/types.rs @@ -1,4 +1,5 @@ use miette::SourceSpan; +use crate::geom::ColourCode; use polymodel_renderer_ledger::{ AdmissionError, RejectionReason, Reservation, ReservationLedger, ReservationOwner, ResourceClass, @@ -9,7 +10,7 @@ use thiserror::Error; pub const SCHEMA_VERSION: &str = "ldraw-canonical-v1"; pub const DEFAULT_ROOT_NAME: &str = "model.ldr"; -pub const DEFAULT_COLOUR: u16 = 16; +pub const DEFAULT_COLOUR: ColourCode = ColourCode(16); pub const DEFAULT_PROVENANCE: &str = "local-parse"; pub const DEFAULT_RESOURCE_BYTES: u64 = 100 * 1024 * 1024; pub const DEFAULT_LINE_BYTES: u64 = 1024 * 1024; @@ -76,6 +77,7 @@ pub enum DiagnosticCode { InvalidType4, InvalidType5, InvalidLineType, + InvalidColour, MetaEmpty, TexmapNextType0, ColourSlotsLimit, @@ -121,6 +123,7 @@ impl fmt::Display for DiagnosticCode { Self::InvalidType4 => "INVALID_TYPE4", Self::InvalidType5 => "INVALID_TYPE5", Self::InvalidLineType => "INVALID_LINE_TYPE", + Self::InvalidColour => "INVALID_COLOUR", Self::MetaEmpty => "META_EMPTY", Self::TexmapNextType0 => "TEXMAP_NEXT_TYPE0", Self::ColourSlotsLimit => "COLOUR_SLOTS_LIMIT", @@ -359,13 +362,6 @@ impl ByteBank { .expect("buffer was just added") } - pub(crate) fn len(&self) -> usize { - self.buffers.len() - } - - pub(crate) fn get(&self, index: usize) -> Option<&[u8]> { - self.buffers.get(index).map(Vec::as_slice) - } } impl Default for ByteBank { diff --git a/crates/polymodel-ldraw-core/src/util.rs b/crates/polymodel-ldraw-core/src/util.rs index 0c5b05f..593bd74 100644 --- a/crates/polymodel-ldraw-core/src/util.rs +++ b/crates/polymodel-ldraw-core/src/util.rs @@ -1,20 +1,3 @@ -pub(crate) fn fmt_num(value: f64) -> String { - if !value.is_finite() { - return "0".into(); - } - if value == 0.0 { - return "0".into(); - } - let mut s = format!("{value:.6}"); - while s.contains('.') && s.ends_with('0') { - s.pop(); - } - if s.ends_with('.') { - s.pop(); - } - s -} - pub(crate) fn hex(bytes: &[u8]) -> String { bytes.iter().map(|b| format!("{b:02x}")).collect() } -- 2.51.2