From 1c5868b9e4a2ff129a63412de1b4b86c5bbbb3e0 Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Mon, 10 Nov 2025 20:15:00 +0000 Subject: [PATCH 01/43] Add intersects util --- .../odk/collect/geo/geopoly/GeoPolyUtils.kt | 93 +++++++++++++++++++ .../collect/geo/geopoly/GeoPolyUtilsTest.kt | 62 +++++++++++++ 2 files changed, 155 insertions(+) create mode 100644 geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt create mode 100644 geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt diff --git a/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt b/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt new file mode 100644 index 00000000000..ed02e3186d5 --- /dev/null +++ b/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt @@ -0,0 +1,93 @@ +package org.odk.collect.geo.geopoly + +import org.odk.collect.maps.MapPoint + +object GeoPolyUtils { + + /** + * Returns `true` if any segment of the trace intersects with any other and `false` otherwise. + */ + fun intersects(trace: List): Boolean { + return if (trace.size >= 3) { + val segments = getSegments(trace) + segments.any { line1 -> + segments.any { line2 -> + intersects(line1, line2) + } + } + } else { + false + } + } + + private fun getSegments(trace: List): List> { + return trace.flatMapIndexed { index, point -> + if (index != trace.size - 1) { + listOf(Pair(point, trace[index + 1])) + } else { + emptyList() + } + } + } + + /** + * Work out whether two line segments intersect by calculating if the endpoints of one segment + * are on opposite sides of the other segment **and** vice versa. This is determined by finding + * the orientation of endpoints relative to the other line. + */ + private fun intersects( + segment1: Pair, + segment2: Pair + ): Boolean { + return orientation(segment1.first, segment2.first, segment2.second) + .isOpposing(orientation(segment1.second, segment2.first, segment2.second)) && + orientation(segment1.first, segment1.second, segment2.first) + .isOpposing(orientation(segment1.first, segment1.second, segment2.second)) + } + + /** + * Calculate the "orientation" (or "direction") of three points using the cross product of the + * vectors of the pairs of points (see + * [here](https://en.wikipedia.org/wiki/Cross_product#Computational_geometry)). This can + * either be clockwise, anticlockwise or collinear (the three points form a straight line). + * + */ + private fun orientation(a: MapPoint, b: MapPoint, c: MapPoint): Orientation { + val ab = Pair(b.latitude - a.latitude, b.longitude - a.longitude) + val ac = Pair(c.latitude - a.latitude, c.longitude - a.longitude) + val crossProduct = crossProduct(ab, ac) + + return if (crossProduct > 0) { + Orientation.AntiClockwise + } else if (crossProduct < 0) { + Orientation.Clockwise + } else { + Orientation.Collinear + } + } + + /** + * [https://en.wikipedia.org/wiki/Cross_product](https://en.wikipedia.org/wiki/Cross_product) + */ + private fun crossProduct(x: Pair, y: Pair): Double { + return (x.first * y.second) - (y.first * x.second) + } + + private enum class Orientation { + Collinear, + Clockwise, + AntiClockwise; + + fun isOpposing(other: Orientation): Boolean { + return if (this == Collinear) { + false + } else if (this == Clockwise && other == AntiClockwise) { + true + } else if (this == AntiClockwise && other == Clockwise) { + true + } else { + false + } + } + } +} diff --git a/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt b/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt new file mode 100644 index 00000000000..d464905f232 --- /dev/null +++ b/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt @@ -0,0 +1,62 @@ +package org.odk.collect.geo.geopoly + +import org.hamcrest.MatcherAssert.assertThat +import org.hamcrest.Matchers.equalTo +import org.junit.Test +import org.odk.collect.maps.MapPoint + +class GeoPolyUtilsTest { + + @Test + fun `#intersects returns false for an empty list`() { + assertThat(GeoPolyUtils.intersects(emptyList()), equalTo(false)) + } + + @Test + fun `#intersects returns false when there is only one point`() { + val trace = listOf(MapPoint(0.0, 0.0)) + assertThat(GeoPolyUtils.intersects(trace), equalTo(false)) + } + + @Test + fun `#intersects returns false when there is only one segment`() { + val trace = listOf(MapPoint(0.0, 0.0), MapPoint(1.0, 0.0)) + assertThat(GeoPolyUtils.intersects(trace), equalTo(false)) + } + + @Test + fun `#intersects returns false when no segment intersects with another`() { + val trace = listOf( + MapPoint(0.0, 0.0), + MapPoint(1.0, 1.0), + MapPoint(2.0, 0.0) + ) + + assertThat(GeoPolyUtils.intersects(trace), equalTo(false)) + } + + @Test + fun `#intersects returns false when no segment intersects with another in a closed trace`() { + val trace = listOf( + MapPoint(0.0, 0.0), + MapPoint(1.0, 1.0), + MapPoint(2.0, 0.0), + MapPoint(0.0, 0.0) + ) + + assertThat(GeoPolyUtils.intersects(trace), equalTo(false)) + } + + @Test + fun `#intersects returns true when a segment intersects with another`() { + val trace = listOf( + MapPoint(1.0, 1.0), + MapPoint(1.0, 3.0), + MapPoint(2.0, 3.0), + MapPoint(2.0, 2.0), + MapPoint(0.0, 2.0) + ) + + assertThat(GeoPolyUtils.intersects(trace), equalTo(true)) + } +} From fd4a6a10542386a87be89e3f398080bd78c69552 Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Tue, 11 Nov 2025 11:03:30 +0000 Subject: [PATCH 02/43] Add intersects function handler --- .../android/widgets/GeoPointMapWidget.java | 4 +- .../android/widgets/GeoPointWidget.java | 4 +- .../items/SelectOneFromMapDialogFragment.kt | 4 +- .../utilities/ActivityGeoDataRequester.kt | 7 +- .../widgets/utilities/GeoWidgetUtils.kt | 34 ------- .../widgets/GeoPointMapWidgetTest.java | 3 +- .../widgets/utilities/GeoWidgetUtilsTest.kt | 4 +- .../javarosa/IntersectsFunctionHandlerTest.kt | 90 +++++++++++++++++++ geo/build.gradle.kts | 4 + .../odk/collect/geo/geopoly/GeoPolyUtils.kt | 34 +++++++ .../geo/javarosa/IntersectsFunctionHandler.kt | 33 +++++++ 11 files changed, 178 insertions(+), 43 deletions(-) create mode 100644 collect_app/src/test/java/org/odk/collect/geo/javarosa/IntersectsFunctionHandlerTest.kt create mode 100644 geo/src/main/java/org/odk/collect/geo/javarosa/IntersectsFunctionHandler.kt diff --git a/collect_app/src/main/java/org/odk/collect/android/widgets/GeoPointMapWidget.java b/collect_app/src/main/java/org/odk/collect/android/widgets/GeoPointMapWidget.java index 7ff19d2a30a..56f3a13ceb8 100644 --- a/collect_app/src/main/java/org/odk/collect/android/widgets/GeoPointMapWidget.java +++ b/collect_app/src/main/java/org/odk/collect/android/widgets/GeoPointMapWidget.java @@ -14,6 +14,8 @@ package org.odk.collect.android.widgets; +import static org.odk.collect.geo.geopoly.GeoPolyUtils.parseGeometryPoint; + import android.annotation.SuppressLint; import android.app.Activity; import android.content.Context; @@ -83,7 +85,7 @@ protected View onCreateWidgetView(Context context, FormEntryPrompt prompt, int a @Override public IAnswerData getAnswer() { - double[] parsedGeometryPoint = GeoWidgetUtils.parseGeometryPoint(answerText); + double[] parsedGeometryPoint = parseGeometryPoint(answerText); return parsedGeometryPoint == null ? null : new GeoPointData(parsedGeometryPoint); diff --git a/collect_app/src/main/java/org/odk/collect/android/widgets/GeoPointWidget.java b/collect_app/src/main/java/org/odk/collect/android/widgets/GeoPointWidget.java index 2df65875be9..cdec3ddcf52 100644 --- a/collect_app/src/main/java/org/odk/collect/android/widgets/GeoPointWidget.java +++ b/collect_app/src/main/java/org/odk/collect/android/widgets/GeoPointWidget.java @@ -14,6 +14,8 @@ package org.odk.collect.android.widgets; +import static org.odk.collect.geo.geopoly.GeoPolyUtils.parseGeometryPoint; + import android.annotation.SuppressLint; import android.app.Activity; import android.content.Context; @@ -76,7 +78,7 @@ protected View onCreateWidgetView(Context context, FormEntryPrompt prompt, int a @Override public IAnswerData getAnswer() { - double[] parsedGeometryPoint = GeoWidgetUtils.parseGeometryPoint(answerText); + double[] parsedGeometryPoint = parseGeometryPoint(answerText); return parsedGeometryPoint == null ? null : new GeoPointData(parsedGeometryPoint); diff --git a/collect_app/src/main/java/org/odk/collect/android/widgets/items/SelectOneFromMapDialogFragment.kt b/collect_app/src/main/java/org/odk/collect/android/widgets/items/SelectOneFromMapDialogFragment.kt index 9af41bbce87..99f7c3f38e0 100644 --- a/collect_app/src/main/java/org/odk/collect/android/widgets/items/SelectOneFromMapDialogFragment.kt +++ b/collect_app/src/main/java/org/odk/collect/android/widgets/items/SelectOneFromMapDialogFragment.kt @@ -27,6 +27,8 @@ import org.odk.collect.androidshared.livedata.NonNullLiveData import org.odk.collect.androidshared.ui.FragmentFactoryBuilder import org.odk.collect.async.Scheduler import org.odk.collect.entities.javarosa.parse.EntitySchema +import org.odk.collect.geo.geopoly.GeoPolyUtils +import org.odk.collect.geo.geopoly.GeoPolyUtils.parseGeometry import org.odk.collect.geo.selection.IconifiedText import org.odk.collect.geo.selection.MappableSelectItem import org.odk.collect.geo.selection.SelectionMapData @@ -139,7 +141,7 @@ internal class SelectChoicesMapData( if (geometry != null) { try { - val points = GeoWidgetUtils.parseGeometry(geometry) + val points = parseGeometry(geometry) if (points.isNotEmpty()) { val withinBounds = points.all { GeoWidgetUtils.isWithinMapBounds(it) diff --git a/collect_app/src/main/java/org/odk/collect/android/widgets/utilities/ActivityGeoDataRequester.kt b/collect_app/src/main/java/org/odk/collect/android/widgets/utilities/ActivityGeoDataRequester.kt index 8ac033c9de8..8d050d17dc9 100644 --- a/collect_app/src/main/java/org/odk/collect/android/widgets/utilities/ActivityGeoDataRequester.kt +++ b/collect_app/src/main/java/org/odk/collect/android/widgets/utilities/ActivityGeoDataRequester.kt @@ -14,6 +14,7 @@ import org.odk.collect.geo.Constants.EXTRA_RETAIN_MOCK_ACCURACY import org.odk.collect.geo.geopoint.GeoPointActivity import org.odk.collect.geo.geopoint.GeoPointMapActivity import org.odk.collect.geo.geopoly.GeoPolyActivity +import org.odk.collect.geo.geopoly.GeoPolyUtils.parseGeometry import org.odk.collect.permissions.PermissionListener import org.odk.collect.permissions.PermissionsProvider import java.lang.Boolean.parseBoolean @@ -35,7 +36,7 @@ class ActivityGeoDataRequester( waitingForDataRegistry.waitForData(prompt.index) val bundle = Bundle().also { - val parsedGeometry = GeoWidgetUtils.parseGeometry(answerText) + val parsedGeometry = parseGeometry(answerText) if (parsedGeometry.isNotEmpty()) { it.putParcelable( GeoPointMapActivity.EXTRA_LOCATION, @@ -97,7 +98,7 @@ class ActivityGeoDataRequester( val intent = Intent(activity, GeoPolyActivity::class.java).also { it.putExtra( GeoPolyActivity.EXTRA_POLYGON, - GeoWidgetUtils.parseGeometry(answerText) + parseGeometry(answerText) ) it.putExtra( GeoPolyActivity.OUTPUT_MODE_KEY, @@ -130,7 +131,7 @@ class ActivityGeoDataRequester( val intent = Intent(activity, GeoPolyActivity::class.java).also { it.putExtra( GeoPolyActivity.EXTRA_POLYGON, - GeoWidgetUtils.parseGeometry(answerText) + parseGeometry(answerText) ) it.putExtra( GeoPolyActivity.OUTPUT_MODE_KEY, diff --git a/collect_app/src/main/java/org/odk/collect/android/widgets/utilities/GeoWidgetUtils.kt b/collect_app/src/main/java/org/odk/collect/android/widgets/utilities/GeoWidgetUtils.kt index ed904352eb3..36fe116b88b 100644 --- a/collect_app/src/main/java/org/odk/collect/android/widgets/utilities/GeoWidgetUtils.kt +++ b/collect_app/src/main/java/org/odk/collect/android/widgets/utilities/GeoWidgetUtils.kt @@ -41,40 +41,6 @@ object GeoWidgetUtils { } } - @JvmStatic - fun parseGeometryPoint(answer: String?): DoubleArray? { - if (answer != null && answer.isNotEmpty()) { - val sa = answer.trim { it <= ' ' }.split(" ").toTypedArray() - return try { - doubleArrayOf( - sa[0].toDouble(), - if (sa.size > 1) sa[1].toDouble() else 0.0, - if (sa.size > 2) sa[2].toDouble() else 0.0, - if (sa.size > 3) sa[3].toDouble() else 0.0 - ) - } catch (e: Throwable) { - null - } - } else { - return null - } - } - - fun parseGeometry(geometry: String?): ArrayList { - val points = ArrayList() - - for (vertex in (geometry ?: "").split(";").toTypedArray()) { - val point = parseGeometryPoint(vertex) - if (point != null) { - points.add(MapPoint(point[0], point[1], point[2], point[3])) - } else { - return ArrayList() - } - } - - return points - } - fun isWithinMapBounds(point: MapPoint): Boolean { return point.latitude.absoluteValue <= 90 && point.longitude.absoluteValue <= 180 } diff --git a/collect_app/src/test/java/org/odk/collect/android/widgets/GeoPointMapWidgetTest.java b/collect_app/src/test/java/org/odk/collect/android/widgets/GeoPointMapWidgetTest.java index 7fdb575a084..4749d3ab03a 100644 --- a/collect_app/src/test/java/org/odk/collect/android/widgets/GeoPointMapWidgetTest.java +++ b/collect_app/src/test/java/org/odk/collect/android/widgets/GeoPointMapWidgetTest.java @@ -27,6 +27,7 @@ import static org.odk.collect.android.widgets.support.QuestionWidgetHelpers.promptWithReadOnlyAndAnswer; import static org.odk.collect.android.widgets.support.QuestionWidgetHelpers.widgetDependencies; import static org.odk.collect.android.widgets.support.QuestionWidgetHelpers.widgetTestActivity; +import static org.odk.collect.geo.geopoly.GeoPolyUtils.parseGeometryPoint; @RunWith(AndroidJUnit4.class) public class GeoPointMapWidgetTest { @@ -51,7 +52,7 @@ public void getAnswer_whenPromptDoesNotHaveAnswer_returnsNull() { public void getAnswer_whenPromptHasAnswer_returnsPromptAnswer() { GeoPointMapWidget widget = createWidget(promptWithAnswer(answer)); assertEquals(widget.getAnswer().getDisplayText(), - new GeoPointData(GeoWidgetUtils.parseGeometryPoint(answer.getDisplayText())).getDisplayText()); + new GeoPointData(parseGeometryPoint(answer.getDisplayText())).getDisplayText()); } @Test diff --git a/collect_app/src/test/java/org/odk/collect/android/widgets/utilities/GeoWidgetUtilsTest.kt b/collect_app/src/test/java/org/odk/collect/android/widgets/utilities/GeoWidgetUtilsTest.kt index b9f67953c28..dabe463bc44 100644 --- a/collect_app/src/test/java/org/odk/collect/android/widgets/utilities/GeoWidgetUtilsTest.kt +++ b/collect_app/src/test/java/org/odk/collect/android/widgets/utilities/GeoWidgetUtilsTest.kt @@ -16,9 +16,9 @@ import org.odk.collect.android.widgets.utilities.GeoWidgetUtils.floor import org.odk.collect.android.widgets.utilities.GeoWidgetUtils.getGeoPointAnswerToDisplay import org.odk.collect.android.widgets.utilities.GeoWidgetUtils.getGeoPolyAnswerToDisplay import org.odk.collect.android.widgets.utilities.GeoWidgetUtils.isWithinMapBounds -import org.odk.collect.android.widgets.utilities.GeoWidgetUtils.parseGeometry -import org.odk.collect.android.widgets.utilities.GeoWidgetUtils.parseGeometryPoint import org.odk.collect.android.widgets.utilities.GeoWidgetUtils.truncateDouble +import org.odk.collect.geo.geopoly.GeoPolyUtils.parseGeometry +import org.odk.collect.geo.geopoly.GeoPolyUtils.parseGeometryPoint import org.odk.collect.maps.MapPoint @RunWith(AndroidJUnit4::class) diff --git a/collect_app/src/test/java/org/odk/collect/geo/javarosa/IntersectsFunctionHandlerTest.kt b/collect_app/src/test/java/org/odk/collect/geo/javarosa/IntersectsFunctionHandlerTest.kt new file mode 100644 index 00000000000..2296ac14137 --- /dev/null +++ b/collect_app/src/test/java/org/odk/collect/geo/javarosa/IntersectsFunctionHandlerTest.kt @@ -0,0 +1,90 @@ +package org.odk.collect.geo.javarosa + +import org.hamcrest.MatcherAssert.assertThat +import org.hamcrest.Matchers.equalTo +import org.javarosa.core.model.data.BooleanData +import org.javarosa.form.api.FormEntryController +import org.javarosa.form.api.FormEntryModel +import org.javarosa.test.BindBuilderXFormsElement.bind +import org.javarosa.test.Scenario +import org.javarosa.test.XFormsElement.body +import org.javarosa.test.XFormsElement.head +import org.javarosa.test.XFormsElement.html +import org.javarosa.test.XFormsElement.input +import org.javarosa.test.XFormsElement.mainInstance +import org.javarosa.test.XFormsElement.model +import org.javarosa.test.XFormsElement.t +import org.javarosa.test.XFormsElement.title +import org.junit.Test + +class IntersectsFunctionHandlerTest { + + @Test + fun `returns false when input is empty`() { + val scenario = Scenario.init( + "Intersects form", + html( + head( + title("Intersects form"), + model( + mainInstance( + t( + "data id=\"intersects-form\"", + t("question"), + t("calculate") + ) + ), + bind("/data/question").type("geotrace"), + bind("/data/calculate").type("boolean") + .calculate("intersects(/data/question)") + ) + ), + body( + input("/data/question"), + input("/data/calculate") + ) + ) + ) { formDef -> + FormEntryController(FormEntryModel(formDef)).also { + it.addFunctionHandler(IntersectsFunctionHandler()) + } + } + + assertThat(scenario.answerOf("/data/calculate").value, equalTo(false)) + } + + @Test + fun `returns true when input is intersecting trace`() { + val scenario = Scenario.init( + "Intersects form", + html( + head( + title("Intersects form"), + model( + mainInstance( + t( + "data id=\"intersects-form\"", + t("question"), + t("calculate") + ) + ), + bind("/data/question").type("geotrace"), + bind("/data/calculate").type("boolean") + .calculate("intersects(/data/question)") + ) + ), + body( + input("/data/question"), + input("/data/calculate") + ) + ) + ) { formDef -> + FormEntryController(FormEntryModel(formDef)).also { + it.addFunctionHandler(IntersectsFunctionHandler()) + } + } + + scenario.answer("/data/question", "1.0 1.0 0.0 0.0; 1.0 3.0 0.0 0.0; 2.0 3.0 0.0 0.0; 2.0 2.0 0.0 0.0; 0.0 2.0 0.0 0.0") + assertThat(scenario.answerOf("/data/calculate").value, equalTo(true)) + } +} diff --git a/geo/build.gradle.kts b/geo/build.gradle.kts index c04cbf797e5..b75c45dadba 100644 --- a/geo/build.gradle.kts +++ b/geo/build.gradle.kts @@ -64,6 +64,10 @@ dependencies { implementation(libs.androidxFragmentKtx) implementation(libs.dagger) kapt(libs.daggerCompiler) + implementation(libs.javarosa) { + exclude(group = "joda-time") + exclude(group = "org.hamcrest", module = "hamcrest-all") + } debugImplementation(project(":fragments-test")) diff --git a/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt b/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt index ed02e3186d5..9eb77fdb7dc 100644 --- a/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt +++ b/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt @@ -4,6 +4,40 @@ import org.odk.collect.maps.MapPoint object GeoPolyUtils { + fun parseGeometry(geometry: String?): ArrayList { + val points = ArrayList() + + for (vertex in (geometry ?: "").split(";").toTypedArray()) { + val point = parseGeometryPoint(vertex) + if (point != null) { + points.add(MapPoint(point[0], point[1], point[2], point[3])) + } else { + return ArrayList() + } + } + + return points + } + + @JvmStatic + fun parseGeometryPoint(answer: String?): DoubleArray? { + if (answer != null && answer.isNotEmpty()) { + val sa = answer.trim { it <= ' ' }.split(" ").toTypedArray() + return try { + doubleArrayOf( + sa[0].toDouble(), + if (sa.size > 1) sa[1].toDouble() else 0.0, + if (sa.size > 2) sa[2].toDouble() else 0.0, + if (sa.size > 3) sa[3].toDouble() else 0.0 + ) + } catch (e: Throwable) { + null + } + } else { + return null + } + } + /** * Returns `true` if any segment of the trace intersects with any other and `false` otherwise. */ diff --git a/geo/src/main/java/org/odk/collect/geo/javarosa/IntersectsFunctionHandler.kt b/geo/src/main/java/org/odk/collect/geo/javarosa/IntersectsFunctionHandler.kt new file mode 100644 index 00000000000..8a2c116602b --- /dev/null +++ b/geo/src/main/java/org/odk/collect/geo/javarosa/IntersectsFunctionHandler.kt @@ -0,0 +1,33 @@ +package org.odk.collect.geo.javarosa + +import org.javarosa.core.model.condition.EvaluationContext +import org.javarosa.core.model.condition.IFunctionHandler +import org.javarosa.xpath.expr.XPathFuncExpr +import org.odk.collect.geo.geopoly.GeoPolyUtils.intersects +import org.odk.collect.geo.geopoly.GeoPolyUtils.parseGeometry + +class IntersectsFunctionHandler : IFunctionHandler { + override fun getName(): String { + return "intersects" + } + + override fun getPrototypes(): List>> { + return emptyList() + } + + override fun rawArgs(): Boolean { + return true + } + + override fun realTime(): Boolean { + TODO("Not yet implemented") + } + + override fun eval( + args: Array, + ec: EvaluationContext + ): Any { + val trace = parseGeometry(XPathFuncExpr.toString(args[0])) + return intersects(trace) + } +} From ab235b819ec3ace5baf8fcd7ae729c09f1a5390d Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Tue, 11 Nov 2025 14:03:44 +0000 Subject: [PATCH 03/43] Use prototypes to get already evaluated args --- .../odk/collect/geo/javarosa/IntersectsFunctionHandler.kt | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/geo/src/main/java/org/odk/collect/geo/javarosa/IntersectsFunctionHandler.kt b/geo/src/main/java/org/odk/collect/geo/javarosa/IntersectsFunctionHandler.kt index 8a2c116602b..d3790562d76 100644 --- a/geo/src/main/java/org/odk/collect/geo/javarosa/IntersectsFunctionHandler.kt +++ b/geo/src/main/java/org/odk/collect/geo/javarosa/IntersectsFunctionHandler.kt @@ -2,7 +2,6 @@ package org.odk.collect.geo.javarosa import org.javarosa.core.model.condition.EvaluationContext import org.javarosa.core.model.condition.IFunctionHandler -import org.javarosa.xpath.expr.XPathFuncExpr import org.odk.collect.geo.geopoly.GeoPolyUtils.intersects import org.odk.collect.geo.geopoly.GeoPolyUtils.parseGeometry @@ -12,11 +11,11 @@ class IntersectsFunctionHandler : IFunctionHandler { } override fun getPrototypes(): List>> { - return emptyList() + return listOf(arrayOf(String::class.java)) } override fun rawArgs(): Boolean { - return true + return false } override fun realTime(): Boolean { @@ -27,7 +26,7 @@ class IntersectsFunctionHandler : IFunctionHandler { args: Array, ec: EvaluationContext ): Any { - val trace = parseGeometry(XPathFuncExpr.toString(args[0])) + val trace = parseGeometry(args[0] as String) return intersects(trace) } } From 4fb7739a1aaa7ed57f3626910b3865d3b69dacfe Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Mon, 17 Nov 2025 14:16:59 +0000 Subject: [PATCH 04/43] Add test for non geotrace/shape strings --- .../javarosa/IntersectsFunctionHandlerTest.kt | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/collect_app/src/test/java/org/odk/collect/geo/javarosa/IntersectsFunctionHandlerTest.kt b/collect_app/src/test/java/org/odk/collect/geo/javarosa/IntersectsFunctionHandlerTest.kt index 2296ac14137..af1fe45f713 100644 --- a/collect_app/src/test/java/org/odk/collect/geo/javarosa/IntersectsFunctionHandlerTest.kt +++ b/collect_app/src/test/java/org/odk/collect/geo/javarosa/IntersectsFunctionHandlerTest.kt @@ -53,6 +53,41 @@ class IntersectsFunctionHandlerTest { assertThat(scenario.answerOf("/data/calculate").value, equalTo(false)) } + @Test + fun `returns false when input is a string`() { + val scenario = Scenario.init( + "Intersects form", + html( + head( + title("Intersects form"), + model( + mainInstance( + t( + "data id=\"intersects-form\"", + t("question"), + t("calculate") + ) + ), + bind("/data/question").type("string"), + bind("/data/calculate").type("boolean") + .calculate("intersects(/data/question)") + ) + ), + body( + input("/data/question"), + input("/data/calculate") + ) + ) + ) { formDef -> + FormEntryController(FormEntryModel(formDef)).also { + it.addFunctionHandler(IntersectsFunctionHandler()) + } + } + + scenario.answer("/data/question", "blah") + assertThat(scenario.answerOf("/data/calculate").value, equalTo(false)) + } + @Test fun `returns true when input is intersecting trace`() { val scenario = Scenario.init( From 3419811c10d1e27eef289404ac223dae92306aca Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Mon, 17 Nov 2025 14:19:32 +0000 Subject: [PATCH 05/43] Add test cases for repeated segments and closed traced intersection --- .../collect/geo/geopoly/GeoPolyUtilsTest.kt | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt b/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt index d464905f232..ba4302af1f5 100644 --- a/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt +++ b/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt @@ -24,6 +24,18 @@ class GeoPolyUtilsTest { assertThat(GeoPolyUtils.intersects(trace), equalTo(false)) } + @Test + fun `#intersects returns false when there is just two of the same segment`() { + val trace = listOf( + MapPoint(0.0, 0.0), + MapPoint(1.0, 0.0), + MapPoint(1.0, 0.0), + MapPoint(0.0, 0.0) + ) + + assertThat(GeoPolyUtils.intersects(trace), equalTo(false)) + } + @Test fun `#intersects returns false when no segment intersects with another`() { val trace = listOf( @@ -59,4 +71,18 @@ class GeoPolyUtilsTest { assertThat(GeoPolyUtils.intersects(trace), equalTo(true)) } + + @Test + fun `#intersects returns true when a segment intersects with another in a closed trace`() { + val trace = listOf( + MapPoint(1.0, 1.0), + MapPoint(1.0, 3.0), + MapPoint(2.0, 3.0), + MapPoint(2.0, 2.0), + MapPoint(0.0, 2.0), + MapPoint(1.0, 1.0) + ) + + assertThat(GeoPolyUtils.intersects(trace), equalTo(true)) + } } From 9641e08c9adea6c68c296f51898721d11573f1da Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Mon, 17 Nov 2025 14:26:41 +0000 Subject: [PATCH 06/43] Add intersects function handler to Collect --- .../formmanagement/CollectFormEntryControllerFactory.kt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/collect_app/src/main/java/org/odk/collect/android/formmanagement/CollectFormEntryControllerFactory.kt b/collect_app/src/main/java/org/odk/collect/android/formmanagement/CollectFormEntryControllerFactory.kt index 352e867f959..e9d6d2164a2 100644 --- a/collect_app/src/main/java/org/odk/collect/android/formmanagement/CollectFormEntryControllerFactory.kt +++ b/collect_app/src/main/java/org/odk/collect/android/formmanagement/CollectFormEntryControllerFactory.kt @@ -22,6 +22,7 @@ import org.odk.collect.entities.javarosa.filter.PullDataFunctionHandler import org.odk.collect.entities.javarosa.finalization.EntityFormFinalizationProcessor import org.odk.collect.entities.storage.EntitiesRepository import org.odk.collect.forms.instances.Instance +import org.odk.collect.geo.javarosa.IntersectsFunctionHandler import org.odk.collect.settings.keys.ProjectKeys import org.odk.collect.shared.settings.Settings import java.io.File @@ -46,6 +47,9 @@ class CollectFormEntryControllerFactory( externalDataHandlerPull ) ) + + it.addFunctionHandler(IntersectsFunctionHandler()) + it.addPostProcessor(EntityFormFinalizationProcessor()) it.addPostProcessor(EditedFormFinalizationProcessor(instance)) From b2f3cc79cd2f846074c4aa60dfba0b9437284c2d Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Mon, 17 Nov 2025 17:19:20 +0000 Subject: [PATCH 07/43] Move parseGeometry and parseGeometryPoint tests to geo --- .../widgets/utilities/GeoWidgetUtilsTest.kt | 45 ------------------- .../collect/geo/geopoly/GeoPolyUtilsTest.kt | 45 +++++++++++++++++++ 2 files changed, 45 insertions(+), 45 deletions(-) diff --git a/collect_app/src/test/java/org/odk/collect/android/widgets/utilities/GeoWidgetUtilsTest.kt b/collect_app/src/test/java/org/odk/collect/android/widgets/utilities/GeoWidgetUtilsTest.kt index dabe463bc44..2d6b7a785da 100644 --- a/collect_app/src/test/java/org/odk/collect/android/widgets/utilities/GeoWidgetUtilsTest.kt +++ b/collect_app/src/test/java/org/odk/collect/android/widgets/utilities/GeoWidgetUtilsTest.kt @@ -9,7 +9,6 @@ import org.hamcrest.Matchers.equalTo import org.javarosa.core.model.data.GeoPointData import org.junit.Test import org.junit.runner.RunWith -import org.odk.collect.android.R import org.odk.collect.android.widgets.support.GeoWidgetHelpers import org.odk.collect.android.widgets.utilities.GeoWidgetUtils.convertCoordinatesIntoDegreeFormat import org.odk.collect.android.widgets.utilities.GeoWidgetUtils.floor @@ -17,8 +16,6 @@ import org.odk.collect.android.widgets.utilities.GeoWidgetUtils.getGeoPointAnswe import org.odk.collect.android.widgets.utilities.GeoWidgetUtils.getGeoPolyAnswerToDisplay import org.odk.collect.android.widgets.utilities.GeoWidgetUtils.isWithinMapBounds import org.odk.collect.android.widgets.utilities.GeoWidgetUtils.truncateDouble -import org.odk.collect.geo.geopoly.GeoPolyUtils.parseGeometry -import org.odk.collect.geo.geopoly.GeoPolyUtils.parseGeometryPoint import org.odk.collect.maps.MapPoint @RunWith(AndroidJUnit4::class) @@ -102,32 +99,6 @@ class GeoWidgetUtilsTest { assertEquals("qwerty", floor("qwerty")) } - @Test - fun parseGeometryPointTest() { - var gp = - parseGeometryPoint("37.45153333333334 -122.15539166666667 0.0 20.0")!! - assertEquals(37.45153333333334, gp[0]) - assertEquals(-122.15539166666667, gp[1]) - assertEquals(0.0, gp[2]) - assertEquals(20.0, gp[3]) - - gp = parseGeometryPoint("37.45153333333334")!! - assertEquals(37.45153333333334, gp[0]) - assertEquals(0.0, gp[1]) - assertEquals(0.0, gp[2]) - assertEquals(0.0, gp[3]) - - gp = parseGeometryPoint(" 37.45153333333334 -122.15539166666667 0.0 ")!! - assertEquals(37.45153333333334, gp[0]) - assertEquals(-122.15539166666667, gp[1]) - assertEquals(0.0, gp[2]) - assertEquals(0.0, gp[3]) - - assertEquals(null, parseGeometryPoint("37.45153333333334 -122.15539166666667 0.0 qwerty")) - assertEquals(null, parseGeometryPoint("")) - assertEquals(null, parseGeometryPoint(null)) - } - @Test fun truncateDoubleTest() { assertEquals("5", truncateDouble("5")) @@ -141,22 +112,6 @@ class GeoWidgetUtilsTest { assertEquals("", truncateDouble("qwerty")) } - @Test - fun parseGeometryTest() { - assertThat(parseGeometry("1.0 2.0 3 4"), equalTo(listOf(MapPoint(1.0, 2.0, 3.0, 4.0)))) - assertThat( - parseGeometry("1.0 2.0 3 4; 5.0 6.0 7 8"), - equalTo(listOf(MapPoint(1.0, 2.0, 3.0, 4.0), MapPoint(5.0, 6.0, 7.0, 8.0))) - ) - - assertThat(parseGeometry("blah"), equalTo(emptyList())) - assertThat(parseGeometry("1.0 2.0 3 4; blah"), equalTo(emptyList())) - assertThat( - parseGeometry("37.45153333333334 -122.15539166666667 0.0 qwerty"), - equalTo(emptyList()) - ) - } - @Test fun isWithinMapBoundsTest() { assertThat(isWithinMapBounds(MapPoint(90.0, 0.0, 0.0, 0.0)), equalTo(true)) diff --git a/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt b/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt index ba4302af1f5..5a3a52353ad 100644 --- a/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt +++ b/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt @@ -1,8 +1,11 @@ package org.odk.collect.geo.geopoly +import junit.framework.TestCase.assertEquals import org.hamcrest.MatcherAssert.assertThat import org.hamcrest.Matchers.equalTo import org.junit.Test +import org.odk.collect.geo.geopoly.GeoPolyUtils.parseGeometry +import org.odk.collect.geo.geopoly.GeoPolyUtils.parseGeometryPoint import org.odk.collect.maps.MapPoint class GeoPolyUtilsTest { @@ -85,4 +88,46 @@ class GeoPolyUtilsTest { assertThat(GeoPolyUtils.intersects(trace), equalTo(true)) } + + @Test + fun parseGeometryPointTest() { + var gp = + parseGeometryPoint("37.45153333333334 -122.15539166666667 0.0 20.0")!! + assertEquals(37.45153333333334, gp[0]) + assertEquals(-122.15539166666667, gp[1]) + assertEquals(0.0, gp[2]) + assertEquals(20.0, gp[3]) + + gp = parseGeometryPoint("37.45153333333334")!! + assertEquals(37.45153333333334, gp[0]) + assertEquals(0.0, gp[1]) + assertEquals(0.0, gp[2]) + assertEquals(0.0, gp[3]) + + gp = parseGeometryPoint(" 37.45153333333334 -122.15539166666667 0.0 ")!! + assertEquals(37.45153333333334, gp[0]) + assertEquals(-122.15539166666667, gp[1]) + assertEquals(0.0, gp[2]) + assertEquals(0.0, gp[3]) + + assertEquals(null, parseGeometryPoint("37.45153333333334 -122.15539166666667 0.0 qwerty")) + assertEquals(null, parseGeometryPoint("")) + assertEquals(null, parseGeometryPoint(null)) + } + + @Test + fun parseGeometryTest() { + assertThat(parseGeometry("1.0 2.0 3 4"), equalTo(listOf(MapPoint(1.0, 2.0, 3.0, 4.0)))) + assertThat( + parseGeometry("1.0 2.0 3 4; 5.0 6.0 7 8"), + equalTo(listOf(MapPoint(1.0, 2.0, 3.0, 4.0), MapPoint(5.0, 6.0, 7.0, 8.0))) + ) + + assertThat(parseGeometry("blah"), equalTo(emptyList())) + assertThat(parseGeometry("1.0 2.0 3 4; blah"), equalTo(emptyList())) + assertThat( + parseGeometry("37.45153333333334 -122.15539166666667 0.0 qwerty"), + equalTo(emptyList()) + ) + } } From a11a20dfcc53399049cd521cf00c91d946c1e525 Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Mon, 17 Nov 2025 17:26:11 +0000 Subject: [PATCH 08/43] Convert GeoUtils to Kotlin --- .../java/org/odk/collect/geo/GeoUtils.java | 61 ----------------- .../main/java/org/odk/collect/geo/GeoUtils.kt | 65 +++++++++++++++++++ .../geo/geopoint/GeoPointDialogFragment.kt | 2 +- .../java/org/odk/collect/geo/GeoUtilsTest.kt | 4 +- 4 files changed, 68 insertions(+), 64 deletions(-) delete mode 100644 geo/src/main/java/org/odk/collect/geo/GeoUtils.java create mode 100644 geo/src/main/java/org/odk/collect/geo/GeoUtils.kt diff --git a/geo/src/main/java/org/odk/collect/geo/GeoUtils.java b/geo/src/main/java/org/odk/collect/geo/GeoUtils.java deleted file mode 100644 index 2390c02d618..00000000000 --- a/geo/src/main/java/org/odk/collect/geo/GeoUtils.java +++ /dev/null @@ -1,61 +0,0 @@ -package org.odk.collect.geo; - -import android.content.Context; -import android.location.Location; - -import org.odk.collect.maps.MapPoint; -import org.odk.collect.shared.strings.StringUtils; - -import java.text.DecimalFormat; -import java.util.List; -import java.util.Locale; - -public final class GeoUtils { - - private GeoUtils() { - - } - - /** - * Serializes a list of vertices into a string, in the format - * appropriate for storing as the result of the form question. - */ - public static String formatPointsResultString(List points, boolean isShape) { - if (isShape) { - // Polygons are stored with a last point that duplicates the - // first point. Add this extra point if it's not already present. - int count = points.size(); - if (count > 1 && !points.get(0).equals(points.get(count - 1))) { - points.add(points.get(0)); - } - } - StringBuilder result = new StringBuilder(); - for (MapPoint point : points) { - // TODO(ping): Remove excess precision when we're ready for the output to change. - result.append(String.format(Locale.US, "%s %s %s %s;", - Double.toString(point.latitude), Double.toString(point.longitude), - Double.toString(point.altitude), Float.toString((float) point.accuracy))); - } - - return StringUtils.removeEnd(result.toString().trim(), ";"); - } - - public static String formatLocationResultString(Location location) { - return formatLocationResultString(new org.odk.collect.location.Location( - location.getLatitude(), - location.getLongitude(), - location.getAltitude(), - location.getAccuracy() - )); - } - - public static String formatLocationResultString(org.odk.collect.location.Location location) { - return String.format("%s %s %s %s", location.getLatitude(), location.getLongitude(), - location.getAltitude(), location.getAccuracy()); - } - - public static String formatAccuracy(Context context, float accuracy) { - String formattedValue = new DecimalFormat("#.##").format(accuracy); - return context.getString(org.odk.collect.strings.R.string.accuracy_m, formattedValue); - } -} diff --git a/geo/src/main/java/org/odk/collect/geo/GeoUtils.kt b/geo/src/main/java/org/odk/collect/geo/GeoUtils.kt new file mode 100644 index 00000000000..7ba3f919a1e --- /dev/null +++ b/geo/src/main/java/org/odk/collect/geo/GeoUtils.kt @@ -0,0 +1,65 @@ +package org.odk.collect.geo + +import android.content.Context +import android.location.Location +import org.odk.collect.maps.MapPoint +import org.odk.collect.shared.strings.StringUtils.removeEnd +import org.odk.collect.strings.R +import java.text.DecimalFormat +import java.util.Locale + +object GeoUtils { + + /** + * Serializes a list of vertices into a string, in the format + * appropriate for storing as the result of the form question. + */ + @JvmStatic + fun formatPointsResultString(points: MutableList, isShape: Boolean): String? { + if (isShape) { + // Polygons are stored with a last point that duplicates the + // first point. Add this extra point if it's not already present. + val count = points.size + if (count > 1 && points[0] != points[count - 1]) { + points.add(points[0]) + } + } + val result = StringBuilder() + for (point in points) { + // TODO(ping): Remove excess precision when we're ready for the output to change. + result.append( + String.format( + Locale.US, "%s %s %s %s;", + point.latitude.toString(), point.longitude.toString(), + point.altitude.toString(), point.accuracy.toFloat().toString() + ) + ) + } + + return removeEnd(result.toString().trim { it <= ' ' }, ";") + } + + @JvmStatic + fun formatLocationResultString(location: Location): String { + return formatLocationResultString( + org.odk.collect.location.Location( + location.latitude, + location.longitude, + location.altitude, + location.accuracy + ) + ) + } + + fun formatLocationResultString(location: org.odk.collect.location.Location): String { + return String.format( + "%s %s %s %s", location.latitude, location.longitude, + location.altitude, location.accuracy + ) + } + + fun formatAccuracy(context: Context, accuracy: Float): String { + val formattedValue = DecimalFormat("#.##").format(accuracy.toDouble()) + return context.getString(R.string.accuracy_m, formattedValue) + } +} diff --git a/geo/src/main/java/org/odk/collect/geo/geopoint/GeoPointDialogFragment.kt b/geo/src/main/java/org/odk/collect/geo/geopoint/GeoPointDialogFragment.kt index 4fda6c9d507..4bb05cb85f8 100644 --- a/geo/src/main/java/org/odk/collect/geo/geopoint/GeoPointDialogFragment.kt +++ b/geo/src/main/java/org/odk/collect/geo/geopoint/GeoPointDialogFragment.kt @@ -58,7 +58,7 @@ class GeoPointDialogFragment : DialogFragment() { } binding.threshold.text = - getString(org.odk.collect.strings.R.string.point_will_be_saved, formatAccuracy(context, accuracyThreshold)) + getString(org.odk.collect.strings.R.string.point_will_be_saved, formatAccuracy(requireContext(), accuracyThreshold)) viewModel.timeElapsed.observe(this) { binding.time.text = diff --git a/geo/src/test/java/org/odk/collect/geo/GeoUtilsTest.kt b/geo/src/test/java/org/odk/collect/geo/GeoUtilsTest.kt index 72f5593ea5b..db19433754f 100644 --- a/geo/src/test/java/org/odk/collect/geo/GeoUtilsTest.kt +++ b/geo/src/test/java/org/odk/collect/geo/GeoUtilsTest.kt @@ -25,8 +25,8 @@ class GeoUtilsTest { @Test fun whenPointsAreNull_formatPoints_returnsEmptyString() { - assertEquals(formatPointsResultString(emptyList(), true), "") - assertEquals(formatPointsResultString(emptyList(), false), "") + assertEquals(formatPointsResultString(mutableListOf(), true), "") + assertEquals(formatPointsResultString(mutableListOf(), false), "") } @Test From 7508fe86def850f0067435aa143cf0311ec9c797 Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Mon, 17 Nov 2025 17:33:23 +0000 Subject: [PATCH 09/43] Move parseGeometryPoint to GeoUtils --- .../android/widgets/GeoPointMapWidget.java | 2 +- .../android/widgets/GeoPointWidget.java | 2 +- .../main/java/org/odk/collect/geo/GeoUtils.kt | 19 ++++++++++++ .../odk/collect/geo/geopoly/GeoPolyUtils.kt | 20 +----------- .../java/org/odk/collect/geo/GeoUtilsTest.kt | 31 +++++++++++++++++++ .../collect/geo/geopoly/GeoPolyUtilsTest.kt | 28 +---------------- 6 files changed, 54 insertions(+), 48 deletions(-) diff --git a/collect_app/src/main/java/org/odk/collect/android/widgets/GeoPointMapWidget.java b/collect_app/src/main/java/org/odk/collect/android/widgets/GeoPointMapWidget.java index 56f3a13ceb8..1841eebe880 100644 --- a/collect_app/src/main/java/org/odk/collect/android/widgets/GeoPointMapWidget.java +++ b/collect_app/src/main/java/org/odk/collect/android/widgets/GeoPointMapWidget.java @@ -14,7 +14,7 @@ package org.odk.collect.android.widgets; -import static org.odk.collect.geo.geopoly.GeoPolyUtils.parseGeometryPoint; +import static org.odk.collect.geo.GeoUtils.parseGeometryPoint; import android.annotation.SuppressLint; import android.app.Activity; diff --git a/collect_app/src/main/java/org/odk/collect/android/widgets/GeoPointWidget.java b/collect_app/src/main/java/org/odk/collect/android/widgets/GeoPointWidget.java index cdec3ddcf52..bd443d78461 100644 --- a/collect_app/src/main/java/org/odk/collect/android/widgets/GeoPointWidget.java +++ b/collect_app/src/main/java/org/odk/collect/android/widgets/GeoPointWidget.java @@ -14,7 +14,7 @@ package org.odk.collect.android.widgets; -import static org.odk.collect.geo.geopoly.GeoPolyUtils.parseGeometryPoint; +import static org.odk.collect.geo.GeoUtils.parseGeometryPoint; import android.annotation.SuppressLint; import android.app.Activity; diff --git a/geo/src/main/java/org/odk/collect/geo/GeoUtils.kt b/geo/src/main/java/org/odk/collect/geo/GeoUtils.kt index 7ba3f919a1e..a6d10abb5b0 100644 --- a/geo/src/main/java/org/odk/collect/geo/GeoUtils.kt +++ b/geo/src/main/java/org/odk/collect/geo/GeoUtils.kt @@ -62,4 +62,23 @@ object GeoUtils { val formattedValue = DecimalFormat("#.##").format(accuracy.toDouble()) return context.getString(R.string.accuracy_m, formattedValue) } + + @JvmStatic + fun parseGeometryPoint(answer: String?): DoubleArray? { + if (answer != null && answer.isNotEmpty()) { + val sa = answer.trim { it <= ' ' }.split(" ").toTypedArray() + return try { + doubleArrayOf( + sa[0].toDouble(), + if (sa.size > 1) sa[1].toDouble() else 0.0, + if (sa.size > 2) sa[2].toDouble() else 0.0, + if (sa.size > 3) sa[3].toDouble() else 0.0 + ) + } catch (e: Throwable) { + null + } + } else { + return null + } + } } diff --git a/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt b/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt index 9eb77fdb7dc..22c5baf1c2b 100644 --- a/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt +++ b/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt @@ -1,5 +1,6 @@ package org.odk.collect.geo.geopoly +import org.odk.collect.geo.GeoUtils.parseGeometryPoint import org.odk.collect.maps.MapPoint object GeoPolyUtils { @@ -19,25 +20,6 @@ object GeoPolyUtils { return points } - @JvmStatic - fun parseGeometryPoint(answer: String?): DoubleArray? { - if (answer != null && answer.isNotEmpty()) { - val sa = answer.trim { it <= ' ' }.split(" ").toTypedArray() - return try { - doubleArrayOf( - sa[0].toDouble(), - if (sa.size > 1) sa[1].toDouble() else 0.0, - if (sa.size > 2) sa[2].toDouble() else 0.0, - if (sa.size > 3) sa[3].toDouble() else 0.0 - ) - } catch (e: Throwable) { - null - } - } else { - return null - } - } - /** * Returns `true` if any segment of the trace intersects with any other and `false` otherwise. */ diff --git a/geo/src/test/java/org/odk/collect/geo/GeoUtilsTest.kt b/geo/src/test/java/org/odk/collect/geo/GeoUtilsTest.kt index db19433754f..8a276c99404 100644 --- a/geo/src/test/java/org/odk/collect/geo/GeoUtilsTest.kt +++ b/geo/src/test/java/org/odk/collect/geo/GeoUtilsTest.kt @@ -4,12 +4,14 @@ import android.content.Context import android.location.Location import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 +import junit.framework.TestCase import org.hamcrest.MatcherAssert.assertThat import org.hamcrest.Matchers.equalTo import org.junit.Assert.assertEquals import org.junit.Test import org.junit.runner.RunWith import org.odk.collect.geo.GeoUtils.formatPointsResultString +import org.odk.collect.geo.GeoUtils.parseGeometryPoint import org.odk.collect.maps.MapPoint import org.odk.collect.testshared.LocationTestUtils.createLocation @@ -59,4 +61,33 @@ class GeoUtilsTest { assertThat(GeoUtils.formatAccuracy(context, 0.10f), equalTo("0.1 m")) assertThat(GeoUtils.formatAccuracy(context, 1.1f), equalTo("1.1 m")) } + + @Test + fun parseGeometryPointTest() { + var gp = + parseGeometryPoint("37.45153333333334 -122.15539166666667 0.0 20.0")!! + TestCase.assertEquals(37.45153333333334, gp[0]) + TestCase.assertEquals(-122.15539166666667, gp[1]) + TestCase.assertEquals(0.0, gp[2]) + TestCase.assertEquals(20.0, gp[3]) + + gp = parseGeometryPoint("37.45153333333334")!! + TestCase.assertEquals(37.45153333333334, gp[0]) + TestCase.assertEquals(0.0, gp[1]) + TestCase.assertEquals(0.0, gp[2]) + TestCase.assertEquals(0.0, gp[3]) + + gp = parseGeometryPoint(" 37.45153333333334 -122.15539166666667 0.0 ")!! + TestCase.assertEquals(37.45153333333334, gp[0]) + TestCase.assertEquals(-122.15539166666667, gp[1]) + TestCase.assertEquals(0.0, gp[2]) + TestCase.assertEquals(0.0, gp[3]) + + TestCase.assertEquals( + null, + parseGeometryPoint("37.45153333333334 -122.15539166666667 0.0 qwerty") + ) + TestCase.assertEquals(null, parseGeometryPoint("")) + TestCase.assertEquals(null, parseGeometryPoint(null)) + } } diff --git a/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt b/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt index 5a3a52353ad..23907ab9324 100644 --- a/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt +++ b/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt @@ -4,8 +4,8 @@ import junit.framework.TestCase.assertEquals import org.hamcrest.MatcherAssert.assertThat import org.hamcrest.Matchers.equalTo import org.junit.Test +import org.odk.collect.geo.GeoUtils.parseGeometryPoint import org.odk.collect.geo.geopoly.GeoPolyUtils.parseGeometry -import org.odk.collect.geo.geopoly.GeoPolyUtils.parseGeometryPoint import org.odk.collect.maps.MapPoint class GeoPolyUtilsTest { @@ -89,32 +89,6 @@ class GeoPolyUtilsTest { assertThat(GeoPolyUtils.intersects(trace), equalTo(true)) } - @Test - fun parseGeometryPointTest() { - var gp = - parseGeometryPoint("37.45153333333334 -122.15539166666667 0.0 20.0")!! - assertEquals(37.45153333333334, gp[0]) - assertEquals(-122.15539166666667, gp[1]) - assertEquals(0.0, gp[2]) - assertEquals(20.0, gp[3]) - - gp = parseGeometryPoint("37.45153333333334")!! - assertEquals(37.45153333333334, gp[0]) - assertEquals(0.0, gp[1]) - assertEquals(0.0, gp[2]) - assertEquals(0.0, gp[3]) - - gp = parseGeometryPoint(" 37.45153333333334 -122.15539166666667 0.0 ")!! - assertEquals(37.45153333333334, gp[0]) - assertEquals(-122.15539166666667, gp[1]) - assertEquals(0.0, gp[2]) - assertEquals(0.0, gp[3]) - - assertEquals(null, parseGeometryPoint("37.45153333333334 -122.15539166666667 0.0 qwerty")) - assertEquals(null, parseGeometryPoint("")) - assertEquals(null, parseGeometryPoint(null)) - } - @Test fun parseGeometryTest() { assertThat(parseGeometry("1.0 2.0 3 4"), equalTo(listOf(MapPoint(1.0, 2.0, 3.0, 4.0)))) From 17e52e85a7d7ed9f1e2b2bd164564573eee7ceb7 Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Tue, 18 Nov 2025 09:16:55 +0000 Subject: [PATCH 10/43] Fix import in test --- .../org/odk/collect/android/widgets/GeoPointMapWidgetTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/collect_app/src/test/java/org/odk/collect/android/widgets/GeoPointMapWidgetTest.java b/collect_app/src/test/java/org/odk/collect/android/widgets/GeoPointMapWidgetTest.java index 4749d3ab03a..fb13d1fee88 100644 --- a/collect_app/src/test/java/org/odk/collect/android/widgets/GeoPointMapWidgetTest.java +++ b/collect_app/src/test/java/org/odk/collect/android/widgets/GeoPointMapWidgetTest.java @@ -27,7 +27,7 @@ import static org.odk.collect.android.widgets.support.QuestionWidgetHelpers.promptWithReadOnlyAndAnswer; import static org.odk.collect.android.widgets.support.QuestionWidgetHelpers.widgetDependencies; import static org.odk.collect.android.widgets.support.QuestionWidgetHelpers.widgetTestActivity; -import static org.odk.collect.geo.geopoly.GeoPolyUtils.parseGeometryPoint; +import static org.odk.collect.geo.GeoUtils.parseGeometryPoint; @RunWith(AndroidJUnit4.class) public class GeoPointMapWidgetTest { From 83a60e23beb5f17d8aa628436f2e7b99b633797c Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Tue, 18 Nov 2025 14:41:25 +0000 Subject: [PATCH 11/43] Throw exception for non-geotrace inputs --- .../javarosa/IntersectsFunctionHandlerTest.kt | 27 +++++++++++++------ .../main/java/org/odk/collect/geo/GeoUtils.kt | 11 +++++--- .../odk/collect/geo/geopoly/GeoPolyUtils.kt | 4 +-- .../geo/javarosa/IntersectsFunctionHandler.kt | 9 +++++-- 4 files changed, 36 insertions(+), 15 deletions(-) diff --git a/collect_app/src/test/java/org/odk/collect/geo/javarosa/IntersectsFunctionHandlerTest.kt b/collect_app/src/test/java/org/odk/collect/geo/javarosa/IntersectsFunctionHandlerTest.kt index af1fe45f713..978ccf3015b 100644 --- a/collect_app/src/test/java/org/odk/collect/geo/javarosa/IntersectsFunctionHandlerTest.kt +++ b/collect_app/src/test/java/org/odk/collect/geo/javarosa/IntersectsFunctionHandlerTest.kt @@ -2,6 +2,7 @@ package org.odk.collect.geo.javarosa import org.hamcrest.MatcherAssert.assertThat import org.hamcrest.Matchers.equalTo +import org.hamcrest.Matchers.instanceOf import org.javarosa.core.model.data.BooleanData import org.javarosa.form.api.FormEntryController import org.javarosa.form.api.FormEntryModel @@ -15,6 +16,8 @@ import org.javarosa.test.XFormsElement.mainInstance import org.javarosa.test.XFormsElement.model import org.javarosa.test.XFormsElement.t import org.javarosa.test.XFormsElement.title +import org.javarosa.xpath.XPathTypeMismatchException +import org.junit.Assert.fail import org.junit.Test class IntersectsFunctionHandlerTest { @@ -54,7 +57,7 @@ class IntersectsFunctionHandlerTest { } @Test - fun `returns false when input is a string`() { + fun `returns true when input is intersecting trace`() { val scenario = Scenario.init( "Intersects form", html( @@ -68,7 +71,7 @@ class IntersectsFunctionHandlerTest { t("calculate") ) ), - bind("/data/question").type("string"), + bind("/data/question").type("geotrace"), bind("/data/calculate").type("boolean") .calculate("intersects(/data/question)") ) @@ -84,12 +87,15 @@ class IntersectsFunctionHandlerTest { } } - scenario.answer("/data/question", "blah") - assertThat(scenario.answerOf("/data/calculate").value, equalTo(false)) + scenario.answer( + "/data/question", + "1.0 1.0 0.0 0.0; 1.0 3.0 0.0 0.0; 2.0 3.0 0.0 0.0; 2.0 2.0 0.0 0.0; 0.0 2.0 0.0 0.0" + ) + assertThat(scenario.answerOf("/data/calculate").value, equalTo(true)) } @Test - fun `returns true when input is intersecting trace`() { + fun `throws exception when input is a non-geo string`() { val scenario = Scenario.init( "Intersects form", html( @@ -103,7 +109,7 @@ class IntersectsFunctionHandlerTest { t("calculate") ) ), - bind("/data/question").type("geotrace"), + bind("/data/question").type("string"), bind("/data/calculate").type("boolean") .calculate("intersects(/data/question)") ) @@ -119,7 +125,12 @@ class IntersectsFunctionHandlerTest { } } - scenario.answer("/data/question", "1.0 1.0 0.0 0.0; 1.0 3.0 0.0 0.0; 2.0 3.0 0.0 0.0; 2.0 2.0 0.0 0.0; 0.0 2.0 0.0 0.0") - assertThat(scenario.answerOf("/data/calculate").value, equalTo(true)) + val expected = XPathTypeMismatchException::class.java + try { + scenario.answer("/data/question", "blah") + fail("Expected exception: $expected") + } catch (e: Exception) { + assertThat(e.cause, instanceOf(expected)) + } } } diff --git a/geo/src/main/java/org/odk/collect/geo/GeoUtils.kt b/geo/src/main/java/org/odk/collect/geo/GeoUtils.kt index a6d10abb5b0..8900613403d 100644 --- a/geo/src/main/java/org/odk/collect/geo/GeoUtils.kt +++ b/geo/src/main/java/org/odk/collect/geo/GeoUtils.kt @@ -64,7 +64,8 @@ object GeoUtils { } @JvmStatic - fun parseGeometryPoint(answer: String?): DoubleArray? { + @JvmOverloads + fun parseGeometryPoint(answer: String?, strict: Boolean = false): DoubleArray? { if (answer != null && answer.isNotEmpty()) { val sa = answer.trim { it <= ' ' }.split(" ").toTypedArray() return try { @@ -74,8 +75,12 @@ object GeoUtils { if (sa.size > 2) sa[2].toDouble() else 0.0, if (sa.size > 3) sa[3].toDouble() else 0.0 ) - } catch (e: Throwable) { - null + } catch (_: Throwable) { + if (strict) { + throw IllegalArgumentException() + } else { + null + } } } else { return null diff --git a/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt b/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt index 22c5baf1c2b..b40c737f2d7 100644 --- a/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt +++ b/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt @@ -5,11 +5,11 @@ import org.odk.collect.maps.MapPoint object GeoPolyUtils { - fun parseGeometry(geometry: String?): ArrayList { + fun parseGeometry(geometry: String?, strict: Boolean = false): ArrayList { val points = ArrayList() for (vertex in (geometry ?: "").split(";").toTypedArray()) { - val point = parseGeometryPoint(vertex) + val point = parseGeometryPoint(vertex, strict) if (point != null) { points.add(MapPoint(point[0], point[1], point[2], point[3])) } else { diff --git a/geo/src/main/java/org/odk/collect/geo/javarosa/IntersectsFunctionHandler.kt b/geo/src/main/java/org/odk/collect/geo/javarosa/IntersectsFunctionHandler.kt index d3790562d76..a88065a3239 100644 --- a/geo/src/main/java/org/odk/collect/geo/javarosa/IntersectsFunctionHandler.kt +++ b/geo/src/main/java/org/odk/collect/geo/javarosa/IntersectsFunctionHandler.kt @@ -2,6 +2,7 @@ package org.odk.collect.geo.javarosa import org.javarosa.core.model.condition.EvaluationContext import org.javarosa.core.model.condition.IFunctionHandler +import org.javarosa.xpath.XPathTypeMismatchException import org.odk.collect.geo.geopoly.GeoPolyUtils.intersects import org.odk.collect.geo.geopoly.GeoPolyUtils.parseGeometry @@ -26,7 +27,11 @@ class IntersectsFunctionHandler : IFunctionHandler { args: Array, ec: EvaluationContext ): Any { - val trace = parseGeometry(args[0] as String) - return intersects(trace) + try { + val trace = parseGeometry(args[0] as String, strict = true) + return intersects(trace) + } catch (_: IllegalArgumentException) { + throw XPathTypeMismatchException() + } } } From 281fd944fda013390a26a9633a7afec969bbe5d2 Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Tue, 18 Nov 2025 15:01:34 +0000 Subject: [PATCH 12/43] Improve converted code --- .../android/widgets/utilities/ActivityGeoDataRequester.kt | 4 ++-- geo/src/main/java/org/odk/collect/geo/GeoUtils.kt | 6 +++--- .../main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/collect_app/src/main/java/org/odk/collect/android/widgets/utilities/ActivityGeoDataRequester.kt b/collect_app/src/main/java/org/odk/collect/android/widgets/utilities/ActivityGeoDataRequester.kt index 8d050d17dc9..90d386f751a 100644 --- a/collect_app/src/main/java/org/odk/collect/android/widgets/utilities/ActivityGeoDataRequester.kt +++ b/collect_app/src/main/java/org/odk/collect/android/widgets/utilities/ActivityGeoDataRequester.kt @@ -98,7 +98,7 @@ class ActivityGeoDataRequester( val intent = Intent(activity, GeoPolyActivity::class.java).also { it.putExtra( GeoPolyActivity.EXTRA_POLYGON, - parseGeometry(answerText) + ArrayList(parseGeometry(answerText)) ) it.putExtra( GeoPolyActivity.OUTPUT_MODE_KEY, @@ -131,7 +131,7 @@ class ActivityGeoDataRequester( val intent = Intent(activity, GeoPolyActivity::class.java).also { it.putExtra( GeoPolyActivity.EXTRA_POLYGON, - parseGeometry(answerText) + ArrayList(parseGeometry(answerText)) ) it.putExtra( GeoPolyActivity.OUTPUT_MODE_KEY, diff --git a/geo/src/main/java/org/odk/collect/geo/GeoUtils.kt b/geo/src/main/java/org/odk/collect/geo/GeoUtils.kt index 8900613403d..dd05fc14dcb 100644 --- a/geo/src/main/java/org/odk/collect/geo/GeoUtils.kt +++ b/geo/src/main/java/org/odk/collect/geo/GeoUtils.kt @@ -36,7 +36,7 @@ object GeoUtils { ) } - return removeEnd(result.toString().trim { it <= ' ' }, ";") + return removeEnd(result.toString().trim(), ";") } @JvmStatic @@ -66,8 +66,8 @@ object GeoUtils { @JvmStatic @JvmOverloads fun parseGeometryPoint(answer: String?, strict: Boolean = false): DoubleArray? { - if (answer != null && answer.isNotEmpty()) { - val sa = answer.trim { it <= ' ' }.split(" ").toTypedArray() + if (!answer.isNullOrEmpty()) { + val sa = answer.trim().split(" ") return try { doubleArrayOf( sa[0].toDouble(), diff --git a/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt b/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt index b40c737f2d7..547ddda8423 100644 --- a/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt +++ b/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt @@ -5,10 +5,10 @@ import org.odk.collect.maps.MapPoint object GeoPolyUtils { - fun parseGeometry(geometry: String?, strict: Boolean = false): ArrayList { + fun parseGeometry(geometry: String?, strict: Boolean = false): List { val points = ArrayList() - for (vertex in (geometry ?: "").split(";").toTypedArray()) { + for (vertex in (geometry ?: "").split(";")) { val point = parseGeometryPoint(vertex, strict) if (point != null) { points.add(MapPoint(point[0], point[1], point[2], point[3])) From d21470cbb603a6870d1bce993783f02077b8b6cf Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Tue, 18 Nov 2025 15:07:55 +0000 Subject: [PATCH 13/43] Use zipWithNext to simplify creating segments --- .../java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt b/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt index 547ddda8423..7b3ebdbd41f 100644 --- a/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt +++ b/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt @@ -25,7 +25,7 @@ object GeoPolyUtils { */ fun intersects(trace: List): Boolean { return if (trace.size >= 3) { - val segments = getSegments(trace) + val segments = trace.zipWithNext() segments.any { line1 -> segments.any { line2 -> intersects(line1, line2) @@ -36,16 +36,6 @@ object GeoPolyUtils { } } - private fun getSegments(trace: List): List> { - return trace.flatMapIndexed { index, point -> - if (index != trace.size - 1) { - listOf(Pair(point, trace[index + 1])) - } else { - emptyList() - } - } - } - /** * Work out whether two line segments intersect by calculating if the endpoints of one segment * are on opposite sides of the other segment **and** vice versa. This is determined by finding From 7256252df3a7149fc4936821dae4f39e795b535d Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Tue, 18 Nov 2025 15:18:56 +0000 Subject: [PATCH 14/43] Add test to check arg length is enforced --- .../javarosa/IntersectsFunctionHandlerTest.kt | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/collect_app/src/test/java/org/odk/collect/geo/javarosa/IntersectsFunctionHandlerTest.kt b/collect_app/src/test/java/org/odk/collect/geo/javarosa/IntersectsFunctionHandlerTest.kt index 978ccf3015b..c5c4e594e2d 100644 --- a/collect_app/src/test/java/org/odk/collect/geo/javarosa/IntersectsFunctionHandlerTest.kt +++ b/collect_app/src/test/java/org/odk/collect/geo/javarosa/IntersectsFunctionHandlerTest.kt @@ -133,4 +133,43 @@ class IntersectsFunctionHandlerTest { assertThat(e.cause, instanceOf(expected)) } } + + @Test + fun `throws exception when passed too many args`() { + val expected = XPathTypeMismatchException::class.java + try { + Scenario.init( + "Intersects form", + html( + head( + title("Intersects form"), + model( + mainInstance( + t( + "data id=\"intersects-form\"", + t("question"), + t("calculate") + ) + ), + bind("/data/question").type("geotrace"), + bind("/data/calculate").type("boolean") + .calculate("intersects(/data/question,/data/question)") + ) + ), + body( + input("/data/question"), + input("/data/calculate") + ) + ) + ) { formDef -> + FormEntryController(FormEntryModel(formDef)).also { + it.addFunctionHandler(IntersectsFunctionHandler()) + } + } + + fail("Expected exception: $expected") + } catch (e: Exception) { + assertThat(e.cause, instanceOf(expected)) + } + } } From 77bec29ad9cb8952e8c18c1793329103df535dcf Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Tue, 18 Nov 2025 17:11:00 +0000 Subject: [PATCH 15/43] Switch test to Hamcrest --- .../java/org/odk/collect/geo/GeoUtilsTest.kt | 47 +++++++++---------- 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/geo/src/test/java/org/odk/collect/geo/GeoUtilsTest.kt b/geo/src/test/java/org/odk/collect/geo/GeoUtilsTest.kt index 8a276c99404..16e54694d36 100644 --- a/geo/src/test/java/org/odk/collect/geo/GeoUtilsTest.kt +++ b/geo/src/test/java/org/odk/collect/geo/GeoUtilsTest.kt @@ -4,7 +4,6 @@ import android.content.Context import android.location.Location import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 -import junit.framework.TestCase import org.hamcrest.MatcherAssert.assertThat import org.hamcrest.Matchers.equalTo import org.junit.Assert.assertEquals @@ -27,30 +26,30 @@ class GeoUtilsTest { @Test fun whenPointsAreNull_formatPoints_returnsEmptyString() { - assertEquals(formatPointsResultString(mutableListOf(), true), "") - assertEquals(formatPointsResultString(mutableListOf(), false), "") + assertThat(formatPointsResultString(mutableListOf(), true), equalTo("")) + assertThat(formatPointsResultString(mutableListOf(), false), equalTo("")) } @Test fun geotraces_areSeparatedBySemicolon_withoutTrialingSemicolon() { - assertEquals( + assertThat( formatPointsResultString(points, false), - "11.0 12.0 13.0 14.0;21.0 22.0 23.0 24.0;31.0 32.0 33.0 34.0" + equalTo("11.0 12.0 13.0 14.0;21.0 22.0 23.0 24.0;31.0 32.0 33.0 34.0") ) } @Test fun geoshapes_areSeparatedBySemicolon_withoutTrialingSemicolon_andHaveMatchingFirstAndLastPoints() { - assertEquals( + assertThat( formatPointsResultString(points, true), - "11.0 12.0 13.0 14.0;21.0 22.0 23.0 24.0;31.0 32.0 33.0 34.0;11.0 12.0 13.0 14.0" + equalTo("11.0 12.0 13.0 14.0;21.0 22.0 23.0 24.0;31.0 32.0 33.0 34.0;11.0 12.0 13.0 14.0") ) } @Test fun test_formatLocationResultString() { val location: Location = createLocation("GPS", 1.0, 2.0, 3.0, 4f) - assertEquals(GeoUtils.formatLocationResultString(location), "1.0 2.0 3.0 4.0") + assertThat(GeoUtils.formatLocationResultString(location), equalTo("1.0 2.0 3.0 4.0")) } @Test @@ -66,28 +65,28 @@ class GeoUtilsTest { fun parseGeometryPointTest() { var gp = parseGeometryPoint("37.45153333333334 -122.15539166666667 0.0 20.0")!! - TestCase.assertEquals(37.45153333333334, gp[0]) - TestCase.assertEquals(-122.15539166666667, gp[1]) - TestCase.assertEquals(0.0, gp[2]) - TestCase.assertEquals(20.0, gp[3]) + assertThat(37.45153333333334, equalTo(gp[0])) + assertThat(-122.15539166666667, equalTo(gp[1])) + assertThat(0.0, equalTo(gp[2])) + assertThat(20.0, equalTo(gp[3])) gp = parseGeometryPoint("37.45153333333334")!! - TestCase.assertEquals(37.45153333333334, gp[0]) - TestCase.assertEquals(0.0, gp[1]) - TestCase.assertEquals(0.0, gp[2]) - TestCase.assertEquals(0.0, gp[3]) + assertThat(37.45153333333334, equalTo(gp[0])) + assertThat(0.0, equalTo(gp[1])) + assertThat(0.0, equalTo(gp[2])) + assertThat(0.0, equalTo(gp[3])) gp = parseGeometryPoint(" 37.45153333333334 -122.15539166666667 0.0 ")!! - TestCase.assertEquals(37.45153333333334, gp[0]) - TestCase.assertEquals(-122.15539166666667, gp[1]) - TestCase.assertEquals(0.0, gp[2]) - TestCase.assertEquals(0.0, gp[3]) + assertThat(37.45153333333334, equalTo(gp[0])) + assertThat(-122.15539166666667, equalTo(gp[1])) + assertThat(0.0, equalTo(gp[2])) + assertThat(0.0, equalTo(gp[3])) - TestCase.assertEquals( + assertThat( null, - parseGeometryPoint("37.45153333333334 -122.15539166666667 0.0 qwerty") + equalTo(parseGeometryPoint("37.45153333333334 -122.15539166666667 0.0 qwerty")) ) - TestCase.assertEquals(null, parseGeometryPoint("")) - TestCase.assertEquals(null, parseGeometryPoint(null)) + assertThat(null, equalTo(parseGeometryPoint(""))) + assertThat(null, equalTo(parseGeometryPoint(null))) } } From 2a3cd02670e85b0c9cc5f751ef52203165fbcff2 Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Wed, 19 Nov 2025 09:55:38 +0000 Subject: [PATCH 16/43] Remove unused imports --- .editorconfig | 1 + .../android/widgets/items/SelectOneFromMapDialogFragment.kt | 1 - geo/src/test/java/org/odk/collect/geo/GeoUtilsTest.kt | 1 - .../test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt | 2 -- 4 files changed, 1 insertion(+), 4 deletions(-) diff --git a/.editorconfig b/.editorconfig index 95581296277..248df6926e9 100644 --- a/.editorconfig +++ b/.editorconfig @@ -29,3 +29,4 @@ ktlint_standard_condition-wrapping = disabled ktlint_standard_function-literal = disabled ktlint_standard_backing-property-naming = disabled ktlint_function_naming_ignore_when_annotated_with = Composable +ktlint_standard_no-unused-imports=enabled diff --git a/collect_app/src/main/java/org/odk/collect/android/widgets/items/SelectOneFromMapDialogFragment.kt b/collect_app/src/main/java/org/odk/collect/android/widgets/items/SelectOneFromMapDialogFragment.kt index 99f7c3f38e0..6c4278fdcfc 100644 --- a/collect_app/src/main/java/org/odk/collect/android/widgets/items/SelectOneFromMapDialogFragment.kt +++ b/collect_app/src/main/java/org/odk/collect/android/widgets/items/SelectOneFromMapDialogFragment.kt @@ -27,7 +27,6 @@ import org.odk.collect.androidshared.livedata.NonNullLiveData import org.odk.collect.androidshared.ui.FragmentFactoryBuilder import org.odk.collect.async.Scheduler import org.odk.collect.entities.javarosa.parse.EntitySchema -import org.odk.collect.geo.geopoly.GeoPolyUtils import org.odk.collect.geo.geopoly.GeoPolyUtils.parseGeometry import org.odk.collect.geo.selection.IconifiedText import org.odk.collect.geo.selection.MappableSelectItem diff --git a/geo/src/test/java/org/odk/collect/geo/GeoUtilsTest.kt b/geo/src/test/java/org/odk/collect/geo/GeoUtilsTest.kt index 16e54694d36..7d9e5bcc1bc 100644 --- a/geo/src/test/java/org/odk/collect/geo/GeoUtilsTest.kt +++ b/geo/src/test/java/org/odk/collect/geo/GeoUtilsTest.kt @@ -6,7 +6,6 @@ import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 import org.hamcrest.MatcherAssert.assertThat import org.hamcrest.Matchers.equalTo -import org.junit.Assert.assertEquals import org.junit.Test import org.junit.runner.RunWith import org.odk.collect.geo.GeoUtils.formatPointsResultString diff --git a/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt b/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt index 23907ab9324..0a1252182fd 100644 --- a/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt +++ b/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt @@ -1,10 +1,8 @@ package org.odk.collect.geo.geopoly -import junit.framework.TestCase.assertEquals import org.hamcrest.MatcherAssert.assertThat import org.hamcrest.Matchers.equalTo import org.junit.Test -import org.odk.collect.geo.GeoUtils.parseGeometryPoint import org.odk.collect.geo.geopoly.GeoPolyUtils.parseGeometry import org.odk.collect.maps.MapPoint From da8e9a7158fceb943202064a4475231370e3d44d Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Wed, 19 Nov 2025 17:46:30 +0000 Subject: [PATCH 17/43] Add another test case to make sure we check intersections from both segments --- .../org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt b/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt index 0a1252182fd..c58e77b67dc 100644 --- a/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt +++ b/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt @@ -87,6 +87,18 @@ class GeoPolyUtilsTest { assertThat(GeoPolyUtils.intersects(trace), equalTo(true)) } + @Test + fun `#intersects returns false when a segment's end points are both on different sides of another, but the segments do not intersect`() { + val trace = listOf( + MapPoint(1.0, 1.0), + MapPoint(1.0, 2.0), + MapPoint(3.0, 3.0), + MapPoint(0.0, 3.0) + ) + + assertThat(GeoPolyUtils.intersects(trace), equalTo(false)) + } + @Test fun parseGeometryTest() { assertThat(parseGeometry("1.0 2.0 3 4"), equalTo(listOf(MapPoint(1.0, 2.0, 3.0, 4.0)))) From d853d05688aab67ff1ef2055a1e900ab26262aac Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Thu, 20 Nov 2025 11:10:00 +0000 Subject: [PATCH 18/43] Add failing tests for non crossing intersections --- .../collect/geo/geopoly/GeoPolyUtilsTest.kt | 35 ++++++++++++------- 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt b/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt index c58e77b67dc..68035c73336 100644 --- a/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt +++ b/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt @@ -25,18 +25,6 @@ class GeoPolyUtilsTest { assertThat(GeoPolyUtils.intersects(trace), equalTo(false)) } - @Test - fun `#intersects returns false when there is just two of the same segment`() { - val trace = listOf( - MapPoint(0.0, 0.0), - MapPoint(1.0, 0.0), - MapPoint(1.0, 0.0), - MapPoint(0.0, 0.0) - ) - - assertThat(GeoPolyUtils.intersects(trace), equalTo(false)) - } - @Test fun `#intersects returns false when no segment intersects with another`() { val trace = listOf( @@ -99,6 +87,29 @@ class GeoPolyUtilsTest { assertThat(GeoPolyUtils.intersects(trace), equalTo(false)) } + @Test + fun `#intersects returns true when just an endpoint touches another segment`() { + val trace = listOf( + MapPoint(0.0, 0.0), + MapPoint(1.0, 1.0), + MapPoint(2.0, 0.0), + MapPoint(-1.0, 0.0) + ) + + assertThat(GeoPolyUtils.intersects(trace), equalTo(true)) + } + + @Test + fun `#intersects returns true when a segment is collinear and within another`() { + val trace = listOf( + MapPoint(0.0, 0.0), + MapPoint(0.0, 1.0), + MapPoint(0.0, 0.0) + ) + + assertThat(GeoPolyUtils.intersects(trace), equalTo(true)) + } + @Test fun parseGeometryTest() { assertThat(parseGeometry("1.0 2.0 3 4"), equalTo(listOf(MapPoint(1.0, 2.0, 3.0, 4.0)))) From 292b7be0d3cf808bbaef5fda321e84fbfc9f69e4 Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Thu, 20 Nov 2025 14:32:40 +0000 Subject: [PATCH 19/43] Rename method --- .../main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt b/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt index 7b3ebdbd41f..72d9a52969a 100644 --- a/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt +++ b/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt @@ -28,7 +28,7 @@ object GeoPolyUtils { val segments = trace.zipWithNext() segments.any { line1 -> segments.any { line2 -> - intersects(line1, line2) + crosses(line1, line2) } } } else { @@ -37,11 +37,11 @@ object GeoPolyUtils { } /** - * Work out whether two line segments intersect by calculating if the endpoints of one segment + * Work out whether two line segments cross by calculating if the endpoints of one segment * are on opposite sides of the other segment **and** vice versa. This is determined by finding * the orientation of endpoints relative to the other line. */ - private fun intersects( + private fun crosses( segment1: Pair, segment2: Pair ): Boolean { From e07472976de823a89e63fa3a9773d24bcf36e66e Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Thu, 20 Nov 2025 14:52:25 +0000 Subject: [PATCH 20/43] Fix origin of one segment touching another case --- .../odk/collect/geo/geopoly/GeoPolyUtils.kt | 43 ++++++++++++++++--- 1 file changed, 36 insertions(+), 7 deletions(-) diff --git a/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt b/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt index 72d9a52969a..85b977bfea0 100644 --- a/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt +++ b/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt @@ -2,6 +2,8 @@ package org.odk.collect.geo.geopoly import org.odk.collect.geo.GeoUtils.parseGeometryPoint import org.odk.collect.maps.MapPoint +import kotlin.math.max +import kotlin.math.min object GeoPolyUtils { @@ -24,18 +26,45 @@ object GeoPolyUtils { * Returns `true` if any segment of the trace intersects with any other and `false` otherwise. */ fun intersects(trace: List): Boolean { - return if (trace.size >= 3) { - val segments = trace.zipWithNext() - segments.any { line1 -> - segments.any { line2 -> - crosses(line1, line2) - } - } + val unclosedTrace = if (trace.isNotEmpty() && trace.first() == trace.last()) { + trace.dropLast(1) + } else { + trace + } + + return if (unclosedTrace.size >= 3) { + val segments = unclosedTrace.zipWithNext() + segments.filterIndexed { line1Index, line1 -> + segments.filterIndexed { line2Index, line2 -> + if (line2Index >= line1Index + 2) { + crosses(line1, line2) || within(line1.first, line2) + } else { + false // Only check following (non-neighbour) segments + } + }.isNotEmpty() + }.isNotEmpty() } else { false } } + /** + * Check if a point is within a line + */ + fun within( + point: MapPoint, + line: Pair + ): Boolean { + val lineLatMin = min(line.first.latitude, line.second.latitude) + val lineLatMax = max(line.first.latitude, line.second.latitude) + val lineLongMin = min(line.first.longitude, line.second.longitude) + val lineLongMax = max(line.first.longitude, line.second.longitude) + val latRange = lineLatMin..lineLatMax + val longRange = lineLongMin..lineLongMax + + return point.latitude in latRange && point.longitude in longRange + } + /** * Work out whether two line segments cross by calculating if the endpoints of one segment * are on opposite sides of the other segment **and** vice versa. This is determined by finding From 7055bdca972c317984a57d50d0a9c9d4d80fc7fd Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Thu, 20 Nov 2025 14:55:14 +0000 Subject: [PATCH 21/43] Correct test The example before would be considered closed and so wouldn't be classes as intersecting --- .../test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt b/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt index 68035c73336..3154925f465 100644 --- a/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt +++ b/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt @@ -104,7 +104,7 @@ class GeoPolyUtilsTest { val trace = listOf( MapPoint(0.0, 0.0), MapPoint(0.0, 1.0), - MapPoint(0.0, 0.0) + MapPoint(0.0, 0.5) ) assertThat(GeoPolyUtils.intersects(trace), equalTo(true)) From 527ae8ef2cb76f46c8674f66081506e65a0ce267 Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Thu, 20 Nov 2025 15:03:11 +0000 Subject: [PATCH 22/43] Fix line moving back on itself case --- geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt b/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt index 85b977bfea0..3a374b12a38 100644 --- a/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt +++ b/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt @@ -38,8 +38,10 @@ object GeoPolyUtils { segments.filterIndexed { line2Index, line2 -> if (line2Index >= line1Index + 2) { crosses(line1, line2) || within(line1.first, line2) + } else if (line2Index == line1Index + 1) { + within(line2.second, line1) } else { - false // Only check following (non-neighbour) segments + false } }.isNotEmpty() }.isNotEmpty() From e7a86625bb8db25dd1776656be6b6174ee1f91ae Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Thu, 20 Nov 2025 15:16:52 +0000 Subject: [PATCH 23/43] Add test for shape that closes outside the origin --- .../odk/collect/geo/geopoly/GeoPolyUtilsTest.kt | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt b/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt index 3154925f465..ce3a88a06ab 100644 --- a/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt +++ b/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt @@ -110,6 +110,20 @@ class GeoPolyUtilsTest { assertThat(GeoPolyUtils.intersects(trace), equalTo(true)) } + @Test + fun `#intersects returns true when the trace closes on a non-origin vertex`() { + val trace = listOf( + MapPoint(0.0, 0.0), + MapPoint(0.0, 1.0), + MapPoint(0.0, 2.0), + MapPoint(1.0, 2.0), + MapPoint(1.0, 1.0), + MapPoint(0.0, 1.0) + ) + + assertThat(GeoPolyUtils.intersects(trace), equalTo(true)) + } + @Test fun parseGeometryTest() { assertThat(parseGeometry("1.0 2.0 3 4"), equalTo(listOf(MapPoint(1.0, 2.0, 3.0, 4.0)))) From 0800a0e69d5d582af798454cad7317f90bbeffbd Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Fri, 21 Nov 2025 12:08:03 +0000 Subject: [PATCH 24/43] Simplify non-origin vertex closing --- .../test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt b/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt index ce3a88a06ab..e3c56833e35 100644 --- a/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt +++ b/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt @@ -114,10 +114,9 @@ class GeoPolyUtilsTest { fun `#intersects returns true when the trace closes on a non-origin vertex`() { val trace = listOf( MapPoint(0.0, 0.0), - MapPoint(0.0, 1.0), + MapPoint(0.0, 1.0), // Close back on this point MapPoint(0.0, 2.0), MapPoint(1.0, 2.0), - MapPoint(1.0, 1.0), MapPoint(0.0, 1.0) ) From 602fee7299cd446eaa04fa0fea5652e45147753b Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Fri, 21 Nov 2025 13:07:26 +0000 Subject: [PATCH 25/43] Use bounding box check based on orientation to solve right angled triangle case --- .../odk/collect/geo/geopoly/GeoPolyUtils.kt | 52 +++++++++++-------- .../collect/geo/geopoly/GeoPolyUtilsTest.kt | 11 ++++ 2 files changed, 40 insertions(+), 23 deletions(-) diff --git a/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt b/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt index 3a374b12a38..b590e61fb59 100644 --- a/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt +++ b/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt @@ -26,20 +26,15 @@ object GeoPolyUtils { * Returns `true` if any segment of the trace intersects with any other and `false` otherwise. */ fun intersects(trace: List): Boolean { - val unclosedTrace = if (trace.isNotEmpty() && trace.first() == trace.last()) { - trace.dropLast(1) - } else { - trace - } - - return if (unclosedTrace.size >= 3) { - val segments = unclosedTrace.zipWithNext() + return if (trace.size >= 3) { + val isClosed = trace.isNotEmpty() && trace.first() == trace.last() + val segments = trace.zipWithNext() segments.filterIndexed { line1Index, line1 -> segments.filterIndexed { line2Index, line2 -> - if (line2Index >= line1Index + 2) { - crosses(line1, line2) || within(line1.first, line2) - } else if (line2Index == line1Index + 1) { - within(line2.second, line1) + if (isClosed && line1Index == 0 && line2Index == segments.size - 1) { + false + } else if (line2Index >= line1Index + 2) { + intersects(line1, line2) } else { false } @@ -51,7 +46,7 @@ object GeoPolyUtils { } /** - * Check if a point is within a line + * Check if a point is within the bounding box of a line */ fun within( point: MapPoint, @@ -68,18 +63,29 @@ object GeoPolyUtils { } /** - * Work out whether two line segments cross by calculating if the endpoints of one segment - * are on opposite sides of the other segment **and** vice versa. This is determined by finding - * the orientation of endpoints relative to the other line. + * Work out whether two line segments intersect by calculating if the endpoints of one segment + * are on opposite sides (or touching of the other segment **and** vice versa. This is + * determined by finding the orientation of endpoints relative to the other line. */ - private fun crosses( - segment1: Pair, - segment2: Pair + private fun intersects( + aB: Pair, + cD: Pair ): Boolean { - return orientation(segment1.first, segment2.first, segment2.second) - .isOpposing(orientation(segment1.second, segment2.first, segment2.second)) && - orientation(segment1.first, segment1.second, segment2.first) - .isOpposing(orientation(segment1.first, segment1.second, segment2.second)) + val (a, b) = aB + val (c, d) = cD + + val orientationA = orientation(a, c, d) + val orientationB = orientation(b, c, d) + val orientationC = orientation(a, b, c) + val orientationD = orientation(a, b, d) + + return if (orientationA.isOpposing(orientationB) && orientationC.isOpposing(orientationD)) { + true + } else if (orientationA == Orientation.Collinear && within(a, cD)) { + true + } else { + false + } } /** diff --git a/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt b/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt index e3c56833e35..62e1160f88d 100644 --- a/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt +++ b/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt @@ -123,6 +123,17 @@ class GeoPolyUtilsTest { assertThat(GeoPolyUtils.intersects(trace), equalTo(true)) } + @Test + fun `#intersects returns false for right angled triangle`() { + val trace = listOf( + MapPoint(0.0, 0.0), + MapPoint(10.0, 10.0), + MapPoint(0.0, 10.0) + ) + + assertThat(GeoPolyUtils.intersects(trace), equalTo(false)) + } + @Test fun parseGeometryTest() { assertThat(parseGeometry("1.0 2.0 3 4"), equalTo(listOf(MapPoint(1.0, 2.0, 3.0, 4.0)))) From ad81da925efa1197c9de38b0c48500d4b9a3312d Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Fri, 21 Nov 2025 13:50:06 +0000 Subject: [PATCH 26/43] Fix special case with two segmnt intersection --- .../odk/collect/geo/geopoly/GeoPolyUtils.kt | 28 +++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt b/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt index b590e61fb59..f760406cbe6 100644 --- a/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt +++ b/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt @@ -27,19 +27,25 @@ object GeoPolyUtils { */ fun intersects(trace: List): Boolean { return if (trace.size >= 3) { - val isClosed = trace.isNotEmpty() && trace.first() == trace.last() + val isClosed = trace.first() == trace.last() val segments = trace.zipWithNext() - segments.filterIndexed { line1Index, line1 -> - segments.filterIndexed { line2Index, line2 -> - if (isClosed && line1Index == 0 && line2Index == segments.size - 1) { - false - } else if (line2Index >= line1Index + 2) { - intersects(line1, line2) - } else { - false - } + + return if (segments.size == 2) { + val orientation = orientation(segments[1].second, segments[0].first, segments[0].second) + return orientation == Orientation.Collinear && within(segments[1].second, segments[0]) + } else { + segments.filterIndexed { line1Index, line1 -> + segments.filterIndexed { line2Index, line2 -> + if (isClosed && line1Index == 0 && line2Index == segments.size - 1) { + false + } else if (line2Index >= line1Index + 2) { + intersects(line1, line2) + } else { + false + } + }.isNotEmpty() }.isNotEmpty() - }.isNotEmpty() + } } else { false } From 69d4c8a6199c27fa7338167ebf8ded301e03c107 Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Fri, 21 Nov 2025 14:42:39 +0000 Subject: [PATCH 27/43] Pull out specific 2D geometry code --- .../odk/collect/geo/geopoly/GeoPolyUtils.kt | 120 -------------- .../geo/javarosa/IntersectsFunctionHandler.kt | 9 +- .../collect/geo/geopoly/GeoPolyUtilsTest.kt | 126 --------------- .../java/org/odk/collect/maps/MapPoint.kt | 5 + .../odk/collect/shared/geometry/Geometry.kt | 124 ++++++++++++++ .../collect/shared/geometry/GeometryTest.kt | 151 ++++++++++++++++++ 6 files changed, 286 insertions(+), 249 deletions(-) create mode 100644 shared/src/main/java/org/odk/collect/shared/geometry/Geometry.kt create mode 100644 shared/src/test/java/org/odk/collect/shared/geometry/GeometryTest.kt diff --git a/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt b/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt index f760406cbe6..ed2cc3953df 100644 --- a/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt +++ b/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyUtils.kt @@ -2,8 +2,6 @@ package org.odk.collect.geo.geopoly import org.odk.collect.geo.GeoUtils.parseGeometryPoint import org.odk.collect.maps.MapPoint -import kotlin.math.max -import kotlin.math.min object GeoPolyUtils { @@ -21,122 +19,4 @@ object GeoPolyUtils { return points } - - /** - * Returns `true` if any segment of the trace intersects with any other and `false` otherwise. - */ - fun intersects(trace: List): Boolean { - return if (trace.size >= 3) { - val isClosed = trace.first() == trace.last() - val segments = trace.zipWithNext() - - return if (segments.size == 2) { - val orientation = orientation(segments[1].second, segments[0].first, segments[0].second) - return orientation == Orientation.Collinear && within(segments[1].second, segments[0]) - } else { - segments.filterIndexed { line1Index, line1 -> - segments.filterIndexed { line2Index, line2 -> - if (isClosed && line1Index == 0 && line2Index == segments.size - 1) { - false - } else if (line2Index >= line1Index + 2) { - intersects(line1, line2) - } else { - false - } - }.isNotEmpty() - }.isNotEmpty() - } - } else { - false - } - } - - /** - * Check if a point is within the bounding box of a line - */ - fun within( - point: MapPoint, - line: Pair - ): Boolean { - val lineLatMin = min(line.first.latitude, line.second.latitude) - val lineLatMax = max(line.first.latitude, line.second.latitude) - val lineLongMin = min(line.first.longitude, line.second.longitude) - val lineLongMax = max(line.first.longitude, line.second.longitude) - val latRange = lineLatMin..lineLatMax - val longRange = lineLongMin..lineLongMax - - return point.latitude in latRange && point.longitude in longRange - } - - /** - * Work out whether two line segments intersect by calculating if the endpoints of one segment - * are on opposite sides (or touching of the other segment **and** vice versa. This is - * determined by finding the orientation of endpoints relative to the other line. - */ - private fun intersects( - aB: Pair, - cD: Pair - ): Boolean { - val (a, b) = aB - val (c, d) = cD - - val orientationA = orientation(a, c, d) - val orientationB = orientation(b, c, d) - val orientationC = orientation(a, b, c) - val orientationD = orientation(a, b, d) - - return if (orientationA.isOpposing(orientationB) && orientationC.isOpposing(orientationD)) { - true - } else if (orientationA == Orientation.Collinear && within(a, cD)) { - true - } else { - false - } - } - - /** - * Calculate the "orientation" (or "direction") of three points using the cross product of the - * vectors of the pairs of points (see - * [here](https://en.wikipedia.org/wiki/Cross_product#Computational_geometry)). This can - * either be clockwise, anticlockwise or collinear (the three points form a straight line). - * - */ - private fun orientation(a: MapPoint, b: MapPoint, c: MapPoint): Orientation { - val ab = Pair(b.latitude - a.latitude, b.longitude - a.longitude) - val ac = Pair(c.latitude - a.latitude, c.longitude - a.longitude) - val crossProduct = crossProduct(ab, ac) - - return if (crossProduct > 0) { - Orientation.AntiClockwise - } else if (crossProduct < 0) { - Orientation.Clockwise - } else { - Orientation.Collinear - } - } - - /** - * [https://en.wikipedia.org/wiki/Cross_product](https://en.wikipedia.org/wiki/Cross_product) - */ - private fun crossProduct(x: Pair, y: Pair): Double { - return (x.first * y.second) - (y.first * x.second) - } - - private enum class Orientation { - Collinear, - Clockwise, - AntiClockwise; - - fun isOpposing(other: Orientation): Boolean { - return if (this == Collinear) { - false - } else if (this == Clockwise && other == AntiClockwise) { - true - } else if (this == AntiClockwise && other == Clockwise) { - true - } else { - false - } - } - } } diff --git a/geo/src/main/java/org/odk/collect/geo/javarosa/IntersectsFunctionHandler.kt b/geo/src/main/java/org/odk/collect/geo/javarosa/IntersectsFunctionHandler.kt index a88065a3239..86ca3b12b00 100644 --- a/geo/src/main/java/org/odk/collect/geo/javarosa/IntersectsFunctionHandler.kt +++ b/geo/src/main/java/org/odk/collect/geo/javarosa/IntersectsFunctionHandler.kt @@ -3,8 +3,10 @@ package org.odk.collect.geo.javarosa import org.javarosa.core.model.condition.EvaluationContext import org.javarosa.core.model.condition.IFunctionHandler import org.javarosa.xpath.XPathTypeMismatchException -import org.odk.collect.geo.geopoly.GeoPolyUtils.intersects import org.odk.collect.geo.geopoly.GeoPolyUtils.parseGeometry +import org.odk.collect.maps.toPoint +import org.odk.collect.shared.geometry.Trace +import org.odk.collect.shared.geometry.intersects class IntersectsFunctionHandler : IFunctionHandler { override fun getName(): String { @@ -28,8 +30,9 @@ class IntersectsFunctionHandler : IFunctionHandler { ec: EvaluationContext ): Any { try { - val trace = parseGeometry(args[0] as String, strict = true) - return intersects(trace) + val mapPoints = parseGeometry(args[0] as String, strict = true) + val trace = Trace(mapPoints.map { it.toPoint() }) + return trace.intersects() } catch (_: IllegalArgumentException) { throw XPathTypeMismatchException() } diff --git a/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt b/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt index 62e1160f88d..cb36c610308 100644 --- a/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt +++ b/geo/src/test/java/org/odk/collect/geo/geopoly/GeoPolyUtilsTest.kt @@ -8,132 +8,6 @@ import org.odk.collect.maps.MapPoint class GeoPolyUtilsTest { - @Test - fun `#intersects returns false for an empty list`() { - assertThat(GeoPolyUtils.intersects(emptyList()), equalTo(false)) - } - - @Test - fun `#intersects returns false when there is only one point`() { - val trace = listOf(MapPoint(0.0, 0.0)) - assertThat(GeoPolyUtils.intersects(trace), equalTo(false)) - } - - @Test - fun `#intersects returns false when there is only one segment`() { - val trace = listOf(MapPoint(0.0, 0.0), MapPoint(1.0, 0.0)) - assertThat(GeoPolyUtils.intersects(trace), equalTo(false)) - } - - @Test - fun `#intersects returns false when no segment intersects with another`() { - val trace = listOf( - MapPoint(0.0, 0.0), - MapPoint(1.0, 1.0), - MapPoint(2.0, 0.0) - ) - - assertThat(GeoPolyUtils.intersects(trace), equalTo(false)) - } - - @Test - fun `#intersects returns false when no segment intersects with another in a closed trace`() { - val trace = listOf( - MapPoint(0.0, 0.0), - MapPoint(1.0, 1.0), - MapPoint(2.0, 0.0), - MapPoint(0.0, 0.0) - ) - - assertThat(GeoPolyUtils.intersects(trace), equalTo(false)) - } - - @Test - fun `#intersects returns true when a segment intersects with another`() { - val trace = listOf( - MapPoint(1.0, 1.0), - MapPoint(1.0, 3.0), - MapPoint(2.0, 3.0), - MapPoint(2.0, 2.0), - MapPoint(0.0, 2.0) - ) - - assertThat(GeoPolyUtils.intersects(trace), equalTo(true)) - } - - @Test - fun `#intersects returns true when a segment intersects with another in a closed trace`() { - val trace = listOf( - MapPoint(1.0, 1.0), - MapPoint(1.0, 3.0), - MapPoint(2.0, 3.0), - MapPoint(2.0, 2.0), - MapPoint(0.0, 2.0), - MapPoint(1.0, 1.0) - ) - - assertThat(GeoPolyUtils.intersects(trace), equalTo(true)) - } - - @Test - fun `#intersects returns false when a segment's end points are both on different sides of another, but the segments do not intersect`() { - val trace = listOf( - MapPoint(1.0, 1.0), - MapPoint(1.0, 2.0), - MapPoint(3.0, 3.0), - MapPoint(0.0, 3.0) - ) - - assertThat(GeoPolyUtils.intersects(trace), equalTo(false)) - } - - @Test - fun `#intersects returns true when just an endpoint touches another segment`() { - val trace = listOf( - MapPoint(0.0, 0.0), - MapPoint(1.0, 1.0), - MapPoint(2.0, 0.0), - MapPoint(-1.0, 0.0) - ) - - assertThat(GeoPolyUtils.intersects(trace), equalTo(true)) - } - - @Test - fun `#intersects returns true when a segment is collinear and within another`() { - val trace = listOf( - MapPoint(0.0, 0.0), - MapPoint(0.0, 1.0), - MapPoint(0.0, 0.5) - ) - - assertThat(GeoPolyUtils.intersects(trace), equalTo(true)) - } - - @Test - fun `#intersects returns true when the trace closes on a non-origin vertex`() { - val trace = listOf( - MapPoint(0.0, 0.0), - MapPoint(0.0, 1.0), // Close back on this point - MapPoint(0.0, 2.0), - MapPoint(1.0, 2.0), - MapPoint(0.0, 1.0) - ) - - assertThat(GeoPolyUtils.intersects(trace), equalTo(true)) - } - - @Test - fun `#intersects returns false for right angled triangle`() { - val trace = listOf( - MapPoint(0.0, 0.0), - MapPoint(10.0, 10.0), - MapPoint(0.0, 10.0) - ) - - assertThat(GeoPolyUtils.intersects(trace), equalTo(false)) - } - @Test fun parseGeometryTest() { assertThat(parseGeometry("1.0 2.0 3 4"), equalTo(listOf(MapPoint(1.0, 2.0, 3.0, 4.0)))) diff --git a/maps/src/main/java/org/odk/collect/maps/MapPoint.kt b/maps/src/main/java/org/odk/collect/maps/MapPoint.kt index 9260d8ccf9d..844c231ad60 100644 --- a/maps/src/main/java/org/odk/collect/maps/MapPoint.kt +++ b/maps/src/main/java/org/odk/collect/maps/MapPoint.kt @@ -15,6 +15,7 @@ package org.odk.collect.maps import android.os.Parcelable import kotlinx.parcelize.Parcelize +import org.odk.collect.shared.geometry.Point @Parcelize data class MapPoint @JvmOverloads constructor( @@ -23,3 +24,7 @@ data class MapPoint @JvmOverloads constructor( @JvmField val altitude: Double = 0.0, @JvmField val accuracy: Double = 0.0 ) : Parcelable + +fun MapPoint.toPoint(): Point { + return Point(latitude, longitude) +} 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 new file mode 100644 index 00000000000..e728e0e5acc --- /dev/null +++ b/shared/src/main/java/org/odk/collect/shared/geometry/Geometry.kt @@ -0,0 +1,124 @@ +package org.odk.collect.shared.geometry + +import kotlin.math.max +import kotlin.math.min + +data class Point(val x: Double, val y: Double) +data class LineSegment(val start: Point, val end: Point) +data class Trace(val points: List) { + fun isClosed(): Boolean { + return points.first() == points.last() + } +} + +fun Trace.segments(): List { + return points.zipWithNext().map { (start, end) -> LineSegment(start, end) } +} + +/** + * Returns `true` if any segment of the trace intersects with any other and `false` otherwise. + */ +fun Trace.intersects(): Boolean { + val points = this.points + return if (points.size >= 3) { + val segments = segments() + return if (segments.size == 2) { + val orientation = orientation(segments[1].end, segments[0].start, segments[0].end) + return orientation == Orientation.Collinear && segments[1].end.within(segments[0]) + } else { + segments.filterIndexed { line1Index, line1 -> + segments.filterIndexed { line2Index, line2 -> + if (isClosed() && line1Index == 0 && line2Index == segments.size - 1) { + false + } else if (line2Index >= line1Index + 2) { + line1.intersects(line2) + } else { + false + } + }.isNotEmpty() + }.isNotEmpty() + } + } else { + false + } +} + +/** + * Check if a point is within the bounding box defined by a line between two non-consecutive corners + */ +fun Point.within(segment: LineSegment): Boolean { + val lineXMin = min(segment.start.x, segment.end.x) + val lineXMax = max(segment.start.x, segment.end.x) + val lineYMin = min(segment.start.y, segment.end.y) + val lineYMax = max(segment.start.y, segment.end.y) + val xRange = lineXMin..lineXMax + val yRange = lineYMin..lineYMax + + return x in xRange && y in yRange +} + +/** + * Work out whether two line segments intersect by calculating if the endpoints of one segment + * are on opposite sides (or touching of the other segment **and** vice versa. This is + * determined by finding the orientation of endpoints relative to the other line. + */ +private fun LineSegment.intersects(other: LineSegment): Boolean { + val (a, b) = this + val (c, d) = other + + val orientationA = orientation(a, c, d) + val orientationB = orientation(b, c, d) + val orientationC = orientation(a, b, c) + val orientationD = orientation(a, b, d) + + return if (orientationA.isOpposing(orientationB) && orientationC.isOpposing(orientationD)) { + true + } else if (orientationA == Orientation.Collinear && a.within(other)) { + true + } else { + false + } +} + +/** + * Calculate the "orientation" (or "direction") of three points using the cross product of the + * vectors of the pairs of points (see + * [here](https://en.wikipedia.org/wiki/Cross_product#Computational_geometry)). This can + * either be clockwise, anticlockwise or collinear (the three points form a straight line). + * + */ +private fun orientation(a: Point, b: Point, c: Point): Orientation { + val crossProduct = crossProduct(Pair(b.x - a.x, b.y - a.y), Pair(c.x - a.x, c.y - a.y)) + return if (crossProduct > 0) { + Orientation.AntiClockwise + } else if (crossProduct < 0) { + Orientation.Clockwise + } else { + Orientation.Collinear + } +} + +/** + * [https://en.wikipedia.org/wiki/Cross_product](https://en.wikipedia.org/wiki/Cross_product) + */ +private fun crossProduct(x: Pair, y: Pair): Double { + return (x.first * y.second) - (y.first * x.second) +} + +private enum class Orientation { + Collinear, + Clockwise, + AntiClockwise; + + fun isOpposing(other: Orientation): Boolean { + return if (this == Collinear) { + false + } else if (this == Clockwise && other == AntiClockwise) { + true + } else if (this == AntiClockwise && other == Clockwise) { + true + } else { + false + } + } +} 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 new file mode 100644 index 00000000000..aa0a2599087 --- /dev/null +++ b/shared/src/test/java/org/odk/collect/shared/geometry/GeometryTest.kt @@ -0,0 +1,151 @@ +package org.odk.collect.shared.geometry + +import org.hamcrest.MatcherAssert.assertThat +import org.hamcrest.Matchers.equalTo +import org.junit.Test + +class GeometryTest { + @Test + fun `Trace#intersects returns false for an empty list`() { + assertThat(Trace(emptyList()).intersects(), equalTo(false)) + } + + @Test + fun `Trace#intersects returns false when there is only one point`() { + val trace = Trace(listOf(Point(0.0, 0.0))) + assertThat(trace.intersects(), equalTo(false)) + } + + @Test + fun `Trace#intersects returns false when there is only one segment`() { + val trace = Trace(listOf(Point(0.0, 0.0), Point(1.0, 0.0))) + assertThat(trace.intersects(), equalTo(false)) + } + + @Test + fun `Trace#intersects returns false when no segment intersects with another`() { + val trace = Trace( + listOf( + Point(0.0, 0.0), + Point(1.0, 1.0), + Point(2.0, 0.0) + ) + ) + + assertThat(trace.intersects(), equalTo(false)) + } + + @Test + fun `Trace#intersects returns false when no segment intersects with another in a closed trace`() { + val trace = Trace( + listOf( + Point(0.0, 0.0), + Point(1.0, 1.0), + Point(2.0, 0.0), + Point(0.0, 0.0) + ) + ) + + assertThat(trace.intersects(), equalTo(false)) + } + + @Test + fun `Trace#intersects returns true when a segment intersects with another`() { + val trace = Trace( + listOf( + Point(1.0, 1.0), + Point(1.0, 3.0), + Point(2.0, 3.0), + Point(2.0, 2.0), + Point(0.0, 2.0) + ) + ) + + assertThat(trace.intersects(), equalTo(true)) + } + + @Test + fun `Trace#intersects returns true when a segment intersects with another in a closed trace`() { + val trace = Trace( + listOf( + Point(1.0, 1.0), + Point(1.0, 3.0), + Point(2.0, 3.0), + Point(2.0, 2.0), + Point(0.0, 2.0), + Point(1.0, 1.0) + ) + ) + + assertThat(trace.intersects(), equalTo(true)) + } + + @Test + fun `Trace#intersects returns false when a segment's end points are both on different sides of another, but the segments do not intersect`() { + val trace = Trace( + listOf( + Point(1.0, 1.0), + Point(1.0, 2.0), + Point(3.0, 3.0), + Point(0.0, 3.0) + ) + ) + + assertThat(trace.intersects(), equalTo(false)) + } + + @Test + fun `Trace#intersects returns true when just an endpoint touches another segment`() { + val trace = Trace( + listOf( + Point(0.0, 0.0), + Point(1.0, 1.0), + Point(2.0, 0.0), + Point(-1.0, 0.0) + ) + ) + + assertThat(trace.intersects(), equalTo(true)) + } + + @Test + fun `Trace#intersects returns true when a segment is collinear and within another`() { + val trace = Trace( + listOf( + Point(0.0, 0.0), + Point(0.0, 1.0), + Point(0.0, 0.5) + ) + ) + + assertThat(trace.intersects(), equalTo(true)) + } + + @Test + fun `Trace#intersects returns true when the trace closes on a non-origin vertex`() { + val trace = Trace( + listOf( + Point(0.0, 0.0), + Point(0.0, 1.0), // Close back on this point + Point(0.0, 2.0), + Point(1.0, 2.0), + Point(0.0, 1.0) + ) + ) + + assertThat(trace.intersects(), equalTo(true)) + } + + @Test + fun `Trace#intersects returns false for right angled triangle`() { + val trace = Trace( + listOf( + Point(0.0, 0.0), + Point(10.0, 10.0), + Point(0.0, 10.0) + ) + ) + + assertThat(trace.intersects(), equalTo(false)) + } +} From 8860a4a049db98094d37a966a776e30ce7ffc945 Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Fri, 21 Nov 2025 15:04:20 +0000 Subject: [PATCH 28/43] Make sure checking instersection between segments is exhaustive --- .../odk/collect/shared/geometry/Geometry.kt | 8 +++++++- .../collect/shared/geometry/GeometryTest.kt | 18 ++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) 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 index e728e0e5acc..ca80891cf64 100644 --- a/shared/src/main/java/org/odk/collect/shared/geometry/Geometry.kt +++ b/shared/src/main/java/org/odk/collect/shared/geometry/Geometry.kt @@ -62,7 +62,7 @@ fun Point.within(segment: LineSegment): Boolean { * are on opposite sides (or touching of the other segment **and** vice versa. This is * determined by finding the orientation of endpoints relative to the other line. */ -private fun LineSegment.intersects(other: LineSegment): Boolean { +fun LineSegment.intersects(other: LineSegment): Boolean { val (a, b) = this val (c, d) = other @@ -75,6 +75,12 @@ private fun LineSegment.intersects(other: LineSegment): Boolean { true } else if (orientationA == Orientation.Collinear && a.within(other)) { true + } else if (orientationB == Orientation.Collinear && b.within(other)) { + true + } else if (orientationC == Orientation.Collinear && c.within(other)) { + true + } else if (orientationD == Orientation.Collinear && d.within(other)) { + true } else { false } 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 index aa0a2599087..49ad39d06a9 100644 --- a/shared/src/test/java/org/odk/collect/shared/geometry/GeometryTest.kt +++ b/shared/src/test/java/org/odk/collect/shared/geometry/GeometryTest.kt @@ -5,6 +5,7 @@ import org.hamcrest.Matchers.equalTo import org.junit.Test class GeometryTest { + @Test fun `Trace#intersects returns false for an empty list`() { assertThat(Trace(emptyList()).intersects(), equalTo(false)) @@ -148,4 +149,21 @@ class GeometryTest { assertThat(trace.intersects(), equalTo(false)) } + + @Test + fun `LineSegment#intersects detects any endpoint touching the other line`() { + val line = LineSegment(Point(0.0, 0.0), Point(0.0, 2.0)) + + val aTouching = LineSegment(Point(-1.0, 0.0), Point(1.0, 0.0)) + assertThat(line.intersects(aTouching), equalTo(true)) + + val bTouching = LineSegment(Point(-1.0, 2.0), Point(1.0, 2.0)) + assertThat(line.intersects(bTouching), equalTo(true)) + + val cTouching = LineSegment(Point(0.0, 1.0), Point(1.0, 1.0)) + assertThat(line.intersects(cTouching), equalTo(true)) + + val dTouching = LineSegment(Point(-1.0, 1.0), Point(0.0, 1.0)) + assertThat(line.intersects(dTouching), equalTo(true)) + } } From e7b2a714d77bbb310eaa98eaf314664f6117156d Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Tue, 25 Nov 2025 14:19:58 +0000 Subject: [PATCH 29/43] Remove unneeded return --- .../src/main/java/org/odk/collect/shared/geometry/Geometry.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index ca80891cf64..3d8f388cc4f 100644 --- a/shared/src/main/java/org/odk/collect/shared/geometry/Geometry.kt +++ b/shared/src/main/java/org/odk/collect/shared/geometry/Geometry.kt @@ -24,7 +24,7 @@ fun Trace.intersects(): Boolean { val segments = segments() return if (segments.size == 2) { val orientation = orientation(segments[1].end, segments[0].start, segments[0].end) - return orientation == Orientation.Collinear && segments[1].end.within(segments[0]) + orientation == Orientation.Collinear && segments[1].end.within(segments[0]) } else { segments.filterIndexed { line1Index, line1 -> segments.filterIndexed { line2Index, line2 -> From 30dbba2ea779299b51e25bc1aa48e360146837e6 Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Tue, 25 Nov 2025 15:06:20 +0000 Subject: [PATCH 30/43] Correct bounding box check direction --- .../main/java/org/odk/collect/shared/geometry/Geometry.kt | 4 ++-- .../java/org/odk/collect/shared/geometry/GeometryTest.kt | 8 ++++++++ 2 files changed, 10 insertions(+), 2 deletions(-) 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 index 3d8f388cc4f..78b8ac2ac83 100644 --- a/shared/src/main/java/org/odk/collect/shared/geometry/Geometry.kt +++ b/shared/src/main/java/org/odk/collect/shared/geometry/Geometry.kt @@ -77,9 +77,9 @@ fun LineSegment.intersects(other: LineSegment): Boolean { true } else if (orientationB == Orientation.Collinear && b.within(other)) { true - } else if (orientationC == Orientation.Collinear && c.within(other)) { + } else if (orientationC == Orientation.Collinear && c.within(this)) { true - } else if (orientationD == Orientation.Collinear && d.within(other)) { + } else if (orientationD == Orientation.Collinear && d.within(this)) { true } else { false 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 index 49ad39d06a9..dcd8462423a 100644 --- a/shared/src/test/java/org/odk/collect/shared/geometry/GeometryTest.kt +++ b/shared/src/test/java/org/odk/collect/shared/geometry/GeometryTest.kt @@ -166,4 +166,12 @@ class GeometryTest { val dTouching = LineSegment(Point(-1.0, 1.0), Point(0.0, 1.0)) assertThat(line.intersects(dTouching), equalTo(true)) } + + @Test + fun `LineSegment#intersects does not detect intersections for collinear endpoints`() { + val segment1 = LineSegment(Point(0.0, 0.0), Point(4.0, 0.0)) + val segment2 = LineSegment(Point(4.0, 4.0), Point(8.0, 0.0)) + + assertThat(segment1.intersects(segment2), equalTo(false)) + } } From 8822207a90e8e38de5d526feeab9bafbeafc2f53 Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Wed, 26 Nov 2025 11:24:44 +0000 Subject: [PATCH 31/43] Add additional (failing) test for 2 segment self intersection --- .../odk/collect/shared/geometry/GeometryTest.kt | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) 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 index dcd8462423a..8c7de507b9b 100644 --- a/shared/src/test/java/org/odk/collect/shared/geometry/GeometryTest.kt +++ b/shared/src/test/java/org/odk/collect/shared/geometry/GeometryTest.kt @@ -110,16 +110,22 @@ class GeometryTest { } @Test - fun `Trace#intersects returns true when a segment is collinear and within another`() { - val trace = Trace( + fun `Trace#intersects returns true when two segments and they intersect`() { + val endpointWithin = Trace( listOf( Point(0.0, 0.0), Point(0.0, 1.0), Point(0.0, 0.5) ) ) - - assertThat(trace.intersects(), equalTo(true)) + assertThat(endpointWithin.intersects(), equalTo(true)) + + val endpointBeyond = Trace(listOf( + Point(0.0, 0.0), + Point(0.0, 1.0), + Point(0.0, -1.0), + )) + assertThat(endpointBeyond.intersects(), equalTo(true)) } @Test From 8e0e0c7a659d5961def24071081b14446edd0460 Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Wed, 26 Nov 2025 11:37:25 +0000 Subject: [PATCH 32/43] Account for case where first endpoint intersects with second segment in 2 segment connected case --- .../org/odk/collect/shared/geometry/Geometry.kt | 13 +++++++++++-- .../org/odk/collect/shared/geometry/GeometryTest.kt | 7 +++++++ 2 files changed, 18 insertions(+), 2 deletions(-) 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 index 78b8ac2ac83..e0b4f76d69c 100644 --- a/shared/src/main/java/org/odk/collect/shared/geometry/Geometry.kt +++ b/shared/src/main/java/org/odk/collect/shared/geometry/Geometry.kt @@ -23,8 +23,17 @@ fun Trace.intersects(): Boolean { return if (points.size >= 3) { val segments = segments() return if (segments.size == 2) { - val orientation = orientation(segments[1].end, segments[0].start, segments[0].end) - orientation == Orientation.Collinear && segments[1].end.within(segments[0]) + val (a, b) = segments[0] + val (c, d) = segments[1] + val orientationA = orientation(a, c, d) + val orientationD = orientation(d, a, b) + return if (orientationA == Orientation.Collinear && a.within(segments[1])) { + true + } else if (orientationD == Orientation.Collinear && d.within(segments[0])) { + true + } else { + false + } } else { segments.filterIndexed { line1Index, line1 -> segments.filterIndexed { line2Index, line2 -> 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 index 8c7de507b9b..303a230f600 100644 --- a/shared/src/test/java/org/odk/collect/shared/geometry/GeometryTest.kt +++ b/shared/src/test/java/org/odk/collect/shared/geometry/GeometryTest.kt @@ -126,6 +126,13 @@ class GeometryTest { Point(0.0, -1.0), )) assertThat(endpointBeyond.intersects(), equalTo(true)) + + val endpointMatching = Trace(listOf( + Point(0.0, 0.0), + Point(0.0, 1.0), + Point(0.0, 0.0), + )) + assertThat(endpointMatching.intersects(), equalTo(true)) } @Test From 970b508c9ce055fe2678a9d43c97fa77279c53b0 Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Wed, 26 Nov 2025 11:49:26 +0000 Subject: [PATCH 33/43] Filter out zero length segments in traces --- .../odk/collect/shared/geometry/Geometry.kt | 8 ++++- .../collect/shared/geometry/GeometryTest.kt | 31 +++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) 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 index e0b4f76d69c..f3340f2213f 100644 --- a/shared/src/main/java/org/odk/collect/shared/geometry/Geometry.kt +++ b/shared/src/main/java/org/odk/collect/shared/geometry/Geometry.kt @@ -12,7 +12,13 @@ data class Trace(val points: List) { } fun Trace.segments(): List { - return points.zipWithNext().map { (start, end) -> LineSegment(start, end) } + return points.zipWithNext().flatMap { (start, end) -> + if (start != end) { + listOf(LineSegment(start, end)) + } else { + emptyList() + } + } } /** 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 index 303a230f600..808bb620cbf 100644 --- a/shared/src/test/java/org/odk/collect/shared/geometry/GeometryTest.kt +++ b/shared/src/test/java/org/odk/collect/shared/geometry/GeometryTest.kt @@ -163,6 +163,37 @@ class GeometryTest { assertThat(trace.intersects(), equalTo(false)) } + @Test + fun `Trace#segments returns false for trace with duplicate points`() { + val trace = Trace( + listOf( + Point(0.0, 0.0), + Point(1.0, 0.0), + Point(1.0, 0.0), + Point(2.0, 0.0) + ) + ) + + assertThat(trace.intersects(), equalTo(false)) + } + + @Test + fun `Trace#segments does not include zero-length segments`() { + val trace = Trace( + listOf( + Point(0.0, 0.0), + Point(1.0, 0.0), + Point(1.0, 0.0), + Point(2.0, 0.0) + ) + ) + + assertThat(trace.segments(), equalTo(listOf( + LineSegment(Point(0.0, 0.0), Point(1.0, 0.0)), + LineSegment(Point(1.0, 0.0), Point(2.0, 0.0)) + ))) + } + @Test fun `LineSegment#intersects detects any endpoint touching the other line`() { val line = LineSegment(Point(0.0, 0.0), Point(0.0, 2.0)) From 1d1a320b7137f31fe734af774732d40ab7430fb8 Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Wed, 26 Nov 2025 11:53:39 +0000 Subject: [PATCH 34/43] Remove unneeded return --- .../src/main/java/org/odk/collect/shared/geometry/Geometry.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 index f3340f2213f..f22457f9dba 100644 --- a/shared/src/main/java/org/odk/collect/shared/geometry/Geometry.kt +++ b/shared/src/main/java/org/odk/collect/shared/geometry/Geometry.kt @@ -28,12 +28,12 @@ fun Trace.intersects(): Boolean { val points = this.points return if (points.size >= 3) { val segments = segments() - return if (segments.size == 2) { + if (segments.size == 2) { val (a, b) = segments[0] val (c, d) = segments[1] val orientationA = orientation(a, c, d) val orientationD = orientation(d, a, b) - return if (orientationA == Orientation.Collinear && a.within(segments[1])) { + if (orientationA == Orientation.Collinear && a.within(segments[1])) { true } else if (orientationD == Orientation.Collinear && d.within(segments[0])) { true From bba426423d8ca146d4a53bc124ada36443c2030b Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Thu, 27 Nov 2025 13:49:55 +0000 Subject: [PATCH 35/43] Fix reversed line with 3 points case --- .../odk/collect/shared/geometry/Geometry.kt | 44 +++++++++---------- .../collect/shared/geometry/GeometryTest.kt | 19 ++++++++ 2 files changed, 41 insertions(+), 22 deletions(-) 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 index f22457f9dba..db28bb7b1d1 100644 --- a/shared/src/main/java/org/odk/collect/shared/geometry/Geometry.kt +++ b/shared/src/main/java/org/odk/collect/shared/geometry/Geometry.kt @@ -29,22 +29,14 @@ fun Trace.intersects(): Boolean { return if (points.size >= 3) { val segments = segments() if (segments.size == 2) { - val (a, b) = segments[0] - val (c, d) = segments[1] - val orientationA = orientation(a, c, d) - val orientationD = orientation(d, a, b) - if (orientationA == Orientation.Collinear && a.within(segments[1])) { - true - } else if (orientationD == Orientation.Collinear && d.within(segments[0])) { - true - } else { - false - } + segments[0].intersects(segments[1], allowConnection = true) } 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) } else if (line2Index >= line1Index + 2) { line1.intersects(line2) } else { @@ -76,28 +68,36 @@ fun Point.within(segment: LineSegment): Boolean { * Work out whether two line segments intersect by calculating if the endpoints of one segment * are on opposite sides (or touching of the other segment **and** vice versa. This is * determined by finding the orientation of endpoints relative to the other line. + * + * @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): Boolean { +fun LineSegment.intersects(other: LineSegment, allowConnection: Boolean = false): Boolean { val (a, b) = this val (c, d) = other val orientationA = orientation(a, c, d) - val orientationB = orientation(b, c, d) - val orientationC = orientation(a, b, c) val orientationD = orientation(a, b, d) - return if (orientationA.isOpposing(orientationB) && orientationC.isOpposing(orientationD)) { - true - } else if (orientationA == Orientation.Collinear && a.within(other)) { - true - } else if (orientationB == Orientation.Collinear && b.within(other)) { - true - } else if (orientationC == Orientation.Collinear && c.within(this)) { + return if (orientationA == Orientation.Collinear && a.within(other)) { true } else if (orientationD == Orientation.Collinear && d.within(this)) { true - } else { + } else if (b == c && allowConnection) { false + } else { + val orientationB = orientation(b, c, d) + val orientationC = orientation(a, b, c) + + if (orientationA.isOpposing(orientationB) && orientationC.isOpposing(orientationD)) { + true + } else if (orientationB == Orientation.Collinear && b.within(other)) { + true + } else if (orientationC == Orientation.Collinear && c.within(this)) { + true + } else { + false + } } } 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 index 808bb620cbf..a0dc5787f2e 100644 --- a/shared/src/test/java/org/odk/collect/shared/geometry/GeometryTest.kt +++ b/shared/src/test/java/org/odk/collect/shared/geometry/GeometryTest.kt @@ -194,6 +194,17 @@ class GeometryTest { ))) } + @Test + fun `Trace#intersects returns true for 3 segment closed trace reversing on itself`() { + val trace = Trace(listOf( + Point(0.0, 0.0), + Point(2.0, 0.0), + Point(1.0, 0.0), + Point(0.0, 0.0) + )) + assertThat(trace.intersects(), equalTo(true)) + } + @Test fun `LineSegment#intersects detects any endpoint touching the other line`() { val line = LineSegment(Point(0.0, 0.0), Point(0.0, 2.0)) @@ -218,4 +229,12 @@ class GeometryTest { assertThat(segment1.intersects(segment2), equalTo(false)) } + + @Test + fun `LineSegment#intersects with allowConnection true still finds intersections in a cross`() { + val segment1 = LineSegment(Point(-1.0, 0.0), Point(1.0, 0.0)) + val segment2 = LineSegment(Point(0.0, -1.0), Point(0.0, 1.0)) + + assertThat(segment1.intersects(segment2, allowConnection = true), equalTo(true)) + } } From 2fb19f82b8771f801af949707f7e6b798f6aeeaf Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Thu, 27 Nov 2025 14:33:13 +0000 Subject: [PATCH 36/43] Add basic metamorphic test for intersects --- .../collect/shared/geometry/GeometryTest.kt | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) 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 index a0dc5787f2e..dc9ddd5c0af 100644 --- 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,7 @@ package org.odk.collect.shared.geometry import org.hamcrest.MatcherAssert.assertThat import org.hamcrest.Matchers.equalTo import org.junit.Test +import kotlin.random.Random class GeometryTest { @@ -205,6 +206,39 @@ class GeometryTest { assertThat(trace.intersects(), equalTo(true)) } + @Test + fun `Trace#intersects satisfies metamorphic relationships`() { + 0.until(1000).map { + val trace = generateTrace() + val intersects = trace.intersects() + + // Check intersects is consistent when trace is reversed + val reversedTrace = Trace(trace.points.reversed()) + assertThat(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) + }) + assertThat(scaledTrace.intersects(), equalTo(intersects)) + + // Check adding an intersection makes intersects true + if (!intersects) { + val randomSegment = if (trace.segments().size > 1) { + trace.segments().drop(1).random() + } else { + trace.segments().first() + } + + val intersectionPoint = randomSegment.start + val intersectingTrace = + Trace(trace.points + listOf(trace.points.last(), intersectionPoint)) + assertThat(intersectingTrace.intersects(), equalTo(true)) + } + } + } + @Test fun `LineSegment#intersects detects any endpoint touching the other line`() { val line = LineSegment(Point(0.0, 0.0), Point(0.0, 2.0)) @@ -237,4 +271,25 @@ class GeometryTest { assertThat(segment1.intersects(segment2, allowConnection = true), equalTo(true)) } + + private fun generateTrace(maxLength: Int = 10, maxCoordinate: Double = 100.0): Trace { + val length = Random.nextInt(2, maxLength) + val trace = Trace(0.until(length).map { + Point( + Random.nextDouble(maxCoordinate * -1, maxCoordinate), + Random.nextDouble(maxCoordinate * -1, maxCoordinate) + ) + }) + + return if (trace.isClosed()) { + trace + } else { + val shouldClose = Random.nextBoolean() + if (shouldClose) { + trace.copy(points = trace.points + trace.points.first()) + } else { + trace + } + } + } } From 9f596bce785445ce667316796beab299a8f02447 Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Thu, 27 Nov 2025 15:43:50 +0000 Subject: [PATCH 37/43] Improve adding intersecting segment using interpolation --- .../odk/collect/shared/geometry/Geometry.kt | 6 +++++ .../collect/shared/geometry/GeometryTest.kt | 27 ++++++++++++------- 2 files changed, 23 insertions(+), 10 deletions(-) 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 index db28bb7b1d1..bd472c55932 100644 --- a/shared/src/main/java/org/odk/collect/shared/geometry/Geometry.kt +++ b/shared/src/main/java/org/odk/collect/shared/geometry/Geometry.kt @@ -101,6 +101,12 @@ fun LineSegment.intersects(other: LineSegment, allowConnection: Boolean = false) } } +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 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 index dc9ddd5c0af..4b0ec42c8f2 100644 --- a/shared/src/test/java/org/odk/collect/shared/geometry/GeometryTest.kt +++ b/shared/src/test/java/org/odk/collect/shared/geometry/GeometryTest.kt @@ -214,27 +214,34 @@ class GeometryTest { // Check intersects is consistent when trace is reversed val reversedTrace = Trace(trace.points.reversed()) - assertThat(reversedTrace.intersects(), equalTo(intersects)) + 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) }) - assertThat(scaledTrace.intersects(), equalTo(intersects)) + assertThat( + "Expected intersects=$intersects:\n$scaledTrace", + scaledTrace.intersects(), + equalTo(intersects) + ) // Check adding an intersection makes intersects true if (!intersects) { - val randomSegment = if (trace.segments().size > 1) { - trace.segments().drop(1).random() - } else { - trace.segments().first() - } - - val intersectionPoint = randomSegment.start + val intersectPosition = Random.nextDouble(0.0, 1.0) + val intersectionPoint = trace.segments().first().interpolate(intersectPosition) val intersectingTrace = Trace(trace.points + listOf(trace.points.last(), intersectionPoint)) - assertThat(intersectingTrace.intersects(), equalTo(true)) + assertThat( + "Expected intersect=true:\n$intersectingTrace", + intersectingTrace.intersects(), + equalTo(true) + ) } } } From 29936a849e9ecf48d055498c5a4c33fb26b5d96d Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Thu, 27 Nov 2025 15:50:13 +0000 Subject: [PATCH 38/43] Add epsilon to colinearity check to prevent precision errors --- .../org/odk/collect/shared/geometry/Geometry.kt | 13 ++++++++----- .../org/odk/collect/shared/geometry/GeometryTest.kt | 2 +- 2 files changed, 9 insertions(+), 6 deletions(-) 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 index bd472c55932..bf14e02a4fb 100644 --- a/shared/src/main/java/org/odk/collect/shared/geometry/Geometry.kt +++ b/shared/src/main/java/org/odk/collect/shared/geometry/Geometry.kt @@ -1,5 +1,6 @@ package org.odk.collect.shared.geometry +import kotlin.math.abs import kotlin.math.max import kotlin.math.min @@ -113,15 +114,17 @@ fun LineSegment.interpolate(position: Double): Point { * [here](https://en.wikipedia.org/wiki/Cross_product#Computational_geometry)). This can * either be clockwise, anticlockwise or collinear (the three points form a straight line). * + * @param epsilon the epsilon used to check for collinearity + * */ -private fun orientation(a: Point, b: Point, c: Point): Orientation { +private fun orientation(a: Point, b: Point, c: Point, epsilon: Double = 0.00000000001): Orientation { val crossProduct = crossProduct(Pair(b.x - a.x, b.y - a.y), Pair(c.x - a.x, c.y - a.y)) - return if (crossProduct > 0) { + return if (abs(crossProduct) < epsilon) { + Orientation.Collinear + } else if (crossProduct > 0) { Orientation.AntiClockwise - } else if (crossProduct < 0) { - Orientation.Clockwise } else { - Orientation.Collinear + Orientation.Clockwise } } 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 index 4b0ec42c8f2..a4822c2cf13 100644 --- a/shared/src/test/java/org/odk/collect/shared/geometry/GeometryTest.kt +++ b/shared/src/test/java/org/odk/collect/shared/geometry/GeometryTest.kt @@ -238,7 +238,7 @@ class GeometryTest { val intersectingTrace = Trace(trace.points + listOf(trace.points.last(), intersectionPoint)) assertThat( - "Expected intersect=true:\n$intersectingTrace", + "Expected intersects=true:\n$intersectingTrace", intersectingTrace.intersects(), equalTo(true) ) From 08c5997b9900a2b81c31f904ac462d309ddf143e Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Thu, 27 Nov 2025 16:05:43 +0000 Subject: [PATCH 39/43] Fix accidental map --- .../test/java/org/odk/collect/shared/geometry/GeometryTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index a4822c2cf13..91c56f3b0ce 100644 --- a/shared/src/test/java/org/odk/collect/shared/geometry/GeometryTest.kt +++ b/shared/src/test/java/org/odk/collect/shared/geometry/GeometryTest.kt @@ -208,7 +208,7 @@ class GeometryTest { @Test fun `Trace#intersects satisfies metamorphic relationships`() { - 0.until(1000).map { + repeat(1000) { val trace = generateTrace() val intersects = trace.intersects() From c6aa7c0209d75b969d554f5bcd6a9103a041b744 Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Thu, 27 Nov 2025 16:37:45 +0000 Subject: [PATCH 40/43] Add docs for interpolate --- .../main/java/org/odk/collect/shared/geometry/Geometry.kt | 5 +++++ 1 file changed, 5 insertions(+) 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 index bf14e02a4fb..63461079348 100644 --- a/shared/src/main/java/org/odk/collect/shared/geometry/Geometry.kt +++ b/shared/src/main/java/org/odk/collect/shared/geometry/Geometry.kt @@ -102,6 +102,11 @@ 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) From 6a0e2e09951c259486ecdd86996ab2c268d4c049 Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Fri, 28 Nov 2025 08:54:10 +0000 Subject: [PATCH 41/43] Increase the number of possible random intersection points in test --- .../java/org/odk/collect/shared/geometry/GeometryTest.kt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) 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 index 91c56f3b0ce..5d3e315681e 100644 --- a/shared/src/test/java/org/odk/collect/shared/geometry/GeometryTest.kt +++ b/shared/src/test/java/org/odk/collect/shared/geometry/GeometryTest.kt @@ -233,8 +233,9 @@ class GeometryTest { // Check adding an intersection makes intersects true if (!intersects) { - val intersectPosition = Random.nextDouble(0.0, 1.0) - val intersectionPoint = trace.segments().first().interpolate(intersectPosition) + 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)) assertThat( From ac135334066cc973bb93bc6d310977c34bbc7b12 Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Fri, 28 Nov 2025 10:55:04 +0000 Subject: [PATCH 42/43] Add tests for LineSegment#interpolate --- .../odk/collect/shared/geometry/Geometry.kt | 4 ++-- .../collect/shared/geometry/GeometryTest.kt | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) 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 index 63461079348..f9732aca023 100644 --- a/shared/src/main/java/org/odk/collect/shared/geometry/Geometry.kt +++ b/shared/src/main/java/org/odk/collect/shared/geometry/Geometry.kt @@ -122,7 +122,7 @@ fun LineSegment.interpolate(position: Double): Point { * @param epsilon the epsilon used to check for collinearity * */ -private fun orientation(a: Point, b: Point, c: Point, epsilon: Double = 0.00000000001): Orientation { +fun orientation(a: Point, b: Point, c: Point, epsilon: Double = 0.00000000001): 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) { Orientation.Collinear @@ -140,7 +140,7 @@ private fun crossProduct(x: Pair, y: Pair): Doub return (x.first * y.second) - (y.first * x.second) } -private enum class Orientation { +enum class Orientation { Collinear, Clockwise, 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 index 5d3e315681e..98cb9aafc5e 100644 --- a/shared/src/test/java/org/odk/collect/shared/geometry/GeometryTest.kt +++ b/shared/src/test/java/org/odk/collect/shared/geometry/GeometryTest.kt @@ -280,6 +280,25 @@ 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 generateTrace(maxLength: Int = 10, maxCoordinate: Double = 100.0): Trace { val length = Random.nextInt(2, maxLength) val trace = Trace(0.until(length).map { From d88de713c6bda5396a250daaa70d4d5bec77e0a7 Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Fri, 28 Nov 2025 11:13:10 +0000 Subject: [PATCH 43/43] Add quickCheck helper --- .../java/org/odk/collect/shared/QuickCheck.kt | 16 +++++++ .../collect/shared/geometry/GeometryTest.kt | 43 ++++++++++--------- 2 files changed, 39 insertions(+), 20 deletions(-) create mode 100644 shared/src/test/java/org/odk/collect/shared/QuickCheck.kt diff --git a/shared/src/test/java/org/odk/collect/shared/QuickCheck.kt b/shared/src/test/java/org/odk/collect/shared/QuickCheck.kt new file mode 100644 index 00000000000..c6d9b6615c6 --- /dev/null +++ b/shared/src/test/java/org/odk/collect/shared/QuickCheck.kt @@ -0,0 +1,16 @@ +package org.odk.collect.shared + +/** + * Simple helper to allow writing neater [QuickCheck](https://en.wikipedia.org/wiki/QuickCheck) + * style tests. + */ +fun ((Input) -> Output).quickCheck( + iterations: Int, + generator: Sequence, + checks: (Input, Output) -> Unit +) { + generator.take(iterations).forEach { input -> + val output = this(input) + checks(input, output) + } +} 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 index 98cb9aafc5e..1e7bd5b7f76 100644 --- 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,7 @@ 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.quickCheck import kotlin.random.Random class GeometryTest { @@ -208,10 +209,10 @@ class GeometryTest { @Test fun `Trace#intersects satisfies metamorphic relationships`() { - repeat(1000) { - val trace = generateTrace() - val intersects = trace.intersects() - + { trace: Trace -> trace.intersects() }.quickCheck( + iterations = 1000, + generator = getTraceGenerator() + ) { trace, intersects -> // Check intersects is consistent when trace is reversed val reversedTrace = Trace(trace.points.reversed()) assertThat( @@ -299,23 +300,25 @@ class GeometryTest { assertThat(interpolatedPoint.within(segment), equalTo(true)) } - private fun generateTrace(maxLength: Int = 10, maxCoordinate: Double = 100.0): Trace { - val length = Random.nextInt(2, maxLength) - val trace = Trace(0.until(length).map { - Point( - Random.nextDouble(maxCoordinate * -1, maxCoordinate), - Random.nextDouble(maxCoordinate * -1, maxCoordinate) - ) - }) - - return if (trace.isClosed()) { - trace - } else { - val shouldClose = Random.nextBoolean() - if (shouldClose) { - trace.copy(points = trace.points + trace.points.first()) - } else { + private fun getTraceGenerator(maxLength: Int = 10, maxCoordinate: Double = 100.0): Sequence { + 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 + } } } }