Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't create duplicate Surfaces in fj-operations #1168

Merged
merged 3 commits into from
Oct 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions crates/fj-operations/src/difference_2d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ use fj_kernel::{
};
use fj_math::Aabb;

use crate::planes::Planes;

use super::Shape;

impl Shape for fj::Difference2d {
Expand All @@ -19,6 +21,7 @@ impl Shape for fj::Difference2d {
&self,
config: &ValidationConfig,
stores: &Stores,
planes: &Planes,
debug_info: &mut DebugInfo,
) -> Result<Validated<Self::Brep>, ValidationError> {
// This method assumes that `b` is fully contained within `a`:
Expand All @@ -33,8 +36,9 @@ impl Shape for fj::Difference2d {
// - https://doc.rust-lang.org/std/primitive.array.html#method.each_ref
// - https://doc.rust-lang.org/std/primitive.array.html#method.try_map
let [a, b] = self.shapes();
let [a, b] =
[a, b].map(|shape| shape.compute_brep(config, stores, debug_info));
let [a, b] = [a, b].map(|shape| {
shape.compute_brep(config, stores, planes, debug_info)
});
let [a, b] = [a?, b?];

if let Some(face) = a.face_iter().next() {
Expand Down
7 changes: 5 additions & 2 deletions crates/fj-operations/src/group.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ use fj_kernel::{
};
use fj_math::Aabb;

use crate::planes::Planes;

use super::Shape;

impl Shape for fj::Group {
Expand All @@ -17,12 +19,13 @@ impl Shape for fj::Group {
&self,
config: &ValidationConfig,
stores: &Stores,
planes: &Planes,
debug_info: &mut DebugInfo,
) -> Result<Validated<Self::Brep>, ValidationError> {
let mut faces = Faces::new();

let a = self.a.compute_brep(config, stores, debug_info)?;
let b = self.b.compute_brep(config, stores, debug_info)?;
let a = self.a.compute_brep(config, stores, planes, debug_info)?;
let b = self.b.compute_brep(config, stores, planes, debug_info)?;

faces.extend(a.into_inner());
faces.extend(b.into_inner());
Expand Down
18 changes: 12 additions & 6 deletions crates/fj-operations/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,13 @@ pub mod shape_processor;

mod difference_2d;
mod group;
mod planes;
mod sketch;
mod sweep;
mod transform;

pub use self::planes::Planes;

use fj_interop::debug::DebugInfo;
use fj_kernel::{
algorithms::validate::{
Expand All @@ -44,6 +47,7 @@ pub trait Shape {
&self,
config: &ValidationConfig,
stores: &Stores,
planes: &Planes,
debug_info: &mut DebugInfo,
) -> Result<Validated<Self::Brep>, ValidationError>;

Expand All @@ -61,19 +65,20 @@ impl Shape for fj::Shape {
&self,
config: &ValidationConfig,
stores: &Stores,
planes: &Planes,
debug_info: &mut DebugInfo,
) -> Result<Validated<Self::Brep>, ValidationError> {
match self {
Self::Shape2d(shape) => shape
.compute_brep(config, stores, debug_info)?
.compute_brep(config, stores, planes, debug_info)?
.into_inner()
.into_faces()
.validate_with_config(config),
Self::Group(shape) => {
shape.compute_brep(config, stores, debug_info)
shape.compute_brep(config, stores, planes, debug_info)
}
Self::Sweep(shape) => shape
.compute_brep(config, stores, debug_info)?
.compute_brep(config, stores, planes, debug_info)?
.into_inner()
.into_shells()
.map(|shell| shell.into_faces())
Expand All @@ -84,7 +89,7 @@ impl Shape for fj::Shape {
.unwrap_or_default()
.validate_with_config(config),
Self::Transform(shape) => {
shape.compute_brep(config, stores, debug_info)
shape.compute_brep(config, stores, planes, debug_info)
}
}
}
Expand All @@ -106,14 +111,15 @@ impl Shape for fj::Shape2d {
&self,
config: &ValidationConfig,
stores: &Stores,
planes: &Planes,
debug_info: &mut DebugInfo,
) -> Result<Validated<Self::Brep>, ValidationError> {
match self {
Self::Difference(shape) => {
shape.compute_brep(config, stores, debug_info)
shape.compute_brep(config, stores, planes, debug_info)
}
Self::Sketch(shape) => {
shape.compute_brep(config, stores, debug_info)
shape.compute_brep(config, stores, planes, debug_info)
}
}
}
Expand Down
50 changes: 50 additions & 0 deletions crates/fj-operations/src/planes.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
use fj_kernel::{
objects::Surface,
stores::{Handle, Stores},
};

/// The static planes
///
/// Keeps [`Handle`]s to the xy-, xz- and yz-planes. The purpose of this struct
/// is to provide these handles to implementations of [`Shape`], so they don't
/// have to create a duplicate `Surface` whenever they need one of those.
///
/// [`Shape`]: crate::Shape
pub struct Planes {
xy: Handle<Surface>,
xz: Handle<Surface>,
yz: Handle<Surface>,
}

impl Planes {
/// Create a new instance of `Planes`
///
/// Please note that the whole point of this struct is to not duplicate the
/// standard planes, and creating multiple instances of it defeats that
/// point.
///
/// Create one instance of this struct, then share it everywhere it's
/// needed.
pub fn new(stores: &Stores) -> Self {
let xy = stores.surfaces.insert(Surface::xy_plane());
let xz = stores.surfaces.insert(Surface::xz_plane());
let yz = stores.surfaces.insert(Surface::yz_plane());

Self { xy, xz, yz }
}

/// Access the xy-plane
pub fn xy(&self) -> Handle<Surface> {
self.xy.clone()
}

/// Access the xz-plane
pub fn xz(&self) -> Handle<Surface> {
self.xz.clone()
}

/// Access the yz-plane
pub fn yz(&self) -> Handle<Surface> {
self.yz.clone()
}
}
6 changes: 4 additions & 2 deletions crates/fj-operations/src/shape_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use fj_kernel::{
};
use fj_math::Scalar;

use crate::Shape as _;
use crate::{planes::Planes, Shape as _};

/// Processes an [`fj::Shape`] into a [`ProcessedShape`]
pub struct ShapeProcessor {
Expand Down Expand Up @@ -44,8 +44,10 @@ impl ShapeProcessor {

let config = ValidationConfig::default();
let stores = Stores::new();
let planes = Planes::new(&stores);
let mut debug_info = DebugInfo::new();
let shape = shape.compute_brep(&config, &stores, &mut debug_info)?;
let shape =
shape.compute_brep(&config, &stores, &planes, &mut debug_info)?;
let mesh = (&shape.into_inner(), tolerance).triangulate();

Ok(ProcessedShape {
Expand Down
7 changes: 5 additions & 2 deletions crates/fj-operations/src/sketch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@ use fj_kernel::{
algorithms::validate::{
Validate, Validated, ValidationConfig, ValidationError,
},
objects::{Cycle, Face, HalfEdge, Sketch, Surface},
objects::{Cycle, Face, HalfEdge, Sketch},
partial::HasPartial,
stores::Stores,
};
use fj_math::{Aabb, Point};

use crate::planes::Planes;

use super::Shape;

impl Shape for fj::Sketch {
Expand All @@ -18,9 +20,10 @@ impl Shape for fj::Sketch {
&self,
config: &ValidationConfig,
stores: &Stores,
planes: &Planes,
_: &mut DebugInfo,
) -> Result<Validated<Self::Brep>, ValidationError> {
let surface = stores.surfaces.insert(Surface::xy_plane());
let surface = planes.xy();

let face = match self.chain() {
fj::Chain::Circle(circle) => {
Expand Down
7 changes: 6 additions & 1 deletion crates/fj-operations/src/sweep.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ use fj_kernel::{
};
use fj_math::{Aabb, Vector};

use crate::planes::Planes;

use super::Shape;

impl Shape for fj::Sweep {
Expand All @@ -18,9 +20,12 @@ impl Shape for fj::Sweep {
&self,
config: &ValidationConfig,
stores: &Stores,
planes: &Planes,
debug_info: &mut DebugInfo,
) -> Result<Validated<Self::Brep>, ValidationError> {
let sketch = self.shape().compute_brep(config, stores, debug_info)?;
let sketch = self
.shape()
.compute_brep(config, stores, planes, debug_info)?;
let path = Vector::from(self.path());

let solid = sketch.into_inner().sweep(path, stores);
Expand Down
5 changes: 4 additions & 1 deletion crates/fj-operations/src/transform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ use fj_kernel::{
};
use fj_math::{Aabb, Transform, Vector};

use crate::planes::Planes;

use super::Shape;

impl Shape for fj::Transform {
Expand All @@ -18,11 +20,12 @@ impl Shape for fj::Transform {
&self,
config: &ValidationConfig,
stores: &Stores,
planes: &Planes,
debug_info: &mut DebugInfo,
) -> Result<Validated<Self::Brep>, ValidationError> {
let faces = self
.shape
.compute_brep(config, stores, debug_info)?
.compute_brep(config, stores, planes, debug_info)?
.into_inner()
.transform(&make_transform(self), stores);

Expand Down