diff --git a/Cargo.lock b/Cargo.lock index 74ca9d6bd9..d266227af1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -549,6 +549,7 @@ dependencies = [ "nalgebra", "notify", "num-traits", + "parking_lot 0.12.0", "parry2d-f64", "parry3d-f64", "spade", @@ -1390,7 +1391,17 @@ checksum = "7d17b78036a60663b797adeaee46f5c9dfebb86948d1255007a1d6be0271ff99" dependencies = [ "instant", "lock_api", - "parking_lot_core", + "parking_lot_core 0.8.5", +] + +[[package]] +name = "parking_lot" +version = "0.12.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "87f5ec2493a61ac0506c0f4199f99070cbe83857b0337006a30f3e6719b8ef58" +dependencies = [ + "lock_api", + "parking_lot_core 0.9.1", ] [[package]] @@ -1407,6 +1418,19 @@ dependencies = [ "winapi", ] +[[package]] +name = "parking_lot_core" +version = "0.9.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "28141e0cc4143da2443301914478dc976a61ffdb3f043058310c70df2fed8954" +dependencies = [ + "cfg-if 1.0.0", + "libc", + "redox_syscall", + "smallvec", + "windows-sys", +] + [[package]] name = "parry2d-f64" version = "0.8.0" @@ -2185,7 +2209,7 @@ dependencies = [ "js-sys", "log", "naga", - "parking_lot", + "parking_lot 0.11.2", "raw-window-handle", "smallvec", "wasm-bindgen", @@ -2210,7 +2234,7 @@ dependencies = [ "fxhash", "log", "naga", - "parking_lot", + "parking_lot 0.11.2", "profiling", "raw-window-handle", "smallvec", @@ -2245,7 +2269,7 @@ dependencies = [ "metal", "naga", "objc", - "parking_lot", + "parking_lot 0.11.2", "profiling", "range-alloc", "raw-window-handle", @@ -2319,6 +2343,49 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" +[[package]] +name = "windows-sys" +version = "0.32.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3df6e476185f92a12c072be4a189a0210dcdcf512a1891d6dff9edb874deadc6" +dependencies = [ + "windows_aarch64_msvc", + "windows_i686_gnu", + "windows_i686_msvc", + "windows_x86_64_gnu", + "windows_x86_64_msvc", +] + +[[package]] +name = "windows_aarch64_msvc" +version = "0.32.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d8e92753b1c443191654ec532f14c199742964a061be25d77d7a96f09db20bf5" + +[[package]] +name = "windows_i686_gnu" +version = "0.32.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6a711c68811799e017b6038e0922cb27a5e2f43a2ddb609fe0b6f3eeda9de615" + +[[package]] +name = "windows_i686_msvc" +version = "0.32.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "146c11bb1a02615db74680b32a68e2d61f553cc24c4eb5b4ca10311740e44172" + +[[package]] +name = "windows_x86_64_gnu" +version = "0.32.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c912b12f7454c6620635bbff3450962753834be2a594819bd5e945af18ec64bc" + +[[package]] +name = "windows_x86_64_msvc" +version = "0.32.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "504a2476202769977a040c6364301a3f65d0cc9e3fb08600b2bda150a0488316" + [[package]] name = "winit" version = "0.26.1" @@ -2340,7 +2407,7 @@ dependencies = [ "ndk-glue", "ndk-sys", "objc", - "parking_lot", + "parking_lot 0.11.2", "percent-encoding", "raw-window-handle", "smithay-client-toolkit", diff --git a/Cargo.toml b/Cargo.toml index b921fe3f98..e4e7c81bd9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,6 +22,7 @@ map-macro = "0.2.0" nalgebra = "0.30.0" notify = "5.0.0-pre.14" num-traits = "0.2.14" +parking_lot = "0.12.0" parry2d-f64 = "0.8.0" parry3d-f64 = "0.8.0" spade = "2.0.0" diff --git a/src/kernel/algorithms/mod.rs b/src/kernel/algorithms/mod.rs index c3f1c1fc3d..77c3932679 100644 --- a/src/kernel/algorithms/mod.rs +++ b/src/kernel/algorithms/mod.rs @@ -1,4 +1,3 @@ pub mod approximation; pub mod sweep; -pub mod transform; pub mod triangulation; diff --git a/src/kernel/algorithms/sweep.rs b/src/kernel/algorithms/sweep.rs index 9c6d48fc15..d16ca8f408 100644 --- a/src/kernel/algorithms/sweep.rs +++ b/src/kernel/algorithms/sweep.rs @@ -186,7 +186,7 @@ mod tests { let mut contains_top_face = false; for face in swept.topology().faces() { - if matches!(face.get(), Face::Face { .. }) { + if matches!(&*face.get(), Face::Face { .. }) { if face.get().clone() == bottom_face { contains_bottom_face = true; } diff --git a/src/kernel/algorithms/transform.rs b/src/kernel/algorithms/transform.rs deleted file mode 100644 index 57e5a03a44..0000000000 --- a/src/kernel/algorithms/transform.rs +++ /dev/null @@ -1,100 +0,0 @@ -use crate::{ - kernel::{ - shape::Shape, - topology::{ - edges::{Cycle, Edge}, - faces::Face, - vertices::Vertex, - }, - }, - math::Transform, -}; - -/// Create a new shape that is a transformed version of an existing one -/// -/// # Implementation note -/// -/// This code isn't really correct, only transforming the faces of the original -/// shape and not taking care of anything else, but this is more a reflection of -/// the state of `Shape`, with its redundant data. -/// -/// Addressing the shortcomings in this method probably doesn't make sense, -/// except as a side effect of addressing the shortcomings of `Shape`. -pub fn transform_shape(mut original: Shape, transform: &Transform) -> Shape { - let mut transformed = Shape::new(); - - for face in original.topology().faces() { - let face = match face.get().clone() { - Face::Face { - cycles, - surface, - color, - } => { - let mut cycles_trans = Vec::new(); - - for cycle in cycles { - let mut edges = Vec::new(); - - for edge in &cycle.get().edges { - let edge = edge.get(); - - let curve = transformed - .geometry() - .add_curve(edge.curve().transform(transform)); - - let vertices = - edge.vertices().clone().map(|vertices| { - vertices.map(|vertex| { - let point = - transformed.geometry().add_point( - transform.transform_point( - &vertex.point(), - ), - ); - - transformed - .topology() - .add_vertex(Vertex { point }) - .unwrap() - }) - }); - - let edge = Edge { curve, vertices }; - let edge = - transformed.topology().add_edge(edge).unwrap(); - - edges.push(edge); - } - - cycles_trans.push( - transformed - .topology() - .add_cycle(Cycle { edges }) - .unwrap(), - ); - } - - let surface = transformed - .geometry() - .add_surface(surface.get().transform(transform)); - - Face::Face { - cycles: cycles_trans, - surface, - color, - } - } - Face::Triangles(mut triangles) => { - for triangle in &mut triangles { - *triangle = transform.transform_triangle(triangle); - } - - Face::Triangles(triangles) - } - }; - - transformed.topology().add_face(face).unwrap(); - } - - transformed -} diff --git a/src/kernel/shape/geometry.rs b/src/kernel/shape/geometry.rs index 8d99fa9a89..9b8119157c 100644 --- a/src/kernel/shape/geometry.rs +++ b/src/kernel/shape/geometry.rs @@ -1,11 +1,14 @@ use crate::{ - kernel::geometry::{Curve, Surface}, - math::Point, + kernel::{ + geometry::{Curve, Surface}, + topology::faces::Face, + }, + math::{Point, Transform}, }; use super::{ handle::{Handle, Storage}, - Curves, Iter, Points, Surfaces, + Curves, Faces, Iter, Points, Surfaces, }; /// API to access a shape's geometry @@ -27,6 +30,15 @@ pub struct Geometry<'r> { pub(super) points: &'r mut Points, pub(super) curves: &'r mut Curves, pub(super) surfaces: &'r mut Surfaces, + + // This is needed here for a weird workaround, which in turn is necessary + // because triangle representation still exists. Once triangle + // representation is no longer a thing, this field can be moved to + // `Topology`, where it belongs. + // + // This issue has some context on triangle representation: + // https://github.com/hannobraun/Fornjot/issues/97 + pub(super) faces: &'r mut Faces, } impl Geometry<'_> { @@ -60,6 +72,45 @@ impl Geometry<'_> { handle } + /// Transform the geometry of the shape + /// + /// Since the topological types refer to geometry, and don't contain any + /// geometry themselves, this transforms the whole shape. + pub fn transform(&mut self, transform: &Transform) { + for point in self.points.iter_mut() { + let trans = { + let point = point.get(); + transform.transform_point(&point) + }; + *point.get_mut() = trans; + } + for curve in self.curves.iter_mut() { + let trans = { + let curve = curve.get(); + curve.transform(transform) + }; + *curve.get_mut() = trans; + } + for surface in self.surfaces.iter_mut() { + let trans = { + let surface = surface.get(); + surface.transform(transform) + }; + *surface.get_mut() = trans; + } + + // While some faces use triangle representation, we need this weird + // workaround here. + for face in self.faces.iter_mut() { + use std::ops::DerefMut as _; + if let Face::Triangles(triangles) = face.get_mut().deref_mut() { + for triangle in triangles { + *triangle = transform.transform_triangle(triangle); + } + } + } + } + /// Access an iterator over all points /// /// The caller must not make any assumptions about the order of points. diff --git a/src/kernel/shape/handle.rs b/src/kernel/shape/handle.rs index 04ecfb02aa..952b3595a8 100644 --- a/src/kernel/shape/handle.rs +++ b/src/kernel/shape/handle.rs @@ -5,6 +5,8 @@ use std::{ sync::Arc, }; +use parking_lot::{RwLock, RwLockReadGuard, RwLockWriteGuard}; + /// A handle to an object stored within [`Shape`] /// /// If an object of type `T` (this could be `Curve`, `Vertex`, etc.) is added to @@ -29,17 +31,19 @@ use std::{ /// Two [`Handle`]s are considered equal, if they refer to objects in the same /// memory location. #[derive(Clone, Eq, PartialEq, Hash, Ord, PartialOrd)] -pub struct Handle(Storage); +pub struct Handle { + storage: Storage, +} impl Handle { /// Access the object that the handle references - pub fn get(&self) -> &T { - self.0.deref() + pub fn get(&self) -> Ref { + self.storage.get() } /// Internal method to access the [`Storage`] this handle refers to pub(super) fn storage(&self) -> &Storage { - &self.0 + &self.storage } } @@ -48,35 +52,48 @@ where T: fmt::Debug, { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{:?}: {:?}", self.0.ptr(), self.get()) + write!(f, "{:?}: {:?}", self.storage.ptr(), &*self.get()) + } +} + +/// Returned by [`Handle::get`] +pub struct Ref<'r, T>(RwLockReadGuard<'r, T>); + +impl Deref for Ref<'_, T> { + type Target = T; + + fn deref(&self) -> &Self::Target { + self.0.deref() } } /// Internal type used in collections within [`Shape`] -#[derive(Debug, Eq, Ord, PartialOrd)] -pub struct Storage(Arc); +#[derive(Debug)] +pub struct Storage(Arc>); impl Storage { /// Create a [`Storage`] instance that wraps the provided object pub(super) fn new(value: T) -> Self { - Self(Arc::new(value)) + Self(Arc::new(RwLock::new(value))) } /// Create a handle that refers to this [`Storage`] instance pub(super) fn handle(&self) -> Handle { - Handle(self.clone()) + Handle { + storage: self.clone(), + } } - fn ptr(&self) -> *const T { - Arc::as_ptr(&self.0) + pub(super) fn get(&self) -> Ref { + Ref(self.0.read()) } -} -impl Deref for Storage { - type Target = T; + pub(super) fn get_mut(&self) -> RwLockWriteGuard { + self.0.write() + } - fn deref(&self) -> &Self::Target { - self.0.deref() + fn ptr(&self) -> *const () { + Arc::as_ptr(&self.0) as _ } } @@ -95,6 +112,20 @@ impl PartialEq for Storage { } } +impl Eq for Storage {} + +impl Ord for Storage { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + self.ptr().cmp(&other.ptr()) + } +} + +impl PartialOrd for Storage { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + impl Hash for Storage { fn hash(&self, state: &mut H) { self.ptr().hash(state); diff --git a/src/kernel/shape/mod.rs b/src/kernel/shape/mod.rs index 092c2bddd4..84b6f3ea93 100644 --- a/src/kernel/shape/mod.rs +++ b/src/kernel/shape/mod.rs @@ -84,6 +84,8 @@ impl Shape { points: &mut self.points, curves: &mut self.curves, surfaces: &mut self.surfaces, + + faces: &mut self.faces, } } @@ -96,12 +98,13 @@ impl Shape { points: &mut self.points, curves: &mut self.curves, surfaces: &mut self.surfaces, + + faces: &mut self.faces, }, vertices: &mut self.vertices, edges: &mut self.edges, cycles: &mut self.cycles, - faces: &mut self.faces, } } } diff --git a/src/kernel/shape/topology.rs b/src/kernel/shape/topology.rs index 0bf3476d0e..27e4b7b629 100644 --- a/src/kernel/shape/topology.rs +++ b/src/kernel/shape/topology.rs @@ -17,8 +17,7 @@ use crate::{ use super::{ handle::{Handle, Storage}, - Cycles, Edges, Faces, Geometry, Iter, ValidationError, ValidationResult, - Vertices, + Cycles, Edges, Geometry, Iter, ValidationError, ValidationResult, Vertices, }; /// The vertices of a shape @@ -30,7 +29,6 @@ pub struct Topology<'r> { pub(super) vertices: &'r mut Vertices, pub(super) edges: &'r mut Edges, pub(super) cycles: &'r mut Cycles, - pub(super) faces: &'r mut Faces, } impl Topology<'_> { @@ -61,7 +59,8 @@ impl Topology<'_> { return Err(ValidationError::Structural(())); } for existing in &*self.vertices { - let distance = (existing.point() - vertex.point()).magnitude(); + let distance = + (existing.get().point() - vertex.point()).magnitude(); if distance < self.min_distance { warn!( @@ -217,7 +216,7 @@ impl Topology<'_> { let storage = Storage::new(face); let handle = storage.handle(); - self.faces.push(storage); + self.geometry.faces.push(storage); Ok(handle) } @@ -247,7 +246,7 @@ impl Topology<'_> { /// /// The caller must not make any assumptions about the order of faces. pub fn faces(&self) -> Iter { - Iter::new(self.faces) + Iter::new(self.geometry.faces) } pub fn triangles( @@ -256,8 +255,8 @@ impl Topology<'_> { out: &mut Vec>, debug_info: &mut DebugInfo, ) { - for face in &*self.faces { - face.triangles(tolerance, out, debug_info); + for face in &*self.geometry.faces { + face.get().triangles(tolerance, out, debug_info); } } } diff --git a/src/kernel/shapes/transform.rs b/src/kernel/shapes/transform.rs index 3c3513f499..966f2a042b 100644 --- a/src/kernel/shapes/transform.rs +++ b/src/kernel/shapes/transform.rs @@ -2,7 +2,7 @@ use parry3d_f64::math::Isometry; use crate::{ debug::DebugInfo, - kernel::{algorithms::transform::transform_shape, shape::Shape}, + kernel::shape::Shape, math::{Aabb, Scalar, Transform}, }; @@ -10,9 +10,12 @@ use super::ToShape; impl ToShape for fj::Transform { fn to_shape(&self, tolerance: Scalar, debug_info: &mut DebugInfo) -> Shape { - let shape = self.shape.to_shape(tolerance, debug_info); + let mut shape = self.shape.to_shape(tolerance, debug_info); let transform = transform(self); - transform_shape(shape, &transform) + + shape.geometry().transform(&transform); + + shape } fn bounding_volume(&self) -> Aabb<3> { diff --git a/src/kernel/shapes/union.rs b/src/kernel/shapes/union.rs index 8f0177c260..0bcdcb2b27 100644 --- a/src/kernel/shapes/union.rs +++ b/src/kernel/shapes/union.rs @@ -100,7 +100,7 @@ fn copy_shape(mut orig: Shape, target: &mut Shape) { } for face_orig in orig.topology().faces() { - match face_orig.get() { + match &*face_orig.get() { Face::Face { surface, cycles: cs,