From d0286a745a78da25d57ee65f3ba59b2e381bbbbb Mon Sep 17 00:00:00 2001 From: KenM76 Date: Tue, 3 Nov 2026 22:56:09 -0500 Subject: [PATCH 1/2] M12: PRC reader: winding by the whole determinant; walk charges each schema once A negative w (uniform or projective) turns mapped triangles inside out though the 3x3 block does not mirror; the flip and the normal sign now follow the 4x4 determinant, falling back to the 3x3 block when the last row is zero. The tree walk read each structure's globals schema with its own full allowance, then again for the globals. It now reads it once, charged to the walk, and continues from there for the globals. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SqmHYx8FiWM5PcsoiErgp9 --- crates/prc/src/bits.rs | 7 ++++ crates/prc/src/geometry.rs | 62 +++++++++++++++++++++++++++++++++--- crates/prc/src/tree/mod.rs | 17 +++++----- crates/prc/src/tree/tests.rs | 21 ++++++++++++ 4 files changed, 93 insertions(+), 14 deletions(-) diff --git a/crates/prc/src/bits.rs b/crates/prc/src/bits.rs index 973e092efd..837d281f3b 100644 --- a/crates/prc/src/bits.rs +++ b/crates/prc/src/bits.rs @@ -56,6 +56,13 @@ impl<'a> BitReader<'a> { Self::with_budget(data, Budget { elements, ..Budget::FULL }) } + /// This reader, at its position, allowed `budget` instead: what readers of the same + /// stream left since it stopped. + pub(crate) fn with_remaining(mut self, budget: Budget) -> Self { + self.budget = budget; + self + } + /// What this reader may still spend. pub(crate) fn budget(&self) -> Budget { self.budget diff --git a/crates/prc/src/geometry.rs b/crates/prc/src/geometry.rs index 9291f3e4ca..baa48331d5 100644 --- a/crates/prc/src/geometry.rs +++ b/crates/prc/src/geometry.rs @@ -63,6 +63,28 @@ impl Matrix { + a(0, 2) * (a(1, 0) * a(2, 1) - a(1, 1) * a(2, 0)) } + /// The determinant of the whole matrix, by expansion along the last row. + fn det4(&self) -> f64 { + (0..4) + .map(|j| { + let cols: Vec = (0..4).filter(|&c| c != j).collect(); + let a = |i: usize, k: usize| cols.get(k).map_or(0.0, |&c| self.at(i, c)); + let minor = a(0, 0) * (a(1, 1) * a(2, 2) - a(1, 2) * a(2, 1)) - a(0, 1) * (a(1, 0) * a(2, 2) - a(1, 2) * a(2, 0)) + + a(0, 2) * (a(1, 0) * a(2, 1) - a(1, 1) * a(2, 0)); + let sign = if (3 + j) % 2 == 0 { 1.0 } else { -1.0 }; + sign * self.at(3, j) * minor + }) + .sum() + } + + /// Whether mapping points through this matrix turns them inside out. Where the point is + /// divided by `w`, the Jacobian's sign is the sign of the 4×4 determinant; a last row of + /// zeros (no division) falls back to the 3×3 block. + fn mirrors(&self) -> bool { + let det = self.det4(); + if det == 0.0 { self.det3() < 0.0 } else { det < 0.0 } + } + /// Cofactor `(i, j)` of the upper-left 3×3 block (`i`, `j` < 3). fn cofactor(&self, i: usize, j: usize) -> f64 { let (r0, r1) = ((i + 1) % 3, (i + 2) % 3); @@ -73,8 +95,9 @@ impl Matrix { impl TriangleMesh { /// This mesh with every position mapped through `m` and every stored normal through its - /// inverse transpose (renormalised); a mirroring `m` (negative determinant) reverses each - /// triangle so the outside stays counter-clockwise. A projective `m` (a last row other than + /// inverse transpose (renormalised); a mirroring `m` (negative 4×4 determinant, which a + /// negative `w` can cause too) reverses each triangle so the outside stays + /// counter-clockwise. A projective `m` (a last row other than /// `[0, 0, 0, w]`) bends the surface per point, so the stored normals are dropped. /// /// ``` @@ -96,8 +119,8 @@ impl TriangleMesh { } // The cofactor matrix is det times the inverse transpose, so its sign is corrected // and the result renormalised. - let det = m.det3(); - let sign = if det < 0.0 { -1.0 } else { 1.0 }; + let mirrors = m.mirrors(); + let sign = if mirrors { -1.0 } else { 1.0 }; if m.at(3, 0) != 0.0 || m.at(3, 1) != 0.0 || m.at(3, 2) != 0.0 { out.normals.clear(); out.triangle_normals.clear(); @@ -108,7 +131,7 @@ impl TriangleMesh { let len = v.iter().map(|c| c * c).sum::().sqrt(); *n = if len > 0.0 && len.is_finite() { v.map(|c| c / len) } else { v }; } - if det < 0.0 { + if mirrors { for t in &mut out.triangles { t.swap(1, 2); } @@ -195,6 +218,35 @@ mod tests { assert_eq!(mesh.transformed(&uniform).triangle_normals, [[0, 0, 0]]); } + /// A negative `w` turns the mapped triangles inside out though the 3×3 block does not + /// mirror, so the winding is reversed by the sign of the whole determinant. + #[test] + fn a_negative_w_reverses_the_winding() { + let mesh = TriangleMesh { + positions: vec![[1.0, 0.0, 0.0], [0.0, 1.0, 0.0], [0.0, 0.0, 1.0]], + triangles: vec![[0, 1, 2]], + normals: vec![[1.0, 0.0, 0.0]], + triangle_normals: vec![[0, 0, 0]], + ..TriangleMesh::default() + }; + let mut negated = Matrix::IDENTITY; + negated.0[3][3] = -1.0; + let placed = mesh.transformed(&negated); + assert_eq!(placed.positions, [[-1.0, 0.0, 0.0], [0.0, -1.0, 0.0], [0.0, 0.0, -1.0]]); + assert_eq!(placed.triangles, [[0, 2, 1]]); + assert_eq!(placed.normals, [[-1.0, 0.0, 0.0]]); + // A projective w that is negative over the mesh: w = z - 3. + let mut projective = Matrix::IDENTITY; + projective.0[3][2] = 1.0; + projective.0[3][3] = -3.0; + assert_eq!(mesh.transformed(&projective).triangles, [[0, 2, 1]]); + // A positive w, uniform or projective, keeps the winding. + let mut positive = Matrix::IDENTITY; + positive.0[3][2] = 1.0; + positive.0[3][3] = 3.0; + assert_eq!(mesh.transformed(&positive).triangles, [[0, 1, 2]]); + } + impl Matrix { fn transformed_normal(&self, mesh: &TriangleMesh) -> [f64; 3] { mesh.transformed(self).normals.first().copied().unwrap_or_default() diff --git a/crates/prc/src/tree/mod.rs b/crates/prc/src/tree/mod.rs index 786348a0e8..3ed9d8eb20 100644 --- a/crates/prc/src/tree/mod.rs +++ b/crates/prc/src/tree/mod.rs @@ -283,20 +283,19 @@ impl PrcFile { let mut left = budget; let mut trees = Vec::with_capacity(self.file_structures.len()); for fs in &self.file_structures { - let schema = fs.schema()?; + // The globals open with the schema every reader of this structure uses: read it + // once, charged to the walk, and go on from there for the globals themselves. + let mut globals_reader = BitReader::with_budget(fs.section(SectionKind::Globals), left); + let schema = Schema::read(&mut globals_reader)?; + left = globals_reader.budget(); let mut ctx = Ctx::new(BitReader::with_budget(fs.section(SectionKind::Tree), left), &schema, fs.authoring_version); let tree = ctx.file_structure_tree()?; left = ctx.r.budget(); // The globals carry the coordinate systems and the colours; colours alone are not // worth refusing the placements over. - let globals = (|| { - let mut r = BitReader::with_budget(fs.section(SectionKind::Globals), left); - Schema::read(&mut r)?; - let mut ctx = Ctx::new(r, &schema, fs.authoring_version); - let globals = ctx.globals(); - left = ctx.r.budget(); - globals - })(); + let mut ctx = Ctx::new(globals_reader.with_remaining(left), &schema, fs.authoring_version); + let globals = ctx.globals(); + left = ctx.r.budget(); let globals = match globals { Ok(g) => g, Err(e) if places_by_system(&tree) => return Err(e), diff --git a/crates/prc/src/tree/tests.rs b/crates/prc/src/tree/tests.rs index 905c0278c5..a2264588bd 100644 --- a/crates/prc/src/tree/tests.rs +++ b/crates/prc/src/tree/tests.rs @@ -453,6 +453,27 @@ fn one_step_allowance_covers_the_whole_walk() { assert_eq!(two.placements().map(|p| p.len()), Ok(2)); } +/// Each structure's schema is read once, charged to the walk's allowance: a schema too large +/// for what is left is an error, not decoded outside the allowance and then skipped. +#[test] +fn the_globals_schema_is_charged_to_the_walk() { + let mut schema = W::default(); + // 200 empty entries cost 200 elements. + schema.uint(200); + for t in 0..200 { + schema.uint(50_000 + t).uint(0); + } + let fs = Fs { + globals: globals_after(schema, &[RED], &[], &[], &[]), + tree: tree(&[plate(S_RED)], &[Occ { part: 1, ..Occ::default() }]), + tess: two_face_square(), + }; + let f = PrcFile::parse(&prc(&[fs], &model(&[(0, 1)]))).unwrap(); + let walk = |n| f.walk_within(StyleAlpha::default(), EntityOverrides::default(), Budget { elements: n, ..Budget::FULL }); + assert!(matches!(walk(100), Err(PrcError::TooManyElements { .. }))); + assert_eq!(walk(1000).map(|w| w.0.len()), Ok(1)); +} + /// Globals that cannot be read are reported, not replaced by empty ones, when an entity /// reference names one of their coordinate systems. #[test] From ca21d4fb7bb763e3dc49d6d9b8f05b5de7a8f8e7 Mon Sep 17 00:00:00 2001 From: KenM76 Date: Tue, 3 Nov 2026 23:06:54 -0500 Subject: [PATCH 2/2] M12: prc-render: scale a triangle's edges before their cross product normalize already scales by the largest component, but the face normal was the normalized cross product of two edges, and edges near 1e160 overflow that product to infinity first, so the triangle was dropped. unit_cross divides both edges by their largest component before the cross product. a_huge_square_is_drawn fails with the old line (white instead of [190,192,200]). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SqmHYx8FiWM5PcsoiErgp9 --- crates/prc-render/src/render.rs | 4 ++-- crates/prc-render/src/render/tests.rs | 8 ++++++++ crates/prc-render/src/vec3.rs | 19 +++++++++++++++++++ 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/crates/prc-render/src/render.rs b/crates/prc-render/src/render.rs index d9072a3055..3670878e17 100644 --- a/crates/prc-render/src/render.rs +++ b/crates/prc-render/src/render.rs @@ -5,7 +5,7 @@ use crate::WorldMesh; use crate::raster::{Corner, Target, View, ViewCorner, at}; -use crate::vec3::{cross, dot, length, normalize, scale, sub}; +use crate::vec3::{cross, dot, length, normalize, scale, sub, unit_cross}; use pdfcraft_prc::TriangleMesh; /// The largest image [`render`] draws, in pixels (width × height). @@ -318,7 +318,7 @@ impl Scene { let Some(world) = mesh.triangles.get(t).and_then(|tri| corners(mesh, tri)) else { return; }; - let Some(face) = normalize(cross(sub(at3(&world, 1), at3(&world, 0)), sub(at3(&world, 2), at3(&world, 0)))) else { + let Some(face) = unit_cross(sub(at3(&world, 1), at3(&world, 0)), sub(at3(&world, 2), at3(&world, 0))) else { return; }; let stored = stored_normals(mesh, t); diff --git a/crates/prc-render/src/render/tests.rs b/crates/prc-render/src/render/tests.rs index 8d0764315d..ec8104a260 100644 --- a/crates/prc-render/src/render/tests.rs +++ b/crates/prc-render/src/render/tests.rs @@ -268,3 +268,11 @@ fn unlit_draws_the_plain_colour_and_the_headlight_shades_by_angle() { assert_eq!(lit(Lighting::Headlight, 1.0), [151, 153, 159, 255]); assert_eq!(lit(Lighting::Unlit, 1.0), [190, 192, 200, 255]); } + +#[test] +fn a_huge_square_is_drawn() { + // Its edges' cross product (about 1e320) overflows unless they are scaled first. + let image = render(&[quad(0.0, 1e160, 0.0)], &ortho(4e160), &small()).unwrap(); + assert_eq!(pixel(&image, 20, 20), [190, 192, 200, 255]); + assert_eq!(pixel(&image, 1, 1), [255, 255, 255, 255]); +} diff --git a/crates/prc-render/src/vec3.rs b/crates/prc-render/src/vec3.rs index 8ebf844087..7a673e0055 100644 --- a/crates/prc-render/src/vec3.rs +++ b/crates/prc-render/src/vec3.rs @@ -40,6 +40,17 @@ pub(crate) fn normalize(a: Vec3) -> Option { (l.is_finite() && l > 0.0).then(|| scale(a, 1.0 / l)) } +/// The unit normal of the plane spanned by `a` and `b` (`a × b`, normalized); `None` when they are +/// parallel, zero or not finite. Both are first divided by their largest component, so the cross +/// product of two huge edges cannot overflow before it is normalized. +pub(crate) fn unit_cross(a: Vec3, b: Vec3) -> Option { + let big = a.iter().chain(b.iter()).fold(0.0_f64, |m, c| m.max(c.abs())); + if !(big.is_finite() && big > 0.0) { + return None; + } + normalize(cross(a.map(|c| c / big), b.map(|c| c / big))) +} + #[cfg(test)] mod tests { use super::*; @@ -63,4 +74,12 @@ mod tests { assert_eq!(normalize([1e200, 0.0, 0.0]), Some([1.0, 0.0, 0.0])); assert_eq!(normalize([0.0, -3e-320, 0.0]), Some([0.0, -1.0, 0.0])); } + + #[test] + fn unit_cross_survives_edges_whose_product_overflows() { + assert_eq!(unit_cross([1e160, 0.0, 0.0], [0.0, 1e160, 0.0]), Some([0.0, 0.0, 1.0])); + assert_eq!(unit_cross([1e-170, 0.0, 0.0], [0.0, 1e-170, 0.0]), Some([0.0, 0.0, 1.0])); + assert_eq!(unit_cross([2.0, 0.0, 0.0], [4.0, 0.0, 0.0]), None); + assert_eq!(unit_cross([f64::INFINITY, 0.0, 0.0], [0.0, 1.0, 0.0]), None); + } }