From 2c1821d0ad1523b03bb6c9762eb34467adf26c0e Mon Sep 17 00:00:00 2001 From: Irakli Kourtis Date: Tue, 13 Oct 2026 14:52:53 -0400 Subject: [PATCH] geometry: read tagged meshes through the VFS, refuse singular transforms, share the model's coordinate limit (#6) read_3mf_file reads through solidcraft_io::vfs like open_design_file (regular files only, browser files too), so the engine drops its own capped reader and EngineError::File. A build item whose transform flattens its object is an error, and coordinates use modelcraft_model::MAX_COORD (1e9) instead of a looser limit of their own. Claude-Session: https://claude.ai/code/session_01NmAttHhUZTokgPeEJia8sm Co-authored-by: jyviko <2501648+jyviko@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 --- Cargo.lock | 1 + crates/engine/src/cmd/geometry.rs | 15 ++++++++++---- crates/engine/src/cmd/mod.rs | 14 ------------- crates/engine/src/lib.rs | 3 --- crates/geometry/Cargo.toml | 1 + crates/geometry/src/tagged_mesh.rs | 32 ++++++++++++++++++++++++++---- docs/formats/tagged-mesh.md | 5 +++-- 7 files changed, 44 insertions(+), 27 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5316062..3100976 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -466,6 +466,7 @@ dependencies = [ name = "modelcraft-geometry" version = "0.1.0" dependencies = [ + "modelcraft-model", "quick-xml", "solidcraft-doc", "solidcraft-geom", diff --git a/crates/engine/src/cmd/geometry.rs b/crates/engine/src/cmd/geometry.rs index 345a38f..81d4224 100644 --- a/crates/engine/src/cmd/geometry.rs +++ b/crates/engine/src/cmd/geometry.rs @@ -1,10 +1,9 @@ use std::sync::Arc; -use modelcraft_geometry::tagged_mesh::{self, MAX_FILE_BYTES}; +use modelcraft_geometry::tagged_mesh; use modelcraft_geometry::{Shape, open_design_file}; use serde_json::{Value, json}; -use super::read_capped; use crate::{CommandSpec, EngineError, Result, Session}; pub const OPEN: CommandSpec = CommandSpec { @@ -93,7 +92,7 @@ fn import_mesh(s: &mut Session, p: &Value) -> Result { .and_then(Value::as_str) .filter(|x| !x.trim().is_empty() && x.len() < 4096) .ok_or_else(|| EngineError::Params("`path` must name a .3mf file".into()))?; - let surfaces = tagged_mesh::read_3mf(&read_capped(path, MAX_FILE_BYTES)?)?; + let surfaces = tagged_mesh::read_3mf_file(path)?; let first = s.surfaces.len(); let added: Vec = surfaces .iter() @@ -139,7 +138,15 @@ mod tests { assert_eq!(s.surfaces[0].mesh.tri_face, vec![0, 1, 1, 1]); assert_eq!(s.journal.len(), 1); - for p in [json!({"path": junk.to_str().unwrap()}), json!({"path": dir.join("missing.3mf").to_str().unwrap()}), json!({"path": ""})] { + let hostile = [ + json!({"path": junk.to_str().unwrap()}), + json!({"path": dir.join("missing.3mf").to_str().unwrap()}), + json!({"path": dir.to_str().unwrap()}), + json!({"path": "/dev/zero"}), + json!({"path": ""}), + json!({"path": "x".repeat(5000)}), + ]; + for p in hostile { assert!(s.execute("geometry.import.mesh", &p).is_err(), "{p}"); } assert_eq!(s.surfaces.len(), 1); diff --git a/crates/engine/src/cmd/mod.rs b/crates/engine/src/cmd/mod.rs index 3b3e142..6f4300f 100644 --- a/crates/engine/src/cmd/mod.rs +++ b/crates/engine/src/cmd/mod.rs @@ -38,17 +38,3 @@ pub(crate) fn xyz(p: &Value, key: &str) -> Result<[f64; 3]> { } Ok(out) } - -/// A file's bytes, refusing anything over `cap`. -pub(crate) fn read_capped(path: &str, cap: usize) -> Result> { - use std::io::Read; - let err = |e: std::io::Error| EngineError::File(format!("{path}: {e}")); - let f = std::fs::File::open(path).map_err(err)?; - let len = f.metadata().map(|m| m.len()).unwrap_or(0); - let mut bytes = Vec::with_capacity(usize::try_from(len).unwrap_or(0).min(cap)); - f.take(cap as u64 + 1).read_to_end(&mut bytes).map_err(err)?; - if bytes.len() > cap { - return Err(EngineError::File(format!("{path}: larger than {} MB", cap >> 20))); - } - Ok(bytes) -} diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 9c4e82a..9239335 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -26,9 +26,6 @@ pub enum EngineError { Model(#[from] modelcraft_model::Error), #[error(transparent)] Geometry(#[from] modelcraft_geometry::Error), - /// A file could not be read or written. - #[error("{0}")] - File(String), /// A command panicked; the model is unchanged. #[error("internal error in {command}: {message}")] Internal { command: String, message: String }, diff --git a/crates/geometry/Cargo.toml b/crates/geometry/Cargo.toml index 64cd73f..1cd01e0 100644 --- a/crates/geometry/Cargo.toml +++ b/crates/geometry/Cargo.toml @@ -9,6 +9,7 @@ license.workspace = true workspace = true [dependencies] +modelcraft-model = { workspace = true } solidcraft-geom = { workspace = true } solidcraft-kernel = { workspace = true } solidcraft-doc = { workspace = true } diff --git a/crates/geometry/src/tagged_mesh.rs b/crates/geometry/src/tagged_mesh.rs index c8d767e..e98dfe0 100644 --- a/crates/geometry/src/tagged_mesh.rs +++ b/crates/geometry/src/tagged_mesh.rs @@ -28,8 +28,12 @@ const MAX_ENTRIES: usize = 10_000; pub const MAX_ELEMENTS: usize = 20_000_000; const MAX_BUILD_ITEMS: usize = 10_000; const MAX_XML_DEPTH: usize = 256; -/// Largest coordinate magnitude accepted, after units and transforms (mm). -const MAX_COORD: f64 = 1e12; +/// Largest coordinate magnitude accepted, in the file and after units and transforms: the FE +/// model's own limit, so every imported vertex can become a node. +use modelcraft_model::MAX_COORD; +/// A transform whose determinant is below this fraction of its largest row length cubed +/// flattens the object (relative, so a uniform scale is never refused). +const MIN_RELATIVE_DET: f64 = 1e-12; const CORE_NS: &[u8] = b"http://schemas.microsoft.com/3dmanufacturing/core/2015/02"; /// The face-tag namespace, version 1. @@ -312,6 +316,11 @@ fn parse_model(xml: &[u8]) -> Result { /// Build item `k` as a surface: object `o` (checked) placed by `xf` and scaled to millimetres. fn surface(mut o: Object, k: usize, id: u32, xf: &Xf, unit: f64) -> Result { + let d = det(xf); + let scale = (0..3).map(|r| (xf[r * 3].powi(2) + xf[r * 3 + 1].powi(2) + xf[r * 3 + 2].powi(2)).sqrt()).fold(0.0, f64::max); + if !(d.abs() > MIN_RELATIVE_DET * scale.powi(3)) { + return bad(format!("build item {}: its transform is singular (it flattens object {id} to zero volume)", k + 1)); + } if o.components { return bad(format!("not supported yet: object {id} is made of components")); } @@ -324,7 +333,7 @@ fn surface(mut o: Object, k: usize, id: u32, xf: &Xf, unit: f64) -> Result Result Result> { + let bytes = solidcraft_io::vfs::read(path, MAX_FILE_BYTES).map_err(|e| Error::Invalid(format!("cannot read `{path}`: {e}")))?; + read_3mf(&bytes) +} + /// The surfaces of a 3MF package, one per build item, in millimetres. pub fn read_3mf(bytes: &[u8]) -> Result> { if bytes.len() > MAX_FILE_BYTES { @@ -476,6 +492,10 @@ mod tests { #[test] fn untagged_units_and_transforms() { let s = read_3mf(&package(&tet(""))).unwrap(); + // Uniform and uneven (but not flattening) scales are fine. + for xf in ["0.001 0 0 0 0.001 0 0 0 0.001 0 0 0", "1000 0 0 0 1 0 0 0 0.001 5 5 5"] { + assert!(read_3mf(&package(&tet(&format!(" transform=\"{xf}\"")))).is_ok(), "{xf}"); + } assert_eq!((s[0].faces, s[0].mesh.tri_face.clone()), (1, vec![0; 4])); let cm = tet(" transform=\"-1 0 0 0 1 0 0 0 1 0 0 2\"").replace("millimeter", "centimeter"); let s = read_3mf(&package(&cm)).unwrap(); @@ -540,7 +560,11 @@ mod tests { // transforms, units, components, duplicates tet(" transform=\"1 2\""), tet(" transform=\"1e300 0 0 0 1 0 0 0 1 0 0 0\""), - tet(" transform=\"1e12 0 0 0 1 0 0 0 1 0 0 0\"").replace("millimeter", "meter"), + tet(" transform=\"1e9 0 0 0 1 0 0 0 1 0 0 0\"").replace("millimeter", "meter"), + tet(" transform=\"0 0 0 0 0 0 0 0 0 0 0 0\""), + tet(" transform=\"1 0 0 0 1 0 0 0 1e-300 0 0 0\""), + tet(" transform=\"1 0 0 1 0 0 0 0 1 0 0 0\""), + tet("").replace("" diff --git a/docs/formats/tagged-mesh.md b/docs/formats/tagged-mesh.md index 700d061..ba94759 100644 --- a/docs/formats/tagged-mesh.md +++ b/docs/formats/tagged-mesh.md @@ -87,10 +87,11 @@ as one face (tag 0). A file mixing tagged and untagged triangles in one object i - Every build item becomes one `SurfaceMesh`, in millimetres (`unit` converted), named after its object (`Mesh` when unnamed). An item's `transform` is applied; a mirroring transform reverses - the winding. Components are not read yet (an error that says so). + the winding; a singular transform (one that flattens the object) is refused. Components are + not read yet (an error that says so). - Untrusted input: file ≤ 512 MB, a part ≤ 1 GB uncompressed, ≤ 10 000 zip entries, ≤ 20 million vertices and 20 million triangles in total, XML nesting ≤ 256. Coordinates must be finite with - magnitude ≤ 10¹²; vertex indices must exist; tags must be below `ft:faces` when it is given. + magnitude ≤ 10⁹ mm (the FE model's limit), in the file and once placed; vertex indices must exist; tags must be below `ft:faces` when it is given. Any violation is an error naming the problem; nothing panics. ## Versions