From 61f339ba45b20165e2fac9713870236f42cf436f Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 21 Feb 2022 15:05:13 +0100 Subject: [PATCH 1/2] Fix method being too broadly available This implementation (transforming only the canonical form) makes sense for 1-dimensional vertices. If the edge it bounds is transformed, then the 1D form of the vertex doesn't need to be transformed, as the curve, and therefore the vertices whole coordinate system, is transformed with the edge. The canonical form needs to be transformed, so the conversion back into 3D space is correct. However, this method was available for 3D vertices too. In that case, the implementation is incorrect, as the vertex location is identical to its canonical form, and both need to be kept identical. --- src/kernel/topology/vertices.rs | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/kernel/topology/vertices.rs b/src/kernel/topology/vertices.rs index ff121db7c..87c7ce924 100644 --- a/src/kernel/topology/vertices.rs +++ b/src/kernel/topology/vertices.rs @@ -69,6 +69,18 @@ impl Vertex<3> { } } +impl Vertex<1> { + /// Create a transformed vertex + /// + /// The transformed vertex has its canonical form transformed by the + /// transformation provided, but is otherwise identical. + #[must_use] + pub fn transform(mut self, transform: &Transform) -> Self { + self.canonical = transform.transform_point(&self.canonical); + self + } +} + impl Vertex { /// Access the location of this vertex pub fn location(&self) -> &Point { @@ -82,14 +94,4 @@ impl Vertex { canonical: self.canonical, } } - - /// Create a transformed vertex - /// - /// The transformed vertex has its canonical form transformed by the - /// transformation provided, but is otherwise identical. - #[must_use] - pub fn transform(mut self, transform: &Transform) -> Self { - self.canonical = transform.transform_point(&self.canonical); - self - } } From aa96f3eac10dd33f1217a7345b6374f9e830c1a2 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 21 Feb 2022 16:06:46 +0100 Subject: [PATCH 2/2] Update doc comment --- src/kernel/topology/vertices.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/kernel/topology/vertices.rs b/src/kernel/topology/vertices.rs index 87c7ce924..3643d98a5 100644 --- a/src/kernel/topology/vertices.rs +++ b/src/kernel/topology/vertices.rs @@ -72,8 +72,13 @@ impl Vertex<3> { impl Vertex<1> { /// Create a transformed vertex /// - /// The transformed vertex has its canonical form transformed by the - /// transformation provided, but is otherwise identical. + /// This is a 3D transformation that transforms the canonical form of the + /// vertex, but leaves the location untouched. Since `self` is a + /// 1-dimensional vertex, transforming the location is not possible. + /// + /// And, presumably, also not necessary, as this is likely part of a larger + /// transformation that also transforms the curve the vertex is on. Making + /// sure this is the case, is the responsibility of the caller. #[must_use] pub fn transform(mut self, transform: &Transform) -> Self { self.canonical = transform.transform_point(&self.canonical);