Merge pull request #2234 from hannobraun/presentation

Remove `color` attribute from `Region`
hannobraun · Feb 22, 2024 · ea8b2f7 · ea8b2f7
2 parents 00faa85 + 761b6d1
commit ea8b2f7
Show file tree

Hide file tree

Showing 19 changed files with 61 additions and 98 deletions.
diff --git a/crates/fj-core/src/objects/kinds/region.rs b/crates/fj-core/src/objects/kinds/region.rs
@@ -1,7 +1,5 @@
 //! A single, continues 2d region
 
-use fj_interop::Color;
-
 use crate::{
     objects::{Cycle, ObjectSet},
     storage::Handle,
@@ -19,20 +17,17 @@ use crate::{
 pub struct Region {
     exterior: Handle<Cycle>,
     interiors: ObjectSet<Cycle>,
-    color: Option<Color>,
 }
 
 impl Region {
     /// Construct an instance of `Region`
     pub fn new(
         exterior: Handle<Cycle>,
         interiors: impl IntoIterator<Item = Handle<Cycle>>,
-        color: Option<Color>,
     ) -> Self {
         Self {
             exterior,
             interiors: interiors.into_iter().collect(),
-            color,
         }
     }
 
@@ -54,9 +49,4 @@ impl Region {
         // for doing that here *and* in `interiors`.
         [self.exterior()].into_iter().chain(self.interiors())
     }
-
-    /// Access the color of the region
-    pub fn color(&self) -> Option<Color> {
-        self.color
-    }
 }
diff --git a/crates/fj-core/src/operations/build/face.rs b/crates/fj-core/src/operations/build/face.rs
@@ -22,7 +22,7 @@ pub trait BuildFace {
     /// Build a face with an empty exterior, no interiors, and no color
     fn unbound(surface: Handle<Surface>, core: &mut Core) -> Face {
         let exterior = Cycle::empty().insert(core);
-        let region = Region::new(exterior, [], None).insert(core);
+        let region = Region::new(exterior, []).insert(core);
         Face::new(surface, region)
     }
 

diff --git a/crates/fj-core/src/operations/build/region.rs b/crates/fj-core/src/operations/build/region.rs
@@ -16,9 +16,8 @@ pub trait BuildRegion {
     fn empty(core: &mut Core) -> Region {
         let exterior = Cycle::empty().insert(core);
         let interiors = [];
-        let color = None;
 
-        Region::new(exterior, interiors, color)
+        Region::new(exterior, interiors)
     }
 
     /// Build a circle
@@ -28,7 +27,7 @@ pub trait BuildRegion {
         core: &mut Core,
     ) -> Region {
         let exterior = Cycle::circle(center, radius, core).insert(core);
-        Region::new(exterior, [], None)
+        Region::new(exterior, [])
     }
 
     /// Build a polygon
@@ -39,7 +38,7 @@ pub trait BuildRegion {
         Ps::IntoIter: Clone + ExactSizeIterator,
     {
         let exterior = Cycle::polygon(points, core).insert(core);
-        Region::new(exterior, [], None)
+        Region::new(exterior, [])
     }
 }
 

diff --git a/crates/fj-core/src/operations/holes.rs b/crates/fj-core/src/operations/holes.rs
@@ -53,6 +53,7 @@ impl AddHole for Shell {
             )
             .sweep_region(
                 location.face.surface(),
+                None,
                 path,
                 &mut SweepCache::default(),
                 core,
@@ -118,6 +119,7 @@ impl AddHole for Shell {
             )
             .sweep_region(
                 entry_location.face.surface(),
+                None,
                 path,
                 &mut SweepCache::default(),
                 core,

diff --git a/crates/fj-core/src/operations/presentation.rs b/crates/fj-core/src/operations/presentation.rs
@@ -2,11 +2,7 @@
 
 use fj_interop::Color;
 
-use crate::{
-    objects::{IsObject, Region},
-    storage::Handle,
-    Core,
-};
+use crate::{objects::Region, storage::Handle, Core};
 
 /// Get the color of an object
 pub trait GetColor {
@@ -21,29 +17,15 @@ impl GetColor for Handle<Region> {
 }
 
 /// Set the color of an object
-pub trait SetColor: IsObject {
+pub trait SetColor {
     /// Set the color of the object
-    fn set_color(
-        &self,
-        color: impl Into<Color>,
-        core: &mut Core,
-    ) -> Self::BareObject;
+    fn set_color(&self, color: impl Into<Color>, core: &mut Core);
 }
 
 impl SetColor for Handle<Region> {
-    fn set_color(
-        &self,
-        color: impl Into<Color>,
-        core: &mut Core,
-    ) -> Self::BareObject {
-        let color = color.into();
-
-        core.layers.presentation.set_color(self.clone(), color);
-
-        Region::new(
-            self.exterior().clone(),
-            self.interiors().into_iter().cloned(),
-            Some(color),
-        )
+    fn set_color(&self, color: impl Into<Color>, core: &mut Core) {
+        core.layers
+            .presentation
+            .set_color(self.clone(), color.into());
     }
 }
diff --git a/crates/fj-core/src/operations/replace/curve.rs b/crates/fj-core/src/operations/replace/curve.rs
@@ -117,7 +117,6 @@ impl ReplaceCurve for Region {
                     })
                     .into_inner(),
                 interiors,
-                self.color(),
             ))
         } else {
             ReplaceOutput::Original(self.clone())

diff --git a/crates/fj-core/src/operations/replace/half_edge.rs b/crates/fj-core/src/operations/replace/half_edge.rs
@@ -83,7 +83,6 @@ impl ReplaceHalfEdge for Region {
                     })
                     .into_inner(),
                 interiors,
-                self.color(),
             ))
         } else {
             ReplaceOutput::Original(self.clone())

diff --git a/crates/fj-core/src/operations/replace/vertex.rs b/crates/fj-core/src/operations/replace/vertex.rs
@@ -119,7 +119,6 @@ impl ReplaceVertex for Region {
                     })
                     .into_inner(),
                 interiors,
-                self.color(),
             ))
         } else {
             ReplaceOutput::Original(self.clone())

diff --git a/crates/fj-core/src/operations/reverse/region.rs b/crates/fj-core/src/operations/reverse/region.rs
@@ -17,7 +17,7 @@ impl Reverse for Region {
             cycle.reverse(core).insert(core).derive_from(cycle, core)
         });
 
