From e0baf2a22d49c2f9ebcd7d8a54304776c196399e Mon Sep 17 00:00:00 2001 From: Shobhit Agarwal Date: Tue, 19 May 2026 20:53:35 +0530 Subject: [PATCH 1/6] Separate committed and tentative vertices in `PolygonDrawingSession` --- .../tasks/polygon/DrawAreaTaskViewModel.kt | 10 +-- .../tasks/polygon/PolygonDrawingSession.kt | 6 ++ .../polygon/PolygonDrawingSessionImpl.kt | 67 +++++++++------ .../tasks/polygon/DrawAreaTaskScreenTest.kt | 1 - .../polygon/DrawAreaTaskViewModelTest.kt | 10 +-- .../polygon/PolygonDrawingSessionTest.kt | 84 +++++++++++++++---- 6 files changed, 125 insertions(+), 53 deletions(-) diff --git a/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/polygon/DrawAreaTaskViewModel.kt b/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/polygon/DrawAreaTaskViewModel.kt index 63564c1a59..12643a725b 100644 --- a/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/polygon/DrawAreaTaskViewModel.kt +++ b/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/polygon/DrawAreaTaskViewModel.kt @@ -150,8 +150,6 @@ internal constructor( .stateIn(viewModelScope, WhileSubscribed(5_000), emptyList()) } - - override fun initialize( job: Job, task: Task, @@ -326,7 +324,7 @@ internal constructor( * This coroutine runs on [viewModelScope] to ensure lifecycle safety. */ private fun refreshMap() = viewModelScope.launch { - if (session.vertices.isEmpty()) { + if (session.displayVertices.isEmpty()) { _draftArea.emit(null) draftTag = null } else { @@ -345,7 +343,7 @@ internal constructor( Feature( id = id ?: uuidGenerator.generateUuid(), type = Feature.Type.USER_POLYGON, - geometry = LineString(session.vertices), + geometry = LineString(session.displayVertices), style = featureStyle, clusterable = false, selected = true, @@ -354,8 +352,8 @@ internal constructor( /** Returns the distance in meters between the last two vertices for displaying in the tooltip. */ private fun getDistanceTooltipText(): String? { - if (sessionState.value.isMarkedComplete || session.vertices.size <= 1) return null - val distance = session.vertices.penult().distanceTo(session.vertices.last()) + if (sessionState.value.isMarkedComplete || session.displayVertices.size <= 1) return null + val distance = session.displayVertices.penult().distanceTo(session.displayVertices.last()) if (distance < TOOLTIP_MIN_DISTANCE_METERS) return null return localeAwareMeasureFormatter.formatDistance(distance, measurementUnits) } diff --git a/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/polygon/PolygonDrawingSession.kt b/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/polygon/PolygonDrawingSession.kt index ea7be15052..4e6f1bb876 100644 --- a/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/polygon/PolygonDrawingSession.kt +++ b/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/polygon/PolygonDrawingSession.kt @@ -28,6 +28,12 @@ interface PolygonDrawingSession { /** The list of committed vertices in the current drawing session. */ val vertices: List + /** The tentative vertex that tracks the uncommitted active position (e.g. camera target). */ + val tentativeVertex: Coordinates? + + /** The complete list of vertices (committed + tentative) to display on the map. */ + val displayVertices: List + /** Whether the current polygon geometry self-intersects. */ val hasSelfIntersection: Boolean diff --git a/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/polygon/PolygonDrawingSessionImpl.kt b/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/polygon/PolygonDrawingSessionImpl.kt index 375b7bdafb..003bc3f406 100644 --- a/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/polygon/PolygonDrawingSessionImpl.kt +++ b/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/polygon/PolygonDrawingSessionImpl.kt @@ -20,6 +20,7 @@ import kotlinx.collections.immutable.toImmutableList import org.groundplatform.android.ui.datacollection.tasks.polygon.PolygonDrawingSession.Companion.DISTANCE_THRESHOLD_DP import org.groundplatform.domain.model.geometry.Coordinates import org.groundplatform.domain.model.geometry.LineString +import org.groundplatform.domain.util.isClosed import org.groundplatform.domain.util.isSelfIntersecting class PolygonDrawingSessionImpl : PolygonDrawingSession { @@ -34,35 +35,39 @@ class PolygonDrawingSessionImpl : PolygonDrawingSession { override val vertices: List get() = _vertices - @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) - internal val redoVertexStack = mutableListOf() + private var _tentativeVertex: Coordinates? = null - override val hasSelfIntersection: Boolean - get() = isSelfIntersecting(_vertices) + override val tentativeVertex: Coordinates? + get() = _tentativeVertex - private fun addVertex(vertex: Coordinates, shouldOverwriteLastVertex: Boolean) { - val updatedVertices = _vertices.toMutableList() - - if (shouldOverwriteLastVertex && updatedVertices.isNotEmpty()) { - updatedVertices.removeAt(updatedVertices.lastIndex) - } + override val displayVertices: List + get() = _tentativeVertex?.let { _vertices + it } ?: _vertices - updatedVertices.add(vertex) + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + internal val redoVertexStack = mutableListOf() - _vertices = updatedVertices.toImmutableList() - } + override val hasSelfIntersection: Boolean + get() = isSelfIntersecting(displayVertices) override fun setVertices(newVertices: List) { _vertices = newVertices.toImmutableList() + _tentativeVertex = null + redoVertexStack.clear() } override fun updateTentativeVertex( target: Coordinates, calculateDistance: (Coordinates, Coordinates) -> Double, ) { + if (isClosed(_vertices)) { + _tentativeVertex = null + _isTooClose = true + return + } + val firstVertex = _vertices.firstOrNull() var updatedTarget = target - if (firstVertex != null && _vertices.size > 2) { + if (firstVertex != null && _vertices.size >= 3) { val distance = calculateDistance(firstVertex, target) if (distance <= DISTANCE_THRESHOLD_DP) { @@ -70,20 +75,20 @@ class PolygonDrawingSessionImpl : PolygonDrawingSession { } } - val prev = _vertices.dropLast(1).lastOrNull() + val lastCommitted = _vertices.lastOrNull() _isTooClose = - _vertices.size > 1 && - prev?.let { calculateDistance(it, target) <= DISTANCE_THRESHOLD_DP } == true + lastCommitted?.let { calculateDistance(it, target) <= DISTANCE_THRESHOLD_DP } == true - addVertex(updatedTarget, true) + _tentativeVertex = updatedTarget } override fun commitTentativeVertex(currentCameraTarget: Coordinates?): List? { redoVertexStack.clear() - val vertex = _vertices.lastOrNull() ?: currentCameraTarget + val vertex = _tentativeVertex ?: currentCameraTarget return vertex?.let { + _vertices = (_vertices + it).toImmutableList() + _tentativeVertex = null _isTooClose = _vertices.size > 1 - addVertex(it, false) _vertices } } @@ -96,21 +101,32 @@ class PolygonDrawingSessionImpl : PolygonDrawingSession { return intersected } - override fun isValidPolygon(): Boolean = - LineString(_vertices).isClosed() && !isSelfIntersecting(_vertices) + override fun isValidPolygon(): Boolean { + return LineString(displayVertices).isClosed() && + !isSelfIntersecting(displayVertices) && + displayVertices.distinct().size >= 3 + } override fun complete() { check(isValidPolygon()) { "Polygon is not valid" } check(!_isMarkedComplete) { "Already marked complete" } + if (_tentativeVertex != null) { + commitTentativeVertex(null) + } _isMarkedComplete = true } override fun removeLastVertex(): Boolean { - if (_vertices.isEmpty()) return false + if (_tentativeVertex == null && _vertices.isEmpty()) return false _isMarkedComplete = false - redoVertexStack.add(_vertices.last()) - _vertices = _vertices.dropLast(1).toImmutableList() + _tentativeVertex = null + + if (_vertices.isNotEmpty()) { + redoVertexStack.add(_vertices.last()) + _vertices = _vertices.dropLast(1).toImmutableList() + } + return true } @@ -120,6 +136,7 @@ class PolygonDrawingSessionImpl : PolygonDrawingSession { _isMarkedComplete = false val redoVertex = redoVertexStack.removeAt(redoVertexStack.lastIndex) _vertices = (_vertices + redoVertex).toImmutableList() + _tentativeVertex = null return redoVertex } diff --git a/app/src/test/java/org/groundplatform/android/ui/datacollection/tasks/polygon/DrawAreaTaskScreenTest.kt b/app/src/test/java/org/groundplatform/android/ui/datacollection/tasks/polygon/DrawAreaTaskScreenTest.kt index 625590fc7f..68503b1f82 100644 --- a/app/src/test/java/org/groundplatform/android/ui/datacollection/tasks/polygon/DrawAreaTaskScreenTest.kt +++ b/app/src/test/java/org/groundplatform/android/ui/datacollection/tasks/polygon/DrawAreaTaskScreenTest.kt @@ -119,7 +119,6 @@ class DrawAreaTaskScreenTest : BaseHiltTest() { Coordinates(0.0, 0.0), Coordinates(10.0, 10.0), Coordinates(20.0, 20.0), - Coordinates(20.0, 20.0), ) ) ) diff --git a/app/src/test/java/org/groundplatform/android/ui/datacollection/tasks/polygon/DrawAreaTaskViewModelTest.kt b/app/src/test/java/org/groundplatform/android/ui/datacollection/tasks/polygon/DrawAreaTaskViewModelTest.kt index 6843f3385e..cff7dec4d9 100644 --- a/app/src/test/java/org/groundplatform/android/ui/datacollection/tasks/polygon/DrawAreaTaskViewModelTest.kt +++ b/app/src/test/java/org/groundplatform/android/ui/datacollection/tasks/polygon/DrawAreaTaskViewModelTest.kt @@ -115,7 +115,7 @@ class DrawAreaTaskViewModelTest : BaseHiltTest() { viewModel.removeLastVertex() - assertGeometry(3, isLineString = true) + assertGeometry(2, isLineString = true) } @Test @@ -199,11 +199,11 @@ class DrawAreaTaskViewModelTest : BaseHiltTest() { viewModel.removeLastVertex() - assertThat(featureTestObserver.value()?.tooltipText).isEqualTo("1,565,109 m") + assertThat(featureTestObserver.value()?.tooltipText).isNull() viewModel.removeLastVertex() - assertThat(featureTestObserver.value()?.tooltipText).isEqualTo(null) + assertThat(featureTestObserver.value()?.tooltipText).isNull() } @Test @@ -568,10 +568,10 @@ class DrawAreaTaskViewModelTest : BaseHiltTest() { updateLastVertexAndAdd(COORDINATE_2) viewModel.removeLastVertex() - assertGeometry(2, isLineString = true) + assertGeometry(1, isLineString = true) viewModel.redoLastVertex() - assertGeometry(3, isLineString = true) + assertGeometry(2, isLineString = true) } @Test diff --git a/app/src/test/java/org/groundplatform/android/ui/datacollection/tasks/polygon/PolygonDrawingSessionTest.kt b/app/src/test/java/org/groundplatform/android/ui/datacollection/tasks/polygon/PolygonDrawingSessionTest.kt index 354fdd50d1..fd7755b672 100644 --- a/app/src/test/java/org/groundplatform/android/ui/datacollection/tasks/polygon/PolygonDrawingSessionTest.kt +++ b/app/src/test/java/org/groundplatform/android/ui/datacollection/tasks/polygon/PolygonDrawingSessionTest.kt @@ -32,6 +32,8 @@ class PolygonDrawingSessionTest { @Test fun `Initial state is empty`() { assertThat(session.vertices).isEmpty() + assertThat(session.tentativeVertex).isNull() + assertThat(session.displayVertices).isEmpty() assertThat(session.redoVertexStack).isEmpty() } @@ -40,19 +42,24 @@ class PolygonDrawingSessionTest { val newVertices = listOf(COORDINATE_1, COORDINATE_2) session.setVertices(newVertices) assertThat(session.vertices).containsExactlyElementsIn(newVertices).inOrder() + assertThat(session.displayVertices).containsExactlyElementsIn(newVertices).inOrder() } @Test - fun `updateTentativeVertex adds vertex when empty`() { + fun `updateTentativeVertex sets tentative vertex when empty`() { session.updateTentativeVertex(COORDINATE_1) { _, _ -> DISTANCE_ABOVE_THRESHOLD } - assertThat(session.vertices).containsExactly(COORDINATE_1) + assertThat(session.vertices).isEmpty() + assertThat(session.tentativeVertex).isEqualTo(COORDINATE_1) + assertThat(session.displayVertices).containsExactly(COORDINATE_1) } @Test - fun `updateTentativeVertex overwrites last vertex`() { + fun `updateTentativeVertex sets tentative vertex without changing committed list`() { session.setVertices(listOf(COORDINATE_1)) session.updateTentativeVertex(COORDINATE_2) { _, _ -> DISTANCE_ABOVE_THRESHOLD } - assertThat(session.vertices).containsExactly(COORDINATE_2) + assertThat(session.vertices).containsExactly(COORDINATE_1) + assertThat(session.tentativeVertex).isEqualTo(COORDINATE_2) + assertThat(session.displayVertices).containsExactly(COORDINATE_1, COORDINATE_2).inOrder() } @Test @@ -61,7 +68,11 @@ class PolygonDrawingSessionTest { // Target is close to COORDINATE_1 session.updateTentativeVertex(COORDINATE_4) { _, _ -> DISTANCE_BELOW_THRESHOLD } - assertThat(session.vertices.last()).isEqualTo(COORDINATE_1) + assertThat(session.vertices).containsExactly(COORDINATE_1, COORDINATE_2, COORDINATE_3).inOrder() + assertThat(session.tentativeVertex).isEqualTo(COORDINATE_1) + assertThat(session.displayVertices) + .containsExactly(COORDINATE_1, COORDINATE_2, COORDINATE_3, COORDINATE_1) + .inOrder() } @Test @@ -70,7 +81,11 @@ class PolygonDrawingSessionTest { // Target is far from COORDINATE_1 session.updateTentativeVertex(COORDINATE_4) { _, _ -> DISTANCE_ABOVE_THRESHOLD } - assertThat(session.vertices.last()).isEqualTo(COORDINATE_4) + assertThat(session.vertices).containsExactly(COORDINATE_1, COORDINATE_2, COORDINATE_3).inOrder() + assertThat(session.tentativeVertex).isEqualTo(COORDINATE_4) + assertThat(session.displayVertices) + .containsExactly(COORDINATE_1, COORDINATE_2, COORDINATE_3, COORDINATE_4) + .inOrder() } @Test @@ -102,9 +117,11 @@ class PolygonDrawingSessionTest { } @Test - fun `commitTentativeVertex adds vertex to committed list`() { - session.commitTentativeVertex(COORDINATE_1) + fun `commitTentativeVertex adds tentative vertex to committed list`() { + session.updateTentativeVertex(COORDINATE_1) { _, _ -> DISTANCE_ABOVE_THRESHOLD } + session.commitTentativeVertex(null) assertThat(session.vertices).containsExactly(COORDINATE_1) + assertThat(session.tentativeVertex).isNull() } @Test @@ -124,12 +141,24 @@ class PolygonDrawingSessionTest { assertThat(session.removeLastVertex()).isFalse() } + @Test + fun `removeLastVertex clears tentative vertex when committed list is empty`() { + session.updateTentativeVertex(COORDINATE_1) { _, _ -> DISTANCE_ABOVE_THRESHOLD } + assertThat(session.tentativeVertex).isEqualTo(COORDINATE_1) + + assertThat(session.removeLastVertex()).isTrue() + assertThat(session.vertices).isEmpty() + assertThat(session.tentativeVertex).isNull() + } + @Test fun `removeLastVertex removes last vertex and adds to redo stack`() { session.setVertices(listOf(COORDINATE_1, COORDINATE_2)) assertThat(session.removeLastVertex()).isTrue() assertThat(session.vertices).containsExactly(COORDINATE_1) + assertThat(session.tentativeVertex).isNull() + assertThat(session.displayVertices).containsExactly(COORDINATE_1) assertThat(session.redoVertexStack).containsExactly(COORDINATE_2) } @@ -139,6 +168,7 @@ class PolygonDrawingSessionTest { assertThat(session.removeLastVertex()).isTrue() assertThat(session.vertices).isEmpty() + assertThat(session.tentativeVertex).isNull() assertThat(session.redoVertexStack).containsExactly(COORDINATE_1) } @@ -154,6 +184,8 @@ class PolygonDrawingSessionTest { assertThat(session.redoLastVertex()).isEqualTo(COORDINATE_2) assertThat(session.vertices).containsExactly(COORDINATE_1, COORDINATE_2).inOrder() + assertThat(session.tentativeVertex).isNull() + assertThat(session.displayVertices).containsExactly(COORDINATE_1, COORDINATE_2).inOrder() assertThat(session.redoVertexStack).isEmpty() } @@ -172,31 +204,36 @@ class PolygonDrawingSessionTest { } @Test - fun `validatePolygonCompletion returns false when size less than 3`() { + fun `validatePolygonCompletion returns false when distinct size less than 3`() { session.setVertices(listOf(C1, C2)) + session.updateTentativeVertex(C1) { _, _ -> DISTANCE_BELOW_THRESHOLD } assertThat(session.isValidPolygon()).isFalse() } @Test fun `validatePolygonCompletion returns false when self-intersecting`() { - session.setVertices(listOf(C1, C2, C3, C4)) + // Setup a square and then add a vertex that intersects it + session.setVertices(listOf(C1, C2, C3)) + session.updateTentativeVertex(C4) { _, _ -> DISTANCE_ABOVE_THRESHOLD } assertThat(session.isValidPolygon()).isFalse() assertThat(session.hasSelfIntersection).isTrue() } @Test fun `validatePolygonCompletion returns true when valid`() { - session.setVertices(listOf(C1, C3, C2, C4, C1)) + session.setVertices(listOf(C1, C3, C2, C4)) + session.updateTentativeVertex(C1) { _, _ -> DISTANCE_BELOW_THRESHOLD } assertThat(session.isValidPolygon()).isTrue() assertThat(session.hasSelfIntersection).isFalse() } @Test fun `hasSelfIntersection updates dynamically when vertices change`() { - session.setVertices(listOf(C1, C2, C3, C4)) + session.setVertices(listOf(C1, C2, C3)) + session.updateTentativeVertex(C4) { _, _ -> DISTANCE_ABOVE_THRESHOLD } assertThat(session.hasSelfIntersection).isTrue() - session.removeLastVertex() + session.updateTentativeVertex(Coordinates(50.0, 50.0)) { _, _ -> DISTANCE_ABOVE_THRESHOLD } assertThat(session.hasSelfIntersection).isFalse() } @@ -208,14 +245,18 @@ class PolygonDrawingSessionTest { @Test fun `complete() marks session as complete`() { - session.setVertices(listOf(C1, C3, C2, C4, C1)) + session.setVertices(listOf(C1, C3, C2, C4)) + session.updateTentativeVertex(C1) { _, _ -> DISTANCE_BELOW_THRESHOLD } session.complete() assertThat(session.state.isMarkedComplete).isTrue() + assertThat(session.vertices).containsExactly(C1, C3, C2, C4, C1).inOrder() + assertThat(session.tentativeVertex).isNull() } @Test fun `removeLastVertex resets completion state`() { - session.setVertices(listOf(C1, C3, C2, C4, C1)) + session.setVertices(listOf(C1, C3, C2, C4)) + session.updateTentativeVertex(C1) { _, _ -> DISTANCE_BELOW_THRESHOLD } session.complete() assertThat(session.state.isMarkedComplete).isTrue() @@ -225,7 +266,8 @@ class PolygonDrawingSessionTest { @Test fun `redoLastVertex resets completion state`() { - session.setVertices(listOf(C1, C3, C2, C4, C1)) + session.setVertices(listOf(C1, C3, C2, C4)) + session.updateTentativeVertex(C1) { _, _ -> DISTANCE_BELOW_THRESHOLD } session.complete() session.removeLastVertex() @@ -233,6 +275,16 @@ class PolygonDrawingSessionTest { assertThat(session.state.isMarkedComplete).isFalse() } + @Test + fun `updateTentativeVertex does nothing when polygon is closed`() { + session.setVertices(listOf(C1, C3, C2, C4, C1)) + session.updateTentativeVertex(Coordinates(50.0, 50.0)) { _, _ -> DISTANCE_ABOVE_THRESHOLD } + + assertThat(session.vertices).containsExactly(C1, C3, C2, C4, C1).inOrder() + assertThat(session.tentativeVertex).isNull() + assertThat(session.state.isTooClose).isTrue() + } + companion object { private const val DISTANCE_BELOW_THRESHOLD = 20.0 private const val DISTANCE_ABOVE_THRESHOLD = 30.0 From 8e2ffa745fb41e6663bfc044924f1bc8aa260def Mon Sep 17 00:00:00 2001 From: Shobhit Agarwal Date: Wed, 20 May 2026 23:58:22 +0530 Subject: [PATCH 2/6] Apply detekt fixes --- .../tasks/polygon/PolygonDrawingSessionImpl.kt | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/polygon/PolygonDrawingSessionImpl.kt b/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/polygon/PolygonDrawingSessionImpl.kt index 003bc3f406..98f2ace607 100644 --- a/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/polygon/PolygonDrawingSessionImpl.kt +++ b/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/polygon/PolygonDrawingSessionImpl.kt @@ -101,11 +101,9 @@ class PolygonDrawingSessionImpl : PolygonDrawingSession { return intersected } - override fun isValidPolygon(): Boolean { - return LineString(displayVertices).isClosed() && - !isSelfIntersecting(displayVertices) && - displayVertices.distinct().size >= 3 - } + override fun isValidPolygon(): Boolean = LineString(displayVertices).isClosed() && + !isSelfIntersecting(displayVertices) && + displayVertices.distinct().size >= 3 override fun complete() { check(isValidPolygon()) { "Polygon is not valid" } From 95bac360b251d95eb60e339e12562febd4936867 Mon Sep 17 00:00:00 2001 From: Shobhit Agarwal Date: Thu, 21 May 2026 00:01:29 +0530 Subject: [PATCH 3/6] Pass vertices as parameter in DrawAreaTaskViewModel internal methods --- .../tasks/polygon/DrawAreaTaskViewModel.kt | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/polygon/DrawAreaTaskViewModel.kt b/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/polygon/DrawAreaTaskViewModel.kt index 12643a725b..5596e5b7cb 100644 --- a/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/polygon/DrawAreaTaskViewModel.kt +++ b/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/polygon/DrawAreaTaskViewModel.kt @@ -324,36 +324,37 @@ internal constructor( * This coroutine runs on [viewModelScope] to ensure lifecycle safety. */ private fun refreshMap() = viewModelScope.launch { - if (session.displayVertices.isEmpty()) { + val vertices = session.displayVertices + if (vertices.isEmpty()) { _draftArea.emit(null) draftTag = null } else { if (draftTag == null) { - val feature = buildPolygonFeature() + val feature = buildPolygonFeature(vertices) draftTag = feature.tag _draftArea.emit(feature) } else { - val feature = buildPolygonFeature(id = draftTag!!.id) + val feature = buildPolygonFeature(vertices, id = draftTag!!.id) _draftUpdates.tryEmit(feature) } } } - private suspend fun buildPolygonFeature(id: String? = null) = + private suspend fun buildPolygonFeature(vertices: List, id: String? = null) = Feature( id = id ?: uuidGenerator.generateUuid(), type = Feature.Type.USER_POLYGON, - geometry = LineString(session.displayVertices), + geometry = LineString(vertices), style = featureStyle, clusterable = false, selected = true, - tooltipText = getDistanceTooltipText(), + tooltipText = getDistanceTooltipText(vertices), ) /** Returns the distance in meters between the last two vertices for displaying in the tooltip. */ - private fun getDistanceTooltipText(): String? { - if (sessionState.value.isMarkedComplete || session.displayVertices.size <= 1) return null - val distance = session.displayVertices.penult().distanceTo(session.displayVertices.last()) + private fun getDistanceTooltipText(vertices: List): String? { + if (sessionState.value.isMarkedComplete || vertices.size <= 1) return null + val distance = vertices.penult().distanceTo(vertices.last()) if (distance < TOOLTIP_MIN_DISTANCE_METERS) return null return localeAwareMeasureFormatter.formatDistance(distance, measurementUnits) } From 8a409b807b1c48f01156de042f25f24b6d03334b Mon Sep 17 00:00:00 2001 From: Shobhit Agarwal Date: Thu, 21 May 2026 19:59:09 +0530 Subject: [PATCH 4/6] Remove tentativeVertex from PolygonDrawingSession interface make it internal to the implementation --- .../tasks/polygon/PolygonDrawingSession.kt | 3 --- .../polygon/PolygonDrawingSessionImpl.kt | 10 +++---- .../polygon/PolygonDrawingSessionTest.kt | 27 +++++++------------ 3 files changed, 14 insertions(+), 26 deletions(-) diff --git a/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/polygon/PolygonDrawingSession.kt b/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/polygon/PolygonDrawingSession.kt index 4e6f1bb876..690e5ba928 100644 --- a/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/polygon/PolygonDrawingSession.kt +++ b/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/polygon/PolygonDrawingSession.kt @@ -28,9 +28,6 @@ interface PolygonDrawingSession { /** The list of committed vertices in the current drawing session. */ val vertices: List - /** The tentative vertex that tracks the uncommitted active position (e.g. camera target). */ - val tentativeVertex: Coordinates? - /** The complete list of vertices (committed + tentative) to display on the map. */ val displayVertices: List diff --git a/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/polygon/PolygonDrawingSessionImpl.kt b/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/polygon/PolygonDrawingSessionImpl.kt index 98f2ace607..23d40e94c8 100644 --- a/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/polygon/PolygonDrawingSessionImpl.kt +++ b/app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/polygon/PolygonDrawingSessionImpl.kt @@ -37,9 +37,6 @@ class PolygonDrawingSessionImpl : PolygonDrawingSession { private var _tentativeVertex: Coordinates? = null - override val tentativeVertex: Coordinates? - get() = _tentativeVertex - override val displayVertices: List get() = _tentativeVertex?.let { _vertices + it } ?: _vertices @@ -101,9 +98,10 @@ class PolygonDrawingSessionImpl : PolygonDrawingSession { return intersected } - override fun isValidPolygon(): Boolean = LineString(displayVertices).isClosed() && - !isSelfIntersecting(displayVertices) && - displayVertices.distinct().size >= 3 + override fun isValidPolygon(): Boolean = + LineString(displayVertices).isClosed() && + !isSelfIntersecting(displayVertices) && + displayVertices.distinct().size >= 3 override fun complete() { check(isValidPolygon()) { "Polygon is not valid" } diff --git a/app/src/test/java/org/groundplatform/android/ui/datacollection/tasks/polygon/PolygonDrawingSessionTest.kt b/app/src/test/java/org/groundplatform/android/ui/datacollection/tasks/polygon/PolygonDrawingSessionTest.kt index fd7755b672..53d2c016d1 100644 --- a/app/src/test/java/org/groundplatform/android/ui/datacollection/tasks/polygon/PolygonDrawingSessionTest.kt +++ b/app/src/test/java/org/groundplatform/android/ui/datacollection/tasks/polygon/PolygonDrawingSessionTest.kt @@ -32,7 +32,6 @@ class PolygonDrawingSessionTest { @Test fun `Initial state is empty`() { assertThat(session.vertices).isEmpty() - assertThat(session.tentativeVertex).isNull() assertThat(session.displayVertices).isEmpty() assertThat(session.redoVertexStack).isEmpty() } @@ -46,19 +45,17 @@ class PolygonDrawingSessionTest { } @Test - fun `updateTentativeVertex sets tentative vertex when empty`() { + fun `updateTentativeVertex updates displayVertices when empty`() { session.updateTentativeVertex(COORDINATE_1) { _, _ -> DISTANCE_ABOVE_THRESHOLD } assertThat(session.vertices).isEmpty() - assertThat(session.tentativeVertex).isEqualTo(COORDINATE_1) assertThat(session.displayVertices).containsExactly(COORDINATE_1) } @Test - fun `updateTentativeVertex sets tentative vertex without changing committed list`() { + fun `updateTentativeVertex updates displayVertices without changing committed vertices`() { session.setVertices(listOf(COORDINATE_1)) session.updateTentativeVertex(COORDINATE_2) { _, _ -> DISTANCE_ABOVE_THRESHOLD } assertThat(session.vertices).containsExactly(COORDINATE_1) - assertThat(session.tentativeVertex).isEqualTo(COORDINATE_2) assertThat(session.displayVertices).containsExactly(COORDINATE_1, COORDINATE_2).inOrder() } @@ -69,7 +66,6 @@ class PolygonDrawingSessionTest { // Target is close to COORDINATE_1 session.updateTentativeVertex(COORDINATE_4) { _, _ -> DISTANCE_BELOW_THRESHOLD } assertThat(session.vertices).containsExactly(COORDINATE_1, COORDINATE_2, COORDINATE_3).inOrder() - assertThat(session.tentativeVertex).isEqualTo(COORDINATE_1) assertThat(session.displayVertices) .containsExactly(COORDINATE_1, COORDINATE_2, COORDINATE_3, COORDINATE_1) .inOrder() @@ -82,7 +78,6 @@ class PolygonDrawingSessionTest { // Target is far from COORDINATE_1 session.updateTentativeVertex(COORDINATE_4) { _, _ -> DISTANCE_ABOVE_THRESHOLD } assertThat(session.vertices).containsExactly(COORDINATE_1, COORDINATE_2, COORDINATE_3).inOrder() - assertThat(session.tentativeVertex).isEqualTo(COORDINATE_4) assertThat(session.displayVertices) .containsExactly(COORDINATE_1, COORDINATE_2, COORDINATE_3, COORDINATE_4) .inOrder() @@ -117,11 +112,11 @@ class PolygonDrawingSessionTest { } @Test - fun `commitTentativeVertex adds tentative vertex to committed list`() { + fun `commitTentativeVertex adds active vertex to committed list`() { session.updateTentativeVertex(COORDINATE_1) { _, _ -> DISTANCE_ABOVE_THRESHOLD } session.commitTentativeVertex(null) assertThat(session.vertices).containsExactly(COORDINATE_1) - assertThat(session.tentativeVertex).isNull() + assertThat(session.displayVertices).containsExactly(COORDINATE_1) } @Test @@ -142,13 +137,13 @@ class PolygonDrawingSessionTest { } @Test - fun `removeLastVertex clears tentative vertex when committed list is empty`() { + fun `removeLastVertex clears active target when committed list is empty`() { session.updateTentativeVertex(COORDINATE_1) { _, _ -> DISTANCE_ABOVE_THRESHOLD } - assertThat(session.tentativeVertex).isEqualTo(COORDINATE_1) + assertThat(session.displayVertices).containsExactly(COORDINATE_1) assertThat(session.removeLastVertex()).isTrue() assertThat(session.vertices).isEmpty() - assertThat(session.tentativeVertex).isNull() + assertThat(session.displayVertices).isEmpty() } @Test @@ -157,7 +152,6 @@ class PolygonDrawingSessionTest { assertThat(session.removeLastVertex()).isTrue() assertThat(session.vertices).containsExactly(COORDINATE_1) - assertThat(session.tentativeVertex).isNull() assertThat(session.displayVertices).containsExactly(COORDINATE_1) assertThat(session.redoVertexStack).containsExactly(COORDINATE_2) } @@ -168,7 +162,7 @@ class PolygonDrawingSessionTest { assertThat(session.removeLastVertex()).isTrue() assertThat(session.vertices).isEmpty() - assertThat(session.tentativeVertex).isNull() + assertThat(session.displayVertices).isEmpty() assertThat(session.redoVertexStack).containsExactly(COORDINATE_1) } @@ -184,7 +178,6 @@ class PolygonDrawingSessionTest { assertThat(session.redoLastVertex()).isEqualTo(COORDINATE_2) assertThat(session.vertices).containsExactly(COORDINATE_1, COORDINATE_2).inOrder() - assertThat(session.tentativeVertex).isNull() assertThat(session.displayVertices).containsExactly(COORDINATE_1, COORDINATE_2).inOrder() assertThat(session.redoVertexStack).isEmpty() } @@ -250,7 +243,7 @@ class PolygonDrawingSessionTest { session.complete() assertThat(session.state.isMarkedComplete).isTrue() assertThat(session.vertices).containsExactly(C1, C3, C2, C4, C1).inOrder() - assertThat(session.tentativeVertex).isNull() + assertThat(session.displayVertices).containsExactly(C1, C3, C2, C4, C1).inOrder() } @Test @@ -281,7 +274,7 @@ class PolygonDrawingSessionTest { session.updateTentativeVertex(Coordinates(50.0, 50.0)) { _, _ -> DISTANCE_ABOVE_THRESHOLD } assertThat(session.vertices).containsExactly(C1, C3, C2, C4, C1).inOrder() - assertThat(session.tentativeVertex).isNull() + assertThat(session.displayVertices).containsExactly(C1, C3, C2, C4, C1).inOrder() assertThat(session.state.isTooClose).isTrue() } From 7913d08ffd0aa35d94ae8ad24e1ff85fef9b6409 Mon Sep 17 00:00:00 2001 From: Shobhit Agarwal Date: Thu, 21 May 2026 20:04:59 +0530 Subject: [PATCH 5/6] Add coverage for non-null tooltip --- .../polygon/DrawAreaTaskViewModelTest.kt | 34 +++++++++++++++---- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/app/src/test/java/org/groundplatform/android/ui/datacollection/tasks/polygon/DrawAreaTaskViewModelTest.kt b/app/src/test/java/org/groundplatform/android/ui/datacollection/tasks/polygon/DrawAreaTaskViewModelTest.kt index cff7dec4d9..a04283c4aa 100644 --- a/app/src/test/java/org/groundplatform/android/ui/datacollection/tasks/polygon/DrawAreaTaskViewModelTest.kt +++ b/app/src/test/java/org/groundplatform/android/ui/datacollection/tasks/polygon/DrawAreaTaskViewModelTest.kt @@ -320,14 +320,34 @@ class DrawAreaTaskViewModelTest : BaseHiltTest() { assertThat(firstLine.coordinates.size).isEqualTo(1) assertThat(firstFeature.tooltipText).isNull() - updateLastVertexAndAdd(COORDINATE_2) - advanceUntilIdle() - val secondFeature = viewModel.draftArea.first() - val secondLine = secondFeature!!.geometry as LineString - assertThat(secondLine.coordinates.size).isEqualTo(1) - assertThat(secondFeature.tooltipText).isNull() + viewModel.draftUpdates.test { + updateLastVertexAndAdd(COORDINATE_2) + advanceUntilIdle() - viewModel.removeLastVertex() + // 1. Consume the transient active target update (from updateLastVertex) + val secondFeature = awaitItem() + val secondLine = secondFeature.geometry as LineString + assertThat(secondLine.coordinates.size).isEqualTo(2) + assertThat(secondFeature.tooltipText).isEqualTo("1,565,109 m") + + // 2. Consume the committed vertex update (from addLastVertex) + val committedFeature = awaitItem() + val committedLine = committedFeature.geometry as LineString + assertThat(committedLine.coordinates.size).isEqualTo(2) + assertThat(committedFeature.tooltipText).isEqualTo("1,565,109 m") + + // 3. Revert/Undo last vertex + viewModel.removeLastVertex() + advanceUntilIdle() + + // 4. Consume the undo update (from removeLastVertex) + val thirdFeature = awaitItem() + val thirdLine = thirdFeature.geometry as LineString + assertThat(thirdLine.coordinates.size).isEqualTo(1) + assertThat(thirdFeature.tooltipText).isNull() + + cancelAndIgnoreRemainingEvents() + } } @Test From ffb8a30362a5766165c2c189ef1a8891e7aaf8c0 Mon Sep 17 00:00:00 2001 From: Shobhit Agarwal Date: Thu, 21 May 2026 20:07:36 +0530 Subject: [PATCH 6/6] Add test for self-intersection --- .../org/groundplatform/domain/util/PolygonUtilTest.kt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/core/domain/src/commonTest/kotlin/org/groundplatform/domain/util/PolygonUtilTest.kt b/core/domain/src/commonTest/kotlin/org/groundplatform/domain/util/PolygonUtilTest.kt index 87b6257f1f..c9525d45f6 100644 --- a/core/domain/src/commonTest/kotlin/org/groundplatform/domain/util/PolygonUtilTest.kt +++ b/core/domain/src/commonTest/kotlin/org/groundplatform/domain/util/PolygonUtilTest.kt @@ -70,6 +70,12 @@ class PolygonUtilTest { assertFalse(isSelfIntersecting(TRIANGLE)) } + @Test + fun `closed triangle does not self intersect`() { + val closedTriangle = TRIANGLE + TRIANGLE.first() + assertFalse(isSelfIntersecting(closedTriangle)) + } + @Test fun `empty list`() { assertFalse(isSelfIntersecting(emptyList()))