From a4cfb489037c3c4f0e38e3890e1eefeec7f3cd4e Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 27 Jun 2024 20:06:50 +0200 Subject: [PATCH 1/4] Update module name --- .../fj-core/src/operations/transform/{edge.rs => half_edge.rs} | 0 crates/fj-core/src/operations/transform/mod.rs | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename crates/fj-core/src/operations/transform/{edge.rs => half_edge.rs} (100%) diff --git a/crates/fj-core/src/operations/transform/edge.rs b/crates/fj-core/src/operations/transform/half_edge.rs similarity index 100% rename from crates/fj-core/src/operations/transform/edge.rs rename to crates/fj-core/src/operations/transform/half_edge.rs diff --git a/crates/fj-core/src/operations/transform/mod.rs b/crates/fj-core/src/operations/transform/mod.rs index 507a598c9..5e25a455b 100644 --- a/crates/fj-core/src/operations/transform/mod.rs +++ b/crates/fj-core/src/operations/transform/mod.rs @@ -2,8 +2,8 @@ mod curve; mod cycle; -mod edge; mod face; +mod half_edge; mod region; mod shell; mod solid; From 48586c15b571e42ac54abe6d1523d4f363c651f3 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 27 Jun 2024 20:16:53 +0200 Subject: [PATCH 2/4] Prepare for follow-on change --- crates/fj-core/src/operations/transform/cycle.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/crates/fj-core/src/operations/transform/cycle.rs b/crates/fj-core/src/operations/transform/cycle.rs index 9b702278b..828039a9c 100644 --- a/crates/fj-core/src/operations/transform/cycle.rs +++ b/crates/fj-core/src/operations/transform/cycle.rs @@ -20,9 +20,14 @@ impl TransformObject for (&Handle, &Handle) { ) -> Self::Transformed { let (cycle, surface) = self; - let half_edges = cycle.half_edges().iter().map(|half_edge| { - (half_edge, surface).transform_with_cache(transform, core, cache) - }); + let half_edges = cycle + .half_edges() + .iter() + .map(|half_edge| { + (half_edge, surface) + .transform_with_cache(transform, core, cache) + }) + .collect::>(); Cycle::new(half_edges).insert(core) } From 42e39bb668b6d30129749fb4c58f8ad33f5f9742 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 27 Jun 2024 20:17:23 +0200 Subject: [PATCH 3/4] Prepare for follow-on change --- crates/fj-core/src/operations/transform/cycle.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fj-core/src/operations/transform/cycle.rs b/crates/fj-core/src/operations/transform/cycle.rs index 828039a9c..2808c2a78 100644 --- a/crates/fj-core/src/operations/transform/cycle.rs +++ b/crates/fj-core/src/operations/transform/cycle.rs @@ -22,8 +22,8 @@ impl TransformObject for (&Handle, &Handle) { let half_edges = cycle .half_edges() - .iter() - .map(|half_edge| { + .pairs() + .map(|(half_edge, _)| { (half_edge, surface) .transform_with_cache(transform, core, cache) }) From f4619258e72b5d687106848565979ad472a8fbb9 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 27 Jun 2024 20:24:48 +0200 Subject: [PATCH 4/4] Update vertex geometry in transform code --- .../fj-core/src/operations/transform/cycle.rs | 77 ++++++++++++++++++- 1 file changed, 73 insertions(+), 4 deletions(-) diff --git a/crates/fj-core/src/operations/transform/cycle.rs b/crates/fj-core/src/operations/transform/cycle.rs index 2808c2a78..562a9f3e9 100644 --- a/crates/fj-core/src/operations/transform/cycle.rs +++ b/crates/fj-core/src/operations/transform/cycle.rs @@ -1,4 +1,5 @@ use fj_math::Transform; +use itertools::Itertools; use crate::{ operations::insert::Insert, @@ -20,15 +21,83 @@ impl TransformObject for (&Handle, &Handle) { ) -> Self::Transformed { let (cycle, surface) = self; - let half_edges = cycle + let half_edges_and_old_vertex_geometries = cycle .half_edges() .pairs() - .map(|(half_edge, _)| { - (half_edge, surface) - .transform_with_cache(transform, core, cache) + .map(|(half_edge, next_half_edge)| { + let vertex_a_geom = core + .layers + .geometry + .of_vertex(half_edge.start_vertex()) + .unwrap() + .local_on(half_edge.curve()) + .unwrap() + .clone(); + let vertex_b_geom = core + .layers + .geometry + .of_vertex(next_half_edge.start_vertex()) + .unwrap() + .local_on(half_edge.curve()) + .unwrap() + .clone(); + + let half_edge = (half_edge, surface) + .transform_with_cache(transform, core, cache); + + (half_edge, vertex_a_geom, vertex_b_geom) }) .collect::>(); + // That we're transforming the vertex geometry here, instead of down in + // the vertex transform implementation, presents an inconsistency in the + // transform architecture. But if we were to transform it down there, + // we'd have to pass all kids of information down there (the half-edge + // transform code would need the half-edge's end vertex, and we would + // need to pass the curve into the vertex transform code). + // + // Doing it here is easier. It might be a hack, but so be it. I have two + // reasons for accepting this, instead of doing it "the right way" (if + // what I've described in the previous paragraph actually is the right + // thing to do): + // + // - At some point, it will no longer be necessary to define vertex + // geometry redundantly in every local curve space. Then this whole + // situation will be easier to handle. + // - With geometry living in a separate layer now, it's unclear if the + // current approach to transforms is the right one anyway. So far, + // we're still creating new topological objects with each + // transformation, but whether this is desirable is not obvious. + // + // When we have progress on those two points, whatever the "right way" + // is will have changed anyway, so I'm not too worried about a hack here + // or there. + let half_edges = half_edges_and_old_vertex_geometries + .into_iter() + .circular_tuple_windows() + .map( + |( + (half_edge, vertex_a_geom, vertex_b_geom), + (next_half_edge, _, _), + )| { + // We have only transformed the vertices in 3D space. They + // still have the same positions in local curve space. We + // just have to copy those over to the new vertices. + core.layers.geometry.define_vertex( + half_edge.start_vertex().clone(), + half_edge.curve().clone(), + vertex_a_geom, + ); + core.layers.geometry.define_vertex( + next_half_edge.start_vertex().clone(), + half_edge.curve().clone(), + vertex_b_geom, + ); + + half_edge + }, + ); + Cycle::new(half_edges).insert(core) } }