Skip to content

Commit

Permalink
Remove ref. to curve from Vertex/PartialVertex
Browse files Browse the repository at this point in the history
This simplifies the object graph, making it less redundant, even making
it possible to remove a now superfluous validation check.
  • Loading branch information
hannobraun committed Jan 18, 2023
1 parent 2521e46 commit a06b0d5
Show file tree
Hide file tree
Showing 9 changed files with 12 additions and 132 deletions.
8 changes: 2 additions & 6 deletions crates/fj-kernel/src/algorithms/sweep/edge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,7 @@ impl Sweep for (Handle<HalfEdge>, Color) {
)
.insert(objects);

Vertex::new(
vertex.position(),
curve.clone(),
surface_vertex,
)
Vertex::new(vertex.position(), surface_vertex)
})
};

Expand Down Expand Up @@ -137,7 +133,7 @@ impl Sweep for (Handle<HalfEdge>, Color) {
.zip(surface_vertices)
.collect::<[_; 2]>()
.map(|(vertex, surface_form)| {
Vertex::new(vertex.position(), curve.clone(), surface_form)
Vertex::new(vertex.position(), surface_form)
});

HalfEdge::new(curve, vertices, global).insert(objects)
Expand Down
10 changes: 1 addition & 9 deletions crates/fj-kernel/src/algorithms/sweep/vertex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,7 @@ impl Sweep for (Vertex, Handle<Surface>) {

// And now the vertices. Again, nothing wild here.
let vertices = vertices_surface.map(|surface_form| {
Vertex::new(
[surface_form.position().v],
curve.clone(),
surface_form,
)
Vertex::new([surface_form.position().v], surface_form)
});

// And finally, creating the output `Edge` is just a matter of
Expand Down Expand Up @@ -177,12 +173,8 @@ mod tests {
..Default::default()
};
curve.update_as_u_axis();
let curve = curve
.build(&mut services.objects)
.insert(&mut services.objects);
let vertex = PartialVertex {
position: Some([0.].into()),
curve: Partial::from(curve),
surface_form: Partial::from_partial(PartialSurfaceVertex {
position: Some(Point::from([0., 0.])),
surface: Partial::from(surface.clone()),
Expand Down
6 changes: 1 addition & 5 deletions crates/fj-kernel/src/algorithms/transform/vertex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,12 @@ impl TransformObject for Vertex {
// coordinates and thus transforming the curve takes care of it.
let position = self.position();

let curve = self
.curve()
.clone()
.transform_with_cache(transform, objects, cache);
let surface_form = self
.surface_form()
.clone()
.transform_with_cache(transform, objects, cache);

Self::new(position, curve, surface_form)
Self::new(position, surface_form)
}
}

Expand Down
2 changes: 0 additions & 2 deletions crates/fj-kernel/src/builder/edge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,6 @@ impl HalfEdgeBuilder for PartialHalfEdge {
self.curve.write().surface = surface.clone();

for (vertex, point) in self.vertices.each_mut_ext().zip_ext(points) {
vertex.curve.write().surface = surface.clone();

let mut surface_form = vertex.surface_form.write();
surface_form.position = Some(point.into());
surface_form.surface = surface.clone();
Expand Down
2 changes: 0 additions & 2 deletions crates/fj-kernel/src/builder/vertex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ pub trait VertexBuilder {
impl VertexBuilder for PartialVertex {
fn replace_surface(&mut self, surface: impl Into<Partial<Surface>>) {
let surface = surface.into();

self.curve.write().surface = surface.clone();
self.surface_form.write().surface = surface;
}
}
Expand Down
13 changes: 1 addition & 12 deletions crates/fj-kernel/src/objects/full/vertex.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
use fj_math::Point;

use crate::{
objects::{Curve, Surface},
storage::Handle,
};
use crate::{objects::Surface, storage::Handle};

/// A vertex
///
Expand All @@ -15,22 +12,19 @@ use crate::{
#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
pub struct Vertex {
position: Point<1>,
curve: Handle<Curve>,
surface_form: Handle<SurfaceVertex>,
}

impl Vertex {
/// Construct an instance of `Vertex`
pub fn new(
position: impl Into<Point<1>>,
curve: Handle<Curve>,
surface_form: Handle<SurfaceVertex>,
) -> Self {
let position = position.into();

Self {
position,
curve,
surface_form,
}
}
Expand All @@ -40,11 +34,6 @@ impl Vertex {
self.position
}

/// Access the curve that the vertex is defined in
pub fn curve(&self) -> &Handle<Curve> {
&self.curve
}

/// Access the surface form of this vertex
pub fn surface_form(&self) -> &Handle<SurfaceVertex> {
&self.surface_form
Expand Down
5 changes: 1 addition & 4 deletions crates/fj-kernel/src/partial/objects/edge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,7 @@ impl PartialObject for PartialHalfEdge {
impl Default for PartialHalfEdge {
fn default() -> Self {
let curve = Partial::<Curve>::new();
let vertices = array::from_fn(|_| PartialVertex {
curve: curve.clone(),
..Default::default()
});
let vertices = array::from_fn(|_| PartialVertex::default());

let global_curve = curve.read().global_form.clone();
let global_vertices =
Expand Down
16 changes: 3 additions & 13 deletions crates/fj-kernel/src/partial/objects/vertex.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use fj_math::Point;

use crate::{
objects::{Curve, GlobalVertex, Objects, Surface, SurfaceVertex, Vertex},
partial::{FullToPartialCache, Partial, PartialCurve, PartialObject},
objects::{GlobalVertex, Objects, Surface, SurfaceVertex, Vertex},
partial::{FullToPartialCache, Partial, PartialObject},
services::Service,
};

Expand All @@ -12,9 +12,6 @@ pub struct PartialVertex {
/// The position of the vertex on the curve
pub position: Option<Point<1>>,

/// The curve that the vertex is defined in
pub curve: Partial<Curve>,

/// The surface form of the vertex
pub surface_form: Partial<SurfaceVertex>,
}
Expand All @@ -25,7 +22,6 @@ impl PartialObject for PartialVertex {
fn from_full(vertex: &Self::Full, cache: &mut FullToPartialCache) -> Self {
Self {
position: Some(vertex.position()),
curve: Partial::from_full(vertex.curve().clone(), cache),
surface_form: Partial::from_full(
vertex.surface_form().clone(),
cache,
Expand All @@ -37,29 +33,23 @@ impl PartialObject for PartialVertex {
let position = self
.position
.expect("Can't build `Vertex` without position");
let curve = self.curve.build(objects);
let surface_form = self.surface_form.build(objects);

Vertex::new(position, curve, surface_form)
Vertex::new(position, surface_form)
}
}

impl Default for PartialVertex {
fn default() -> Self {
let surface = Partial::new();

let curve = Partial::from_partial(PartialCurve {
surface: surface.clone(),
..Default::default()
});
let surface_form = Partial::from_partial(PartialSurfaceVertex {
surface,
..Default::default()
});

Self {
position: None,
curve,
surface_form,
}
}
Expand Down
82 changes: 3 additions & 79 deletions crates/fj-kernel/src/validate/edge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ use fj_math::{Point, Scalar};

use crate::{
objects::{
Curve, GlobalCurve, GlobalEdge, GlobalVertex, HalfEdge, Surface,
Vertex, VerticesInNormalizedOrder,
GlobalCurve, GlobalEdge, GlobalVertex, HalfEdge, Surface, Vertex,
VerticesInNormalizedOrder,
},
storage::Handle,
};
Expand All @@ -17,7 +17,6 @@ impl Validate for HalfEdge {
config: &ValidationConfig,
errors: &mut Vec<ValidationError>,
) {
HalfEdgeValidationError::check_curve_identity(self, errors);
HalfEdgeValidationError::check_global_curve_identity(self, errors);
HalfEdgeValidationError::check_global_vertex_identity(self, errors);
HalfEdgeValidationError::check_surface_identity(self, errors);
Expand All @@ -42,24 +41,6 @@ impl Validate for GlobalEdge {
/// [`HalfEdge`] validation failed
#[derive(Clone, Debug, thiserror::Error)]
pub enum HalfEdgeValidationError {
/// [`HalfEdge`] vertices are not defined on the same `Curve`
#[error(
"`HalfEdge` vertices are not defined on the same `Curve`\n\
- `Curve` of back vertex: {back_curve:#?}\n\
- `Curve` of front vertex: {front_curve:#?}\n\
- `HalfEdge`: {half_edge:#?}"
)]
CurveMismatch {
/// The curve of the [`HalfEdge`]'s back vertex
back_curve: Handle<Curve>,

/// The curve of the [`HalfEdge`]'s front vertex
front_curve: Handle<Curve>,

/// The half-edge
half_edge: HalfEdge,
},

/// [`HalfEdge`]'s [`GlobalCurve`]s do not match
#[error(
"Global form of `HalfEdge`'s `Curve` does not match `GlobalCurve` of \n\
Expand All @@ -69,7 +50,7 @@ pub enum HalfEdgeValidationError {
- `HalfEdge`: {half_edge:#?}",
)]
GlobalCurveMismatch {
/// The [`GlobalCurve`] from the [`HalfEdge`]'s [`Curve`]
/// The [`GlobalCurve`] from the [`HalfEdge`]'s `Curve`
global_curve_from_curve: Handle<GlobalCurve>,

/// The [`GlobalCurve`] from the [`HalfEdge`]'s global form
Expand Down Expand Up @@ -155,25 +136,6 @@ pub enum HalfEdgeValidationError {
}

impl HalfEdgeValidationError {
fn check_curve_identity(
half_edge: &HalfEdge,
errors: &mut Vec<ValidationError>,
) {
let back_curve = half_edge.back().curve();
let front_curve = half_edge.front().curve();

if back_curve.id() != front_curve.id() {
errors.push(
Box::new(Self::CurveMismatch {
back_curve: back_curve.clone(),
front_curve: front_curve.clone(),
half_edge: half_edge.clone(),
})
.into(),
);
}
}

fn check_global_curve_identity(
half_edge: &HalfEdge,
errors: &mut Vec<ValidationError>,
Expand Down Expand Up @@ -322,44 +284,6 @@ mod tests {
validate::Validate,
};

#[test]
fn half_edge_curve_mismatch() -> anyhow::Result<()> {
let mut services = Services::new();

let valid = {
let mut half_edge = PartialHalfEdge::default();
half_edge.update_as_line_segment_from_points(
services.objects.surfaces.xy_plane(),
[[0., 0.], [1., 0.]],
);

half_edge.build(&mut services.objects)
};
let invalid = {
let mut vertices = valid.vertices().clone();
let mut vertex = PartialVertex::from_full(
&vertices[1],
&mut FullToPartialCache::default(),
);
// Arranging for an equal but not identical curve here.
vertex.curve = Partial::from_partial(
Partial::from(valid.curve().clone()).read().clone(),
);
vertices[1] = vertex.build(&mut services.objects);

HalfEdge::new(
valid.curve().clone(),
vertices,
valid.global_form().clone(),
)
};

valid.validate_and_return_first_error()?;
assert!(invalid.validate_and_return_first_error().is_err());

Ok(())
}

#[test]
fn half_edge_global_curve_mismatch() -> anyhow::Result<()> {
let mut services = Services::new();
Expand Down

0 comments on commit a06b0d5

Please sign in to comment.