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..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 @@ -150,8 +150,6 @@ internal constructor( .stateIn(viewModelScope, WhileSubscribed(5_000), emptyList()) } - - override fun initialize( job: Job, task: Task, @@ -326,36 +324,37 @@ internal constructor( * This coroutine runs on [viewModelScope] to ensure lifecycle safety. */ private fun refreshMap() = viewModelScope.launch { - if (session.vertices.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.vertices), + 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.vertices.size <= 1) return null - val distance = session.vertices.penult().distanceTo(session.vertices.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) } 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..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,6 +28,9 @@ interface PolygonDrawingSession { /** The list of committed vertices in the current drawing session. */ val vertices: List + /** 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..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 @@ -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,36 @@ class PolygonDrawingSessionImpl : PolygonDrawingSession { override val vertices: List get() = _vertices + private var _tentativeVertex: Coordinates? = null + + override val displayVertices: List + get() = _tentativeVertex?.let { _vertices + it } ?: _vertices + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) internal val redoVertexStack = mutableListOf() override val hasSelfIntersection: Boolean - get() = isSelfIntersecting(_vertices) - - private fun addVertex(vertex: Coordinates, shouldOverwriteLastVertex: Boolean) { - val updatedVertices = _vertices.toMutableList() - - if (shouldOverwriteLastVertex && updatedVertices.isNotEmpty()) { - updatedVertices.removeAt(updatedVertices.lastIndex) - } - - updatedVertices.add(vertex) - - _vertices = updatedVertices.toImmutableList() - } + 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 +72,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 } } @@ -97,20 +99,30 @@ class PolygonDrawingSessionImpl : PolygonDrawingSession { } override fun isValidPolygon(): Boolean = - LineString(_vertices).isClosed() && !isSelfIntersecting(_vertices) + 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 +132,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..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 @@ -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 @@ -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 @@ -568,10 +588,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..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,6 +32,7 @@ class PolygonDrawingSessionTest { @Test fun `Initial state is empty`() { assertThat(session.vertices).isEmpty() + assertThat(session.displayVertices).isEmpty() assertThat(session.redoVertexStack).isEmpty() } @@ -40,19 +41,22 @@ 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 updates displayVertices when empty`() { session.updateTentativeVertex(COORDINATE_1) { _, _ -> DISTANCE_ABOVE_THRESHOLD } - assertThat(session.vertices).containsExactly(COORDINATE_1) + assertThat(session.vertices).isEmpty() + assertThat(session.displayVertices).containsExactly(COORDINATE_1) } @Test - fun `updateTentativeVertex overwrites last vertex`() { + 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_2) + assertThat(session.vertices).containsExactly(COORDINATE_1) + assertThat(session.displayVertices).containsExactly(COORDINATE_1, COORDINATE_2).inOrder() } @Test @@ -61,7 +65,10 @@ 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.displayVertices) + .containsExactly(COORDINATE_1, COORDINATE_2, COORDINATE_3, COORDINATE_1) + .inOrder() } @Test @@ -70,7 +77,10 @@ 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.displayVertices) + .containsExactly(COORDINATE_1, COORDINATE_2, COORDINATE_3, COORDINATE_4) + .inOrder() } @Test @@ -102,9 +112,11 @@ class PolygonDrawingSessionTest { } @Test - fun `commitTentativeVertex adds vertex to committed list`() { - session.commitTentativeVertex(COORDINATE_1) + 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.displayVertices).containsExactly(COORDINATE_1) } @Test @@ -124,12 +136,23 @@ class PolygonDrawingSessionTest { assertThat(session.removeLastVertex()).isFalse() } + @Test + fun `removeLastVertex clears active target when committed list is empty`() { + session.updateTentativeVertex(COORDINATE_1) { _, _ -> DISTANCE_ABOVE_THRESHOLD } + assertThat(session.displayVertices).containsExactly(COORDINATE_1) + + assertThat(session.removeLastVertex()).isTrue() + assertThat(session.vertices).isEmpty() + assertThat(session.displayVertices).isEmpty() + } + @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.displayVertices).containsExactly(COORDINATE_1) assertThat(session.redoVertexStack).containsExactly(COORDINATE_2) } @@ -139,6 +162,7 @@ class PolygonDrawingSessionTest { assertThat(session.removeLastVertex()).isTrue() assertThat(session.vertices).isEmpty() + assertThat(session.displayVertices).isEmpty() assertThat(session.redoVertexStack).containsExactly(COORDINATE_1) } @@ -154,6 +178,7 @@ class PolygonDrawingSessionTest { assertThat(session.redoLastVertex()).isEqualTo(COORDINATE_2) assertThat(session.vertices).containsExactly(COORDINATE_1, COORDINATE_2).inOrder() + assertThat(session.displayVertices).containsExactly(COORDINATE_1, COORDINATE_2).inOrder() assertThat(session.redoVertexStack).isEmpty() } @@ -172,31 +197,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 +238,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.displayVertices).containsExactly(C1, C3, C2, C4, C1).inOrder() } @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 +259,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 +268,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.displayVertices).containsExactly(C1, C3, C2, C4, C1).inOrder() + assertThat(session.state.isTooClose).isTrue() + } + companion object { private const val DISTANCE_BELOW_THRESHOLD = 20.0 private const val DISTANCE_ABOVE_THRESHOLD = 30.0 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()))