From d0286a745a78da25d57ee65f3ba59b2e381bbbbb Mon Sep 17 00:00:00 2001 From: KenM76 Date: Tue, 3 Nov 2026 22:56:09 -0500 Subject: [PATCH] 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]