Skip to content

Commit

Permalink
Merge pull request #1305 from hannobraun/partial
Browse files Browse the repository at this point in the history
Start extracting new builder API from partial object API
  • Loading branch information
hannobraun authored Nov 3, 2022
2 parents eae922b + f3ef004 commit 412f1b2
Show file tree
Hide file tree
Showing 22 changed files with 373 additions and 238 deletions.
9 changes: 5 additions & 4 deletions crates/fj-kernel/src/algorithms/approx/curve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ mod tests {

use crate::{
algorithms::approx::{path::RangeOnPath, Approx, ApproxPoint},
builder::CurveBuilder,
objects::{Curve, Objects, Surface},
partial::HasPartial,
path::GlobalPath,
Expand All @@ -213,7 +214,7 @@ mod tests {
.insert(Surface::new(GlobalPath::x_axis(), [0., 0., 1.]))?;
let curve = Curve::partial()
.with_surface(Some(surface))
.as_line_from_points([[1., 1.], [2., 1.]])
.update_as_line_from_points([[1., 1.], [2., 1.]])
.build(&objects)?;
let range = RangeOnPath::from([[0.], [1.]]);

Expand All @@ -234,7 +235,7 @@ mod tests {
))?;
let curve = Curve::partial()
.with_surface(Some(surface))
.as_line_from_points([[1., 1.], [1., 2.]])
.update_as_line_from_points([[1., 1.], [1., 2.]])
.build(&objects)?;
let range = RangeOnPath::from([[0.], [1.]]);

Expand All @@ -253,7 +254,7 @@ mod tests {
objects.surfaces.insert(Surface::new(path, [0., 0., 1.]))?;
let curve = Curve::partial()
.with_surface(Some(surface.clone()))
.as_line_from_points([[0., 1.], [1., 1.]])
.update_as_line_from_points([[0., 1.], [1., 1.]])
.build(&objects)?;

let range = RangeOnPath::from([[0.], [TAU]]);
Expand Down Expand Up @@ -285,7 +286,7 @@ mod tests {
.insert(Surface::new(GlobalPath::x_axis(), [0., 0., 1.]))?;
let curve = Curve::partial()
.with_surface(Some(surface))
.as_circle_from_radius(1.)
.update_as_circle_from_radius(1.)
.build(&objects)?;

let range = RangeOnPath::from([[0.], [TAU]]);
Expand Down
9 changes: 5 additions & 4 deletions crates/fj-kernel/src/algorithms/intersect/curve_edge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ mod tests {
use fj_math::Point;

use crate::{
builder::CurveBuilder,
objects::{Curve, HalfEdge, Objects},
partial::HasPartial,
};
Expand All @@ -88,7 +89,7 @@ mod tests {
let surface = objects.surfaces.xy_plane();
let curve = Curve::partial()
.with_surface(Some(surface.clone()))
.as_u_axis()
.update_as_u_axis()
.build(&objects)?;
let half_edge = HalfEdge::partial()
.with_surface(Some(surface))
Expand All @@ -113,7 +114,7 @@ mod tests {
let surface = objects.surfaces.xy_plane();
let curve = Curve::partial()
.with_surface(Some(surface.clone()))
.as_u_axis()
.update_as_u_axis()
.build(&objects)?;
let half_edge = HalfEdge::partial()
.with_surface(Some(surface))
Expand All @@ -138,7 +139,7 @@ mod tests {
let surface = objects.surfaces.xy_plane();
let curve = Curve::partial()
.with_surface(Some(surface.clone()))
.as_u_axis()
.update_as_u_axis()
.build(&objects)?;
let half_edge = HalfEdge::partial()
.with_surface(Some(surface))
Expand All @@ -158,7 +159,7 @@ mod tests {
let surface = objects.surfaces.xy_plane();
let curve = Curve::partial()
.with_surface(Some(surface.clone()))
.as_u_axis()
.update_as_u_axis()
.build(&objects)?;
let half_edge = HalfEdge::partial()
.with_surface(Some(surface))
Expand Down
3 changes: 2 additions & 1 deletion crates/fj-kernel/src/algorithms/intersect/curve_face.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ where
#[cfg(test)]
mod tests {
use crate::{
builder::CurveBuilder,
objects::{Curve, Face, Objects},
partial::HasPartial,
};
Expand All @@ -164,7 +165,7 @@ mod tests {

let curve = Curve::partial()
.with_surface(Some(surface.clone()))
.as_line_from_points([[-3., 0.], [-2., 0.]])
.update_as_line_from_points([[-3., 0.], [-2., 0.]])
.build(&objects)?;

#[rustfmt::skip]
Expand Down
3 changes: 2 additions & 1 deletion crates/fj-kernel/src/algorithms/intersect/face_face.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ mod tests {

use crate::{
algorithms::intersect::CurveFaceIntersection,
builder::CurveBuilder,
objects::{Curve, Face, Objects},
partial::HasPartial,
};
Expand Down Expand Up @@ -124,7 +125,7 @@ mod tests {
let expected_curves = surfaces.try_map_ext(|surface| {
Curve::partial()
.with_surface(Some(surface))
.as_line_from_points([[0., 0.], [1., 0.]])
.update_as_line_from_points([[0., 0.], [1., 0.]])
.build(&objects)
})?;
let expected_intervals =
Expand Down
5 changes: 3 additions & 2 deletions crates/fj-kernel/src/algorithms/intersect/surface_surface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ mod tests {

use crate::{
algorithms::transform::TransformObject,
builder::CurveBuilder,
objects::{Curve, Objects},
partial::HasPartial,
};
Expand Down Expand Up @@ -122,11 +123,11 @@ mod tests {

let expected_xy = Curve::partial()
.with_surface(Some(xy.clone()))
.as_u_axis()
.update_as_u_axis()
.build(&objects)?;
let expected_xz = Curve::partial()
.with_surface(Some(xz.clone()))
.as_u_axis()
.update_as_u_axis()
.build(&objects)?;

assert_eq!(
Expand Down
3 changes: 2 additions & 1 deletion crates/fj-kernel/src/algorithms/sweep/vertex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ mod tests {

use crate::{
algorithms::sweep::Sweep,
builder::CurveBuilder,
objects::{Curve, HalfEdge, Objects, Vertex},
partial::HasPartial,
};
Expand All @@ -179,7 +180,7 @@ mod tests {
let surface = objects.surfaces.xz_plane();
let curve = Curve::partial()
.with_surface(Some(surface.clone()))
.as_u_axis()
.update_as_u_axis()
.build(&objects)?;
let vertex = Vertex::partial()
.with_position(Some([0.]))
Expand Down
30 changes: 12 additions & 18 deletions crates/fj-kernel/src/algorithms/transform/curve.rs
Original file line number Diff line number Diff line change
@@ -1,29 +1,24 @@
use fj_math::Transform;

use crate::{
objects::{GlobalCurve, Objects},
partial::PartialCurve,
storage::Handle,
objects::Objects,
partial::{PartialCurve, PartialGlobalCurve},
validate::ValidationError,
};

use super::TransformObject;

impl TransformObject for Handle<GlobalCurve> {
impl TransformObject for PartialGlobalCurve {
fn transform(
self,
_: &Transform,
objects: &Objects,
_: &Objects,
) -> Result<Self, ValidationError> {
// `GlobalCurve` doesn't contain any internal geometry. If it did, that
// would just be redundant with the geometry of other objects, and this
// other geometry is already being transformed by other implementations
// of this trait.
//
// All we need to do here is create a new `GlobalCurve` instance, to
// make sure the transformed `GlobalCurve` has a different identity than
// the original one.
Ok(objects.global_curves.insert(GlobalCurve)?)
Ok(self)
}
}

Expand All @@ -34,20 +29,19 @@ impl TransformObject for PartialCurve {
objects: &Objects,
) -> Result<Self, ValidationError> {
let surface = self
.surface
.surface()
.map(|surface| surface.transform(transform, objects))
.transpose()?;
let global_form = self
.global_form
.map(|global_form| global_form.0.transform(transform, objects))
.global_form()
.map(|global_form| global_form.transform(transform, objects))
.transpose()?;

// Don't need to transform `self.path`, as that's defined in surface
// coordinates, and thus transforming `surface` takes care of it.
Ok(Self {
surface,
path: self.path,
global_form: global_form.map(Into::into),
})
Ok(Self::default()
.with_surface(surface)
.with_path(self.path())
.with_global_form(global_form))
}
}
10 changes: 2 additions & 8 deletions crates/fj-kernel/src/algorithms/transform/edge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,7 @@ impl TransformObject for PartialGlobalEdge {
transform: &Transform,
objects: &Objects,
) -> Result<Self, ValidationError> {
let curve = self
.curve
.map(|curve| curve.0.transform(transform, objects))
.transpose()?;
let curve = self.curve.transform(transform, objects)?;
let vertices = self
.vertices
.map(|vertices| {
Expand All @@ -70,9 +67,6 @@ impl TransformObject for PartialGlobalEdge {
})
.transpose()?;

Ok(Self {
curve: curve.map(Into::into),
vertices,
})
Ok(Self { curve, vertices })
}
}
30 changes: 14 additions & 16 deletions crates/fj-kernel/src/algorithms/transform/vertex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,19 @@ impl TransformObject for PartialVertex {
transform: &Transform,
objects: &Objects,
) -> Result<Self, ValidationError> {
let curve = self.curve.transform(transform, objects)?;
let curve = self.curve().transform(transform, objects)?;
let surface_form = self
.surface_form
.surface_form()
.into_partial()
.transform(transform, objects)?
.into();

// Don't need to transform `self.position`, as that is in curve
// coordinates and thus transforming the curve takes care of it.
Ok(Self {
position: self.position,
curve,
surface_form,
})
Ok(Self::default()
.with_position(self.position())
.with_curve(Some(curve))
.with_surface_form(surface_form))
}
}

Expand All @@ -38,18 +37,17 @@ impl TransformObject for PartialSurfaceVertex {
objects: &Objects,
) -> Result<Self, ValidationError> {
let surface = self
.surface
.surface()
.map(|surface| surface.transform(transform, objects))
.transpose()?;
let global_form = self.global_form.transform(transform, objects)?;
let global_form = self.global_form().transform(transform, objects)?;

// Don't need to transform `self.position`, as that is in surface
// coordinates and thus transforming the surface takes care of it.
Ok(Self {
position: self.position,
surface,
global_form,
})
Ok(Self::default()
.with_position(self.position())
.with_surface(surface)
.with_global_form(Some(global_form)))
}
}

Expand All @@ -60,9 +58,9 @@ impl TransformObject for PartialGlobalVertex {
_: &Objects,
) -> Result<Self, ValidationError> {
let position = self
.position
.position()
.map(|position| transform.transform_point(&position));

Ok(Self { position })
Ok(Self::default().with_position(position))
}
}
48 changes: 48 additions & 0 deletions crates/fj-kernel/src/builder/curve.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
use fj_math::{Point, Scalar, Vector};

use crate::{partial::PartialCurve, path::SurfacePath};

/// Builder API for [`PartialCurve`]
pub trait CurveBuilder {
/// Update partial curve to represent the u-axis
fn update_as_u_axis(self) -> Self;

/// Update partial curve to represent the v-axis
fn update_as_v_axis(self) -> Self;

/// Update partial curve as a circle, from the provided radius
fn update_as_circle_from_radius(self, radius: impl Into<Scalar>) -> Self;

/// Update partial curve as a line, from the provided points
fn update_as_line_from_points(
self,
points: [impl Into<Point<2>>; 2],
) -> Self;
}

impl CurveBuilder for PartialCurve {
fn update_as_u_axis(self) -> Self {
let a = Point::origin();
let b = a + Vector::unit_u();

self.update_as_line_from_points([a, b])
}

fn update_as_v_axis(self) -> Self {
let a = Point::origin();
let b = a + Vector::unit_v();

self.update_as_line_from_points([a, b])
}

fn update_as_circle_from_radius(self, radius: impl Into<Scalar>) -> Self {
self.with_path(Some(SurfacePath::circle_from_radius(radius)))
}

fn update_as_line_from_points(
self,
points: [impl Into<Point<2>>; 2],
) -> Self {
self.with_path(Some(SurfacePath::line_from_points(points)))
}
}
13 changes: 12 additions & 1 deletion crates/fj-kernel/src/builder/mod.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,22 @@
//! API for building objects
// These are the old-style builders that need to be transferred to the partial
// object API. Issue:
// https://github.com/hannobraun/Fornjot/issues/1147
mod face;
mod shell;
mod sketch;
mod solid;

// These are new-style builders that build on top of the partial object API.
mod curve;
mod vertex;

pub use self::{
face::FaceBuilder, shell::ShellBuilder, sketch::SketchBuilder,
curve::CurveBuilder,
face::FaceBuilder,
shell::ShellBuilder,
sketch::SketchBuilder,
solid::SolidBuilder,
vertex::{GlobalVertexBuilder, SurfaceVertexBuilder, VertexBuilder},
};
Loading

0 comments on commit 412f1b2

Please sign in to comment.