fixup! Introduce the geomgraph module for DE-9IM Relate trait

geo/src/algorithm/relate/geomgraph/edge.rs

@@ -101,9 +101,10 @@ impl<F: GeoFloat> Edge<F> {
         }
     }
 
-    /// Add an EdgeIntersection for intersection intIndex.
+    /// Add an EdgeIntersection for `intersection`.
+    ///
     /// An intersection that falls exactly on a vertex of the edge is normalized to use the higher
-    /// of the two possible segmentIndexes
+    /// of the two possible `segment_index`
     pub fn add_intersection(
         &mut self,
         intersection_coord: Coordinate<F>,
@@ -128,20 +129,24 @@ impl<F: GeoFloat> Edge<F> {
             distance,
         ));
     }
+
+    /// Update the IM with the contribution for this component.
+    ///
+    /// A component only contributes if it has a labelling for both parent geometries
     pub fn update_intersection_matrix(label: &Label, intersection_matrix: &mut IntersectionMatrix) {
-        intersection_matrix.set_at_least_if_valid(
+        intersection_matrix.set_at_least_if_in_both(
             label.position(0, Direction::On),
             label.position(1, Direction::On),
             Dimensions::OneDimensional,
         );
 
         if label.is_area() {
-            intersection_matrix.set_at_least_if_valid(
+            intersection_matrix.set_at_least_if_in_both(
                 label.position(0, Direction::Left),
                 label.position(1, Direction::Left),
                 Dimensions::TwoDimensional,
             );
-            intersection_matrix.set_at_least_if_valid(
+            intersection_matrix.set_at_least_if_in_both(
                 label.position(0, Direction::Right),
                 label.position(1, Direction::Right),
                 Dimensions::TwoDimensional,

geo/src/algorithm/relate/geomgraph/geometry_graph.rs

@@ -265,7 +265,7 @@ where
     /// intersection tests.  (E.g. rings are not tested for self-intersection, since they are
     /// assumed to be valid).
     ///
-    /// `li` the [`LineIntersector`] to use to determine intersection
+    /// `line_intersector` the [`LineIntersector`] to use to determine intersection
     pub fn compute_self_nodes(
         &mut self,
         line_intersector: Box<dyn LineIntersector<F>>,
@@ -341,6 +341,7 @@ where
         let new_position = Self::determine_boundary(boundary_count);
         label.set_on_position(arg_index, new_position);
     }
+
     fn add_self_intersection_nodes(&mut self) {
         let positions_and_intersections: Vec<(CoordPos, Vec<Coordinate<F>>)> = self
             .edges()

geo/src/algorithm/relate/geomgraph/index/segment_intersector.rs

@@ -94,6 +94,7 @@ where
 
         false
     }
+
     pub fn add_intersections(
         &mut self,
         edge0: &RefCell<Edge<F>>,
@@ -170,8 +171,5 @@ where
                 .any(|node| intersection == node.coordinate()),
             None => false,
         }
-
-        // is_boundary(intersection, &boundary_nodes[0])
-        //     || self.is_boundary_point_internal(intersection, &boundary_nodes[1])
     }
 }

geo/src/algorithm/relate/geomgraph/intersection_matrix.rs

@@ -24,8 +24,16 @@ use crate::algorithm::dimensions::Dimensions;
 #[derive(PartialEq, Eq)]
 pub struct IntersectionMatrix(LocationArray<LocationArray<Dimensions>>);
 
+/// Helper struct so we can index IntersectionMatrix by CoordPos
+///
+/// CoordPos enum members are ordered: OnBondary, Inside, Outside
+/// DE-9IM matrices are ordered: Inside, Boundary, Exterior
+///
+/// So we can't simply `CoordPos as usize` without losing the conventional ordering
+/// of elements, which is useful for debug / interop.
 #[derive(PartialEq, Eq, Clone, Copy)]
 struct LocationArray<T>([T; 3]);
+
 impl<T> LocationArray<T> {
     fn iter(&self) -> impl Iterator<Item = &T> {
         self.0.iter()
@@ -55,18 +63,18 @@ impl<T> std::ops::IndexMut<CoordPos> for LocationArray<T> {
 }
 
 #[derive(Debug)]
-pub struct InvalidInput {
+pub struct InvalidInputError {
     message: String,
 }
 
-impl InvalidInput {
+impl InvalidInputError {
     fn new(message: String) -> Self {
         Self { message }
     }
 }
 
-impl std::error::Error for InvalidInput {}
-impl std::fmt::Display for InvalidInput {
+impl std::error::Error for InvalidInputError {}
+impl std::fmt::Display for InvalidInputError {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         write!(f, "invalid input:  {}", self.message)
     }
@@ -98,46 +106,65 @@ impl IntersectionMatrix {
         IntersectionMatrix(LocationArray([LocationArray([Dimensions::Empty; 3]); 3]))
     }
 
+    /// Set `dimensions` of the cell specified by the positions.
+    ///
+    /// `position_a`: which position `dimensions` applies to within the first geometry
+    /// `position_b`: which position `dimensions` applies to within the second geometry
+    /// `dimensions`: the dimension of the incident
     pub(crate) fn set(
         &mut self,
         position_a: CoordPos,
         position_b: CoordPos,
-        dimensionality: Dimensions,
+        dimensions: Dimensions,
     ) {
-        self.0[position_a][position_b] = dimensionality;
+        self.0[position_a][position_b] = dimensions;
     }
 
+    /// Reports an incident of `dimensions`, which updates the IntersectionMatrix if it's greater
+    /// than what has been reported so far.
+    ///
+    /// `position_a`: which position `minimum_dimensions` applies to within the first geometry
+    /// `position_b`: which position `minimum_dimensions` applies to within the second geometry
+    /// `minimum_dimensions`: the dimension of the incident
     pub(crate) fn set_at_least(
         &mut self,
         position_a: CoordPos,
         position_b: CoordPos,
-        minimum_dimension_value: Dimensions,
+        minimum_dimensions: Dimensions,
     ) {
-        if self.0[position_a][position_b] < minimum_dimension_value {
-            self.0[position_a][position_b] = minimum_dimension_value;
+        if self.0[position_a][position_b] < minimum_dimensions {
+            self.0[position_a][position_b] = minimum_dimensions;
         }
     }
 
-    pub(crate) fn set_at_least_if_valid(
+    /// If both geometries have `Some` position, then changes the specified element to at
+    /// least `minimum_dimensions`.
+    ///
+    /// Else, if either is none, do nothing.
+    ///
+    /// `position_a`: which position `minimum_dimensions` applies to within the first geometry, or
+    ///               `None` if the dimension was not incident with the first geometry.
+    /// `position_b`: which position `minimum_dimensions` applies to within the second geometry, or
+    ///               `None` if the dimension was not incident with the second geometry.
+    /// `minimum_dimensions`: the dimension of the incident
+    pub(crate) fn set_at_least_if_in_both(
         &mut self,
         position_a: Option<CoordPos>,
         position_b: Option<CoordPos>,
-        minimum_dimension_value: Dimensions,
+        minimum_dimensions: Dimensions,
     ) {
-        if let Some(position_a) = position_a {
-            if let Some(position_b) = position_b {
-                self.set_at_least(position_a, position_b, minimum_dimension_value);
-            }
+        if let (Some(position_a), Some(position_b)) = (position_a, position_b) {
+            self.set_at_least(position_a, position_b, minimum_dimensions);
         }
     }
 
     pub(crate) fn set_at_least_from_string(
         &mut self,
         dimensions: &str,
-    ) -> Result<(), InvalidInput> {
+    ) -> Result<(), InvalidInputError> {
         if dimensions.len() != 9 {
             let message = format!("Expected dimensions length 9, found: {}", dimensions.len());
-            return Err(InvalidInput::new(message));
+            return Err(InvalidInputError::new(message));
         }
 
         let mut chars = dimensions.chars();
@@ -151,7 +178,7 @@ impl IntersectionMatrix {
                     other => {
                         let message = format!("expected '0', '1', '2', or 'F'. Found: {}", other);
                         // We should make this return Err if this method becomes public
-                        return Err(InvalidInput::new(message));
+                        return Err(InvalidInputError::new(message));
                     }
                 }
             }
@@ -197,7 +224,7 @@ impl IntersectionMatrix {
 }
 
 impl std::str::FromStr for IntersectionMatrix {
-    type Err = InvalidInput;
+    type Err = InvalidInputError;
     fn from_str(str: &str) -> Result<Self, Self::Err> {
         let mut im = IntersectionMatrix::empty();
         im.set_at_least_from_string(str)?;

geo/src/algorithm/relate/geomgraph/label.rs

@@ -2,18 +2,17 @@ use super::{CoordPos, Direction, TopologyPosition};
 
 use std::fmt;
 
-/// A `Label` indicates the topological relationship of a node or edge of a topology graph to a given
-/// [`Geometry`].
+/// A GeometryGraph has components (nodes and edges) which are labeled with their topological
+/// relations to the geometries.
 ///
-/// Topology graphs support the concept of labeling nodes and edges in the graph.  The label of a
-/// node or edge specifies its topological relationship to one or more geometries.  A label
-/// for a node or edge has one or two elements, depending on whether the node or edge occurs in one
-/// or both of the input `Geometry`s.
+/// More precisely, each `Label` holds a `TopologyPosition` for each geometry that states whether
+/// the node or edge being labeled occurs `Inside`, `Outside`, or `OnBoundary` of the geometry.
 ///
-/// Elements contain attributes which categorize the topological
-/// location of the node or edge relative to the parent `Geometry`; that is, whether the node or
-/// edge is in the interior, boundary or exterior of the `Geometry`.  Attributes have a value
-/// from the set `{Inside, OnBoundary, Outside}`.
+/// For lines and points, a `TopologyPosition` tracks only an `On` position,
+/// while areas have positions for `On`, `Left`, and `Right`.
+///
+/// If the component has *no* incidence with one of the geometries, than the `Label`'s
+/// `TopologyPosition` for that geometry is called `empty`.
 #[derive(Clone)]
 pub(crate) struct Label {
     geometry_topologies: [TopologyPosition; 2],

geo/src/algorithm/relate/geomgraph/node.rs

@@ -52,14 +52,13 @@ where
         };
         self.label.set_on_position(geom_index, new_position);
     }
-}
 
-impl<F: GeoFloat> CoordNode<F> {
-    // from JTS#GraphComponent - seems like only node uses this impl, so implementing it directly
-    // onto node rather than the GraphComponent trait
+    // In JTS this method is implemented on a `GraphComponent` superclass, but since it's only used
+    // by this one "subclass" I've implemented it directly on the node, rather than introducing
+    // something like a `GraphComponent` trait
     pub fn update_intersection_matrix(&self, intersection_matrix: &mut IntersectionMatrix) {
         assert!(self.label.geometry_count() >= 2, "found partial label");
-        intersection_matrix.set_at_least_if_valid(
+        intersection_matrix.set_at_least_if_in_both(
             self.label.on_position(0),
             self.label.on_position(1),
             Dimensions::ZeroDimensional,
@@ -69,6 +68,4 @@ impl<F: GeoFloat> CoordNode<F> {
             intersection_matrix, self
         );
     }
-
-    // from JTS#RelateNode, which we've squashed into the base Node class to avoid wrestling OO hierarchies into rust.
 }

geo/src/algorithm/relate/geomgraph/quadrant.rs

@@ -20,18 +20,13 @@ impl Quadrant {
         if dx.is_zero() && dy.is_zero() {
             return None;
         }
-        if dx >= F::zero() {
-            if dy >= F::zero() {
-                Some(Quadrant::NE)
-            } else {
-                Some(Quadrant::SE)
-            }
-        } else {
-            if dy >= F::zero() {
-                Some(Quadrant::NW)
-            } else {
-                Some(Quadrant::SW)
-            }
+
+        match (dy >= F::zero(), dx >= F::zero()) {
+            (true, true) => Quadrant::NE,
+            (true, false) => Quadrant::NW,
+            (false, false) => Quadrant::SW,
+            (false, true) => Quadrant::SE,
         }
+        .into()
     }
 }