-        Region::new(exterior, interiors, self.color())
+        Region::new(exterior, interiors)
     }
 }
 
@@ -35,6 +35,6 @@ impl ReverseCurveCoordinateSystems for Region {
                 .derive_from(cycle, core)
         });
 
-        Region::new(exterior, interiors, self.color())
+        Region::new(exterior, interiors)
     }
 }
diff --git a/crates/fj-core/src/operations/sweep/face.rs b/crates/fj-core/src/operations/sweep/face.rs
@@ -2,7 +2,7 @@ use fj_math::Vector;
 
 use crate::{
     objects::{Face, Shell},
-    operations::insert::Insert,
+    operations::{insert::Insert, presentation::GetColor},
     storage::Handle,
     Core,
 };
@@ -49,7 +49,13 @@ impl SweepFace for Handle<Face> {
         let bottom_face = self;
         let other_faces = bottom_face
             .region()
-            .sweep_region(bottom_face.surface(), path, cache, core)
+            .sweep_region(
+                bottom_face.surface(),
+                bottom_face.region().get_color(core),
+                path,
+                cache,
+                core,
+            )
             .all_faces()
             .map(|side_face| side_face.insert(core));
 

diff --git a/crates/fj-core/src/operations/sweep/half_edge.rs b/crates/fj-core/src/operations/sweep/half_edge.rs
@@ -6,6 +6,7 @@ use crate::{
     operations::{
         build::{BuildCycle, BuildHalfEdge},
         insert::Insert,
+        presentation::SetColor,
         update::{UpdateCycle, UpdateHalfEdge},
     },
     storage::Handle,
@@ -135,7 +136,12 @@ impl SweepHalfEdge for HalfEdge {
             });
 
         let exterior = exterior.insert(core);
-        let region = Region::new(exterior, [], color).insert(core);
+        let region = Region::new(exterior, []).insert(core);
+
+        if let Some(color) = color {
+            region.set_color(color, core);
+        }
+
         let face = Face::new(surface, region);
 
         (face, edge_top)

diff --git a/crates/fj-core/src/operations/sweep/region.rs b/crates/fj-core/src/operations/sweep/region.rs
@@ -32,6 +32,7 @@ pub trait SweepRegion {
     fn sweep_region(
         &self,
         surface: &Surface,
+        color: Option<Color>,
         path: impl Into<Vector<3>>,
         cache: &mut SweepCache,
         core: &mut Core,
@@ -42,6 +43,7 @@ impl SweepRegion for Region {
     fn sweep_region(
         &self,
         surface: &Surface,
+        color: Option<Color>,
         path: impl Into<Vector<3>>,
         cache: &mut SweepCache,
         core: &mut Core,
@@ -53,7 +55,7 @@ impl SweepRegion for Region {
         let top_exterior = sweep_cycle(
             self.exterior(),
             surface,
-            self.color(),
+            color,
             &mut faces,
             path,
             cache,
@@ -67,7 +69,7 @@ impl SweepRegion for Region {
                 sweep_cycle(
                     bottom_cycle,
                     surface,
-                    self.color(),
+                    color,
                     &mut faces,
                     path,
                     cache,
@@ -79,8 +81,7 @@ impl SweepRegion for Region {
         let top_face = {
             let top_surface = surface.translate(path, core).insert(core);
             let top_region =
-                Region::new(top_exterior, top_interiors, self.color())
-                    .insert(core);
+                Region::new(top_exterior, top_interiors).insert(core);
 
             Face::new(top_surface, top_region)
         };

diff --git a/crates/fj-core/src/operations/sweep/shell_face.rs b/crates/fj-core/src/operations/sweep/shell_face.rs
@@ -5,6 +5,7 @@ use crate::{
     operations::{
         derive::DeriveFrom,
         insert::Insert,
+        presentation::GetColor,
         reverse::Reverse,
         sweep::{SweepCache, SweepRegion},
         update::UpdateShell,
@@ -63,9 +64,15 @@ impl SweepFaceOfShell for Shell {
             .reverse(core)
             .insert(core)
             .derive_from(face.region().exterior(), core);
-        let region = Region::new(exterior, [], face.region().color());
+        let region = Region::new(exterior, []);
         let faces = region
-            .sweep_region(face.surface(), path, &mut cache, core)
+            .sweep_region(
+                face.surface(),
+                face.region().get_color(core),
+                path,
+                &mut cache,
+                core,
+            )
             .all_faces()
             .collect::<Vec<_>>();
 

diff --git a/crates/fj-core/src/operations/transform/region.rs b/crates/fj-core/src/operations/transform/region.rs
@@ -9,9 +9,6 @@ impl TransformObject for Region {
         core: &mut Core,
         cache: &mut super::TransformCache,
     ) -> Self {
-        // Color does not need to be transformed.
-        let color = self.color();
-
         let exterior = self
             .exterior()
             .clone()
@@ -20,6 +17,6 @@ impl TransformObject for Region {
             interior.transform_with_cache(transform, core, cache)
         });
 
-        Region::new(exterior, interiors, color)
+        Region::new(exterior, interiors)
     }
 }
diff --git a/crates/fj-core/src/operations/update/region.rs b/crates/fj-core/src/operations/update/region.rs
@@ -57,7 +57,7 @@ impl UpdateRegion for Region {
         let exterior = update(self.exterior(), core)
             .insert(core)
             .derive_from(self.exterior(), core);
-        Region::new(exterior, self.interiors().iter().cloned(), self.color())
+        Region::new(exterior, self.interiors().iter().cloned())
     }
 
     fn add_interiors<T>(
@@ -70,7 +70,7 @@ impl UpdateRegion for Region {
     {
         let interiors = interiors.into_iter().map(|cycle| cycle.insert(core));
         let interiors = self.interiors().iter().cloned().chain(interiors);
-        Region::new(self.exterior().clone(), interiors, self.color())
+        Region::new(self.exterior().clone(), interiors)
     }
 
     fn update_interior<T, const N: usize>(
@@ -91,6 +91,6 @@ impl UpdateRegion for Region {
                 }),
             )
             .expect("Cycle not found");
-        Region::new(self.exterior().clone(), interiors, self.color())
+        Region::new(self.exterior().clone(), interiors)
     }
 }
diff --git a/crates/fj-core/src/validate/face.rs b/crates/fj-core/src/validate/face.rs
@@ -174,12 +174,9 @@ mod tests {
                 })
                 .collect::<Vec<_>>();
 
-            let region = Region::new(
-                valid.region().exterior().clone(),
-                interiors,
-                valid.region().color(),
-            )
-            .insert(&mut core);
+            let region =
+                Region::new(valid.region().exterior().clone(), interiors)
+                    .insert(&mut core);
 
             Face::new(valid.surface().clone(), region)
         };