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 <[email protected]>
Co-authored-by: Claude Opus 5.5 <[email protected]>

Cargo.lock

@@ -466,6 +466,7 @@ dependencies = [
 name = "modelcraft-geometry"
 version = "0.1.0"
 dependencies = [
+ "modelcraft-model",
  "quick-xml",
  "solidcraft-doc",
  "solidcraft-geom",

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<Value> {
         .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<Value> = 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);

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<Vec<u8>> {
-    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)
-}

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 },

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 }

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<Model> {
 
 /// 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<TaggedSurface> {
+    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<Tagge
             return bad(format!("object {id}: a placed vertex is out of range"));
         }
     }
-    if det(xf) < 0.0 {
+    if d < 0.0 {
         o.triangles.iter_mut().for_each(|t| t.swap(1, 2));
     }
     if o.tags.is_empty() {
@@ -335,6 +344,13 @@ fn surface(mut o: Object, k: usize, id: u32, xf: &Xf, unit: f64) -> Result<Tagge
     Ok(TaggedSurface { name, mesh: SurfaceMesh { positions: o.positions, triangles: o.triangles, tri_face: o.tags }, faces })
 }
 
+/// [`read_3mf`] of a file: a regular file of at most [`MAX_FILE_BYTES`] (SolidCraft's virtual
+/// file system, so the browser's in-memory files work too).
+pub fn read_3mf_file(path: &str) -> Result<Vec<TaggedSurface>> {
+    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<Vec<TaggedSurface>> {
     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("<vertex x=\"1\"", "<vertex x=\"2e9\""),
             tet("").replace("millimeter", "parsec"),
             format!(
                 "{HEAD}<object id=\"1\"><components><component objectid=\"1\"/></components></object></resources><build><item objectid=\"1\"/></build></model>"

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<n>` 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