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

Remove geometry from GlobalCurve #1153

Merged
merged 6 commits into from
Sep 29, 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
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,7 @@ impl SurfaceSurfaceIntersection {

let curves = planes_parametric.map(|(surface, plane)| {
let path = project_line_into_plane(&line, &plane);
let global_form = stores.global_curves.insert(
GlobalCurve::from_path(GlobalPath::Line(
Line::from_origin_and_direction(origin, direction),
)),
);
let global_form = GlobalCurve::new(stores);

Curve::new(surface, path, global_form)
});
Expand Down
42 changes: 9 additions & 33 deletions crates/fj-kernel/src/algorithms/sweep/vertex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@ use crate::{
Curve, GlobalCurve, GlobalEdge, GlobalVertex, HalfEdge, Surface,
SurfaceVertex, Vertex,
},
partial::HasPartial,
path::SurfacePath,
stores::{Handle, Stores},
stores::Stores,
};

use super::Sweep;
Expand Down Expand Up @@ -41,19 +40,15 @@ impl Sweep for (Vertex, Surface) {

// So, we're supposed to create the `Edge` by sweeping a `Vertex` using
// `path`. Unless `path` is identical to the path that created the
// `Surface`, this doesn't make any sense.
// `Surface`, this doesn't make any sense. Let's make sure this
// requirement is met.
//
// Further, the `Curve` that was swept to create the `Surface` needs to
// be the same `Curve` that the input `Vertex` is defined on. If it's
// not, we have no way of knowing the surface coordinates of the input
// `Vertex` on the `Surface`, and we're going to need to do that further
// down.
//
// Let's make sure that these requirements are met.
{
assert_eq!(vertex.curve().global_form().path(), surface.u());
assert_eq!(path, surface.v());
}
// down. There's no way to check for that, unfortunately.
assert_eq!(path, surface.v());

// With that out of the way, let's start by creating the `GlobalEdge`,
// as that is the most straight-forward part of this operations, and
Expand Down Expand Up @@ -135,13 +130,11 @@ impl Sweep for GlobalVertex {
type Swept = GlobalEdge;

fn sweep(self, path: impl Into<Vector<3>>, stores: &Stores) -> Self::Swept {
let curve = GlobalCurve::new(stores);

let a = self;
let b = GlobalVertex::from_position(self.position() + path.into());

let curve = Handle::<GlobalCurve>::partial()
.as_line_from_points([a.position(), b.position()])
.build(stores);

GlobalEdge::new(curve, [a, b])
}
}
Expand All @@ -152,12 +145,9 @@ mod tests {

use crate::{
algorithms::sweep::Sweep,
objects::{
Curve, GlobalCurve, GlobalEdge, GlobalVertex, HalfEdge, Surface,
Vertex,
},
objects::{Curve, HalfEdge, Surface, Vertex},
partial::HasPartial,
stores::{Handle, Stores},
stores::Stores,
};

#[test]
Expand All @@ -181,18 +171,4 @@ mod tests {
.build(&stores);
assert_eq!(half_edge, expected_half_edge);
}

#[test]
fn global_vertex() {
let stores = Stores::new();

let edge = GlobalVertex::from_position([0., 0., 0.])
.sweep([0., 0., 1.], &stores);

let expected_edge = GlobalEdge::new(
Handle::<GlobalCurve>::partial().as_z_axis().build(&stores),
[[0., 0., 0.], [0., 0., 1.]].map(GlobalVertex::from_position),
);
assert_eq!(edge, expected_edge);
}
}
27 changes: 13 additions & 14 deletions crates/fj-kernel/src/algorithms/transform/curve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use fj_math::Transform;

use crate::{
objects::{Curve, GlobalCurve},
partial::{PartialCurve, PartialGlobalCurve},
partial::PartialCurve,
stores::{Handle, Stores},
};

Expand All @@ -21,10 +21,16 @@ impl TransformObject for Curve {
}

impl TransformObject for Handle<GlobalCurve> {
fn transform(self, transform: &Transform, stores: &Stores) -> Self {
stores.global_curves.insert(GlobalCurve::from_path(
self.path().transform(transform, stores),
))
fn transform(self, _: &Transform, stores: &Stores) -> Self {
// `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.
GlobalCurve::new(stores)
}
}

Expand All @@ -35,21 +41,14 @@ impl TransformObject for PartialCurve {
.map(|surface| surface.transform(transform, stores));
let global_form = self
.global_form
.map(|global_form| global_form.transform(transform, stores));
.map(|global_form| global_form.0.transform(transform, stores));

// Don't need to transform `self.path`, as that's defined in surface
// coordinates, and thus transforming `surface` takes care of it.
Self {
surface,
path: self.path,
global_form,
global_form: global_form.map(Into::into),
}
}
}

impl TransformObject for PartialGlobalCurve {
fn transform(self, transform: &Transform, stores: &Stores) -> Self {
let path = self.path.map(|path| path.transform(transform, stores));
Self { path }
}
}
8 changes: 6 additions & 2 deletions crates/fj-kernel/src/algorithms/transform/edge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,15 @@ impl TransformObject for GlobalEdge {

impl TransformObject for PartialGlobalEdge {
fn transform(self, transform: &Transform, stores: &Stores) -> Self {
let curve = self.curve.map(|curve| curve.transform(transform, stores));
let curve =
self.curve.map(|curve| curve.0.transform(transform, stores));
let vertices = self.vertices.map(|vertices| {
vertices.map(|vertex| vertex.transform(transform, stores))
});

Self { curve, vertices }
Self {
curve: curve.map(Into::into),
vertices,
}
}
}
33 changes: 0 additions & 33 deletions crates/fj-kernel/src/algorithms/validate/coherence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,39 +4,6 @@ use fj_math::{Point, Scalar};

use crate::objects::{Curve, Vertex};

pub fn validate_curve(
curve: &Curve,
max_distance: impl Into<Scalar>,
) -> Result<(), CoherenceIssues> {
let max_distance = max_distance.into();

let points_curve = [-2., -1., 0., 1., 2.].map(|point| Point::from([point]));

for point_curve in points_curve {
let point_surface = curve.path().point_from_path_coords(point_curve);
let point_surface_as_global =
curve.surface().point_from_surface_coords(point_surface);
let point_global = curve
.global_form()
.path()
.point_from_path_coords(point_curve);

let distance = (point_surface_as_global - point_global).magnitude();

if distance > max_distance {
Err(CurveCoherenceMismatch {
point_curve,
point_surface,
point_surface_as_global,
point_global,
curve: curve.clone(),
})?
}
}

Ok(())
}

pub fn validate_vertex(
vertex: &Vertex,
max_distance: impl Into<Scalar>,
Expand Down
33 changes: 3 additions & 30 deletions crates/fj-kernel/src/algorithms/validate/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,6 @@ where
) -> Result<Validated<Self>, ValidationError> {
let mut global_vertices = HashSet::new();

for curve in self.curve_iter() {
coherence::validate_curve(curve, config.identical_max_distance)?;
}

for global_vertex in self.global_vertex_iter() {
uniqueness::validate_vertex(
global_vertex,
Expand Down Expand Up @@ -156,7 +152,7 @@ pub enum ValidationError {

#[cfg(test)]
mod tests {
use fj_math::{Line, Point, Scalar};
use fj_math::{Point, Scalar};

use crate::{
algorithms::validate::{Validate, ValidationConfig, ValidationError},
Expand All @@ -165,30 +161,10 @@ mod tests {
SurfaceVertex, Vertex,
},
partial::HasPartial,
path::{GlobalPath, SurfacePath},
path::SurfacePath,
stores::Stores,
};

#[test]
fn coherence_curve() {
let stores = Stores::new();

let line_global = Line::from_points([[0., 0., 0.], [1., 0., 0.]]);
let global_curve = stores
.global_curves
.insert(GlobalCurve::from_path(GlobalPath::Line(line_global)));

let line_surface = Line::from_points([[0., 0.], [2., 0.]]);
let curve = Curve::new(
Surface::xy_plane(),
SurfacePath::Line(line_surface),
global_curve,
);

let result = curve.validate();
assert!(result.is_err());
}

#[test]
fn coherence_edge() {
let stores = Stores::new();
Expand All @@ -200,10 +176,7 @@ mod tests {

let curve = {
let path = SurfacePath::line_from_points(points_surface);
let curve_global =
stores.global_curves.insert(GlobalCurve::from_path(
GlobalPath::line_from_points(points_global),
));
let curve_global = GlobalCurve::new(&stores);
Curve::new(surface, path, curve_global)
};

Expand Down
5 changes: 2 additions & 3 deletions crates/fj-kernel/src/iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ mod tests {
Sketch, Solid, Surface, SurfaceVertex, Vertex,
},
partial::HasPartial,
stores::{Handle, Stores},
stores::Stores,
};

use super::ObjectIters as _;
Expand Down Expand Up @@ -441,8 +441,7 @@ mod tests {
fn global_curve() {
let stores = Stores::new();

let object =
Handle::<GlobalCurve>::partial().as_x_axis().build(&stores);
let object = GlobalCurve::new(&stores);

assert_eq!(0, object.curve_iter().count());
assert_eq!(0, object.cycle_iter().count());
Expand Down
27 changes: 11 additions & 16 deletions crates/fj-kernel/src/objects/curve.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::{
path::{GlobalPath, SurfacePath},
stores::Handle,
path::SurfacePath,
stores::{Handle, HandleWrapper, Stores},
};

use super::Surface;
Expand All @@ -10,16 +10,18 @@ use super::Surface;
pub struct Curve {
path: SurfacePath,
surface: Surface,
global_form: Handle<GlobalCurve>,
global_form: HandleWrapper<GlobalCurve>,
}

impl Curve {
/// Construct a new instance of `Curve`
pub fn new(
surface: Surface,
path: SurfacePath,
global_form: Handle<GlobalCurve>,
global_form: impl Into<HandleWrapper<GlobalCurve>>,
) -> Self {
let global_form = global_form.into();

Self {
surface,
path,
Expand All @@ -44,19 +46,12 @@ impl Curve {
}

/// A curve, defined in global (3D) coordinates
#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
pub struct GlobalCurve {
path: GlobalPath,
}
#[derive(Clone, Copy, Debug)]
pub struct GlobalCurve;

impl GlobalCurve {
/// Construct a `GlobalCurve` from the path that defines it
pub fn from_path(path: GlobalPath) -> Self {
Self { path }
}

/// Access the path that defines this curve
pub fn path(&self) -> GlobalPath {
self.path
/// Construct a new instance of `Handle` and add it to the store
pub fn new(stores: &Stores) -> Handle<Self> {
stores.global_curves.insert(GlobalCurve)
}
}
11 changes: 6 additions & 5 deletions crates/fj-kernel/src/objects/edge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::fmt;

use pretty_assertions::{assert_eq, assert_ne};

use crate::stores::Handle;
use crate::stores::{Handle, HandleWrapper};

use super::{Curve, GlobalCurve, GlobalVertex, Vertex};

Expand Down Expand Up @@ -46,8 +46,8 @@ impl HalfEdge {

// Make sure `curve` and `vertices` match `global_form`.
assert_eq!(
curve.global_form(),
global_form.curve(),
curve.global_form().id(),
global_form.curve().id(),
"The global form of a half-edge's curve must match the curve of \
the half-edge's global form"
);
Expand Down Expand Up @@ -110,16 +110,17 @@ impl fmt::Display for HalfEdge {
/// An edge, defined in global (3D) coordinates
#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
pub struct GlobalEdge {
curve: Handle<GlobalCurve>,
curve: HandleWrapper<GlobalCurve>,
vertices: [GlobalVertex; 2],
}

impl GlobalEdge {
/// Create a new instance
pub fn new(
curve: Handle<GlobalCurve>,
curve: impl Into<HandleWrapper<GlobalCurve>>,
vertices: [GlobalVertex; 2],
) -> Self {
let curve = curve.into();
Self { curve, vertices }
}

Expand Down
Loading