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]