Merge pull request #6980 from seadowg/epsilon

grzesiek2010 · web-flow · commit f725a3bf4aa6 · 2025-12-03T10:06:38.000+01:00
Only detect exact collinear intersections in traces
diff --git a/shared/src/main/java/org/odk/collect/shared/geometry/Geometry.kt b/shared/src/main/java/org/odk/collect/shared/geometry/Geometry.kt
@@ -25,21 +25,21 @@ fun Trace.segments(): List<LineSegment> {
 /**
  * Returns `true` if any segment of the trace intersects with any other and `false` otherwise.
  */
-fun Trace.intersects(): Boolean {
+fun Trace.intersects(epsilon: Double = 0.0): Boolean {
     val points = this.points
     return if (points.size >= 3) {
         val segments = segments()
         if (segments.size == 2) {
-            segments[0].intersects(segments[1], allowConnection = true)
+            segments[0].intersects(segments[1], allowConnection = true, epsilon = epsilon)
         } else {
             segments.filterIndexed { line1Index, line1 ->
                 segments.filterIndexed { line2Index, line2 ->
                     if (isClosed() && line1Index == 0 && line2Index == segments.size - 1) {
                         false
                     } else if (line2Index == line1Index + 1) {
-                        line1.intersects(line2, allowConnection = true)
+                        line1.intersects(line2, allowConnection = true, epsilon = epsilon)
                     } else if (line2Index >= line1Index + 2) {
-                        line1.intersects(line2)
+                        line1.intersects(line2, epsilon = epsilon)
                     } else {
                         false
                     }
@@ -73,12 +73,12 @@ fun Point.within(segment: LineSegment): Boolean {
  * @param allowConnection will allow the end of `this` and the start of `other` to intersect
  * provided they are equivalent (the two segments are "connected")
  */
-fun LineSegment.intersects(other: LineSegment, allowConnection: Boolean = false): Boolean {
+fun LineSegment.intersects(other: LineSegment, allowConnection: Boolean = false, epsilon: Double = 0.0): Boolean {
     val (a, b) = this
     val (c, d) = other
 
-    val orientationA = orientation(a, c, d)
-    val orientationD = orientation(a, b, d)
+    val orientationA = orientation(a, c, d, epsilon)
+    val orientationD = orientation(a, b, d, epsilon)
 
     return if (orientationA == Orientation.Collinear && a.within(other)) {
         true
@@ -87,8 +87,8 @@ fun LineSegment.intersects(other: LineSegment, allowConnection: Boolean = false)
     } else if (b == c && allowConnection) {
         false
     } else {
-        val orientationB = orientation(b, c, d)
-        val orientationC = orientation(a, b, c)
+        val orientationB = orientation(b, c, d, epsilon)
+        val orientationC = orientation(a, b, c, epsilon)
 
         if (orientationA.isOpposing(orientationB) && orientationC.isOpposing(orientationD)) {
             true
@@ -102,17 +102,6 @@ fun LineSegment.intersects(other: LineSegment, allowConnection: Boolean = false)
     }
 }
 
-/**
- * Calculate a [Point] on this [LineSegment] based on the `position` using
- * [Linear interpolation](https://en.wikipedia.org/wiki/Linear_interpolation). `0` will return
- * [LineSegment.start] and `1` will return [LineSegment.end].
- */
-fun LineSegment.interpolate(position: Double): Point {
-    val x = start.x + position * (end.x - start.x)
-    val y = start.y + position * (end.y - start.y)
-    return Point(x, y)
-}
-
 /**
  * Calculate the "orientation" (or "direction") of three points using the cross product of the
  * vectors of the pairs of points (see
@@ -122,9 +111,9 @@ fun LineSegment.interpolate(position: Double): Point {
  * @param epsilon the epsilon used to check for collinearity
  *
  */
-fun orientation(a: Point, b: Point, c: Point, epsilon: Double = 0.00000000001): Orientation {
+fun orientation(a: Point, b: Point, c: Point, epsilon: Double = 0.0): Orientation {
     val crossProduct = crossProduct(Pair(b.x - a.x, b.y - a.y), Pair(c.x - a.x, c.y - a.y))
-    return if (abs(crossProduct) < epsilon) {
+    return if (abs(crossProduct) <= epsilon) {
         Orientation.Collinear
     } else if (crossProduct > 0) {
         Orientation.AntiClockwise
diff --git a/shared/src/test/java/org/odk/collect/shared/geometry/GeometryTest.kt b/shared/src/test/java/org/odk/collect/shared/geometry/GeometryTest.kt
@@ -3,6 +3,10 @@ package org.odk.collect.shared.geometry
 import org.hamcrest.MatcherAssert.assertThat
 import org.hamcrest.Matchers.equalTo
 import org.junit.Test
+import org.odk.collect.shared.geometry.support.GeometryTestUtils.addRandomIntersectingSegment
+import org.odk.collect.shared.geometry.support.GeometryTestUtils.getTraceGenerator
+import org.odk.collect.shared.geometry.support.GeometryTestUtils.reverse
+import org.odk.collect.shared.geometry.support.GeometryTestUtils.scale
 import org.odk.collect.shared.quickCheck
 import kotlin.random.Random
 
@@ -214,18 +218,15 @@ class GeometryTest {
             generator = getTraceGenerator()
         ) { trace, intersects ->
             // Check intersects is consistent when trace is reversed
-            val reversedTrace = Trace(trace.points.reversed())
+            val reversedTrace = trace.reverse()
             assertThat(
                 "Expected intersects=$intersects:\n$reversedTrace",
                 reversedTrace.intersects(),
                 equalTo(intersects)
             )
 
             // Check intersects is consistent when trace is scaled
-            val scaleFactor = Random.nextDouble(0.1, 10.0)
-            val scaledTrace = Trace(trace.points.map {
-                Point(it.x * scaleFactor, it.y * scaleFactor)
-            })
+            val scaledTrace = trace.scale(Random.nextDouble(0.1, 10.0))
             assertThat(
                 "Expected intersects=$intersects:\n$scaledTrace",
                 scaledTrace.intersects(),
@@ -234,11 +235,7 @@ class GeometryTest {
 
             // Check adding an intersection makes intersects true
             if (!intersects) {
-                val intersectionSegment = trace.segments().random()
-                val intersectPosition = Random.nextDouble(0.1, 1.0)
-                val intersectionPoint = intersectionSegment.interpolate(intersectPosition)
-                val intersectingTrace =
-                    Trace(trace.points + listOf(trace.points.last(), intersectionPoint))
+                val intersectingTrace = trace.addRandomIntersectingSegment()
                 assertThat(
                     "Expected intersects=true:\n$intersectingTrace",
                     intersectingTrace.intersects(),
@@ -280,46 +277,4 @@ class GeometryTest {
 
         assertThat(segment1.intersects(segment2, allowConnection = true), equalTo(true))
     }
-
-    @Test
-    fun `LineSegment#interpolate returns a point on the segment at a proportional distance`() {
-        val segment = LineSegment(Point(0.0, 0.0), Point(1.0, 0.0))
-
-        assertThat(segment.interpolate(0.0), equalTo(Point(0.0, 0.0)))
-        assertThat(segment.interpolate(0.5), equalTo(Point(0.5, 0.0)))
-        assertThat(segment.interpolate(1.0), equalTo(Point(1.0, 0.0)))
-    }
-
-    @Test
-    fun `LineSegment#interpolate returns a collinear point within the line's bounding box`() {
-        val segment = LineSegment(Point(0.0, 0.0), Point(1.0, 1.0))
-        val interpolatedPoint = segment.interpolate(0.5)
-
-        val orientation = orientation(interpolatedPoint, segment.start, segment.end)
-        assertThat(orientation, equalTo(Orientation.Collinear))
-        assertThat(interpolatedPoint.within(segment), equalTo(true))
-    }
-
-    private fun getTraceGenerator(maxLength: Int = 10, maxCoordinate: Double = 100.0): Sequence<Trace> {
-        return generateSequence {
-            val length = Random.nextInt(2, maxLength)
-            val trace = Trace(0.until(length).map {
-                Point(
-                    Random.nextDouble(maxCoordinate * -1, maxCoordinate),
-                    Random.nextDouble(maxCoordinate * -1, maxCoordinate)
-                )
-            })
-
-            if (trace.isClosed()) {
-                trace
-            } else {
-                val shouldClose = Random.nextBoolean()
-                if (shouldClose) {
-                    trace.copy(points = trace.points + trace.points.first())
-                } else {
-                    trace
-                }
-            }
-        }
-    }
 }
diff --git a/shared/src/test/java/org/odk/collect/shared/geometry/support/GeometryTestUtils.kt b/shared/src/test/java/org/odk/collect/shared/geometry/support/GeometryTestUtils.kt
@@ -0,0 +1,72 @@
+package org.odk.collect.shared.geometry.support
+
+import org.odk.collect.shared.geometry.LineSegment
+import org.odk.collect.shared.geometry.Point
+import org.odk.collect.shared.geometry.Trace
+import org.odk.collect.shared.geometry.segments
+import org.odk.collect.shared.geometry.support.GeometryTestUtils.interpolate
+import kotlin.random.Random
+
+object GeometryTestUtils {
+
+    fun getTraceGenerator(maxLength: Int = 10, maxCoordinate: Double = 100.0): Sequence<Trace> {
+        return generateSequence {
+            val length = Random.nextInt(3, maxLength)
+            val trace = Trace(0.until(length).map {
+                Point(
+                    Random.nextDouble(maxCoordinate * -1, maxCoordinate),
+                    Random.nextDouble(maxCoordinate * -1, maxCoordinate)
+                )
+            })
+
+            if (trace.isClosed()) {
+                trace
+            } else {
+                val shouldClose = Random.nextBoolean()
+                if (shouldClose) {
+                    trace.copy(points = trace.points + trace.points.first())
+                } else {
+                    trace
+                }
+            }
+        }
+    }
+
+    fun Trace.reverse(): Trace {
+        return Trace(points.reversed())
+    }
+
+    fun Trace.scale(factor: Double): Trace {
+        return Trace(points.map {
+            Point(it.x * factor, it.y * factor)
+        })
+    }
+
+    /**
+     * Choose random segment, a random (interpolated) point on that segment and then create a new
+     * trace with an additional point just beyond that to create an intersecting trace.
+     *
+     * Never chooses the last segment as a target for intersecting as that can only create a
+     * collinear intersection which is unlikely to be accurate due to inaccuracies in [interpolate].
+     */
+    fun Trace.addRandomIntersectingSegment(): Trace {
+        val intersectionSegment = segments().dropLast(1).random()
+        val intersectPosition = Random.nextDouble(0.1, 1.0)
+        val intersectionPoint = intersectionSegment.interpolate(intersectPosition)
+        val lineSegment = LineSegment(points.last(), intersectionPoint)
+        val intersectingSegment =
+            LineSegment(lineSegment.start, lineSegment.interpolate(1.1))
+        return Trace(points + intersectingSegment.end)
+    }
+
+    /**
+     * Calculate a [Point] on this [LineSegment] based on the `position` using
+     * [Linear interpolation](https://en.wikipedia.org/wiki/Linear_interpolation). `0` will return
+     * [LineSegment.start] and `1` will return [LineSegment.end].
+     */
+    fun LineSegment.interpolate(position: Double): Point {
+        val x = start.x + position * (end.x - start.x)
+        val y = start.y + position * (end.y - start.y)
+        return Point(x, y)
+    }
+}
diff --git a/shared/src/test/java/org/odk/collect/shared/geometry/support/GeometryTestUtilsTest.kt b/shared/src/test/java/org/odk/collect/shared/geometry/support/GeometryTestUtilsTest.kt
@@ -0,0 +1,43 @@
+package org.odk.collect.shared.geometry.support
+
+import org.hamcrest.MatcherAssert.assertThat
+import org.hamcrest.Matchers.equalTo
+import org.junit.Test
+import org.odk.collect.shared.geometry.LineSegment
+import org.odk.collect.shared.geometry.Orientation
+import org.odk.collect.shared.geometry.Point
+import org.odk.collect.shared.geometry.orientation
+import org.odk.collect.shared.geometry.support.GeometryTestUtils.interpolate
+import org.odk.collect.shared.geometry.within
+
+class GeometryTestUtilsTest {
+
+    @Test
+    fun `LineSegment#interpolate returns a point on the segment at a proportional distance`() {
+        val segment = LineSegment(Point(0.0, 0.0), Point(1.0, 0.0))
+
+        assertThat(segment.interpolate(0.0), equalTo(Point(0.0, 0.0)))
+        assertThat(segment.interpolate(0.5), equalTo(Point(0.5, 0.0)))
+        assertThat(segment.interpolate(1.0), equalTo(Point(1.0, 0.0)))
+    }
+
+    @Test
+    fun `LineSegment#interpolate returns a collinear point within the line's bounding box`() {
+        val segment = LineSegment(Point(0.0, 0.0), Point(1.0, 1.0))
+        val interpolatedPoint = segment.interpolate(0.5)
+
+        val orientation = orientation(interpolatedPoint, segment.start, segment.end)
+        assertThat(orientation, equalTo(Orientation.Collinear))
+        assertThat(interpolatedPoint.within(segment), equalTo(true))
+    }
+
+    @Test
+    fun `LineSegment#interpolate returns a collinear point within the line's bounding box for higher precision points with a suitable epsilon`() {
+        val segment = LineSegment(Point(56.6029153, 20.2311124), Point(56.6029192, 20.2310467))
+        val interpolatedPoint = segment.interpolate(0.5)
+
+        val orientation = orientation(interpolatedPoint, segment.start, segment.end, epsilon = 0.000001)
+        assertThat(orientation, equalTo(Orientation.Collinear))
+        assertThat(interpolatedPoint.within(segment), equalTo(true))
+    }
+}
