google / google/ground-android
[Bug] DrawAreaTask saves duplicate points and allows degenerate polygons
- Dominant language
- Kotlin
- Stars
- 293
- Forks
- 149
- Avg merge
- 1d 16h
- Merged PRs (30d)
- 42
Description
### Description
During a refactor of the `DrawAreaTask` state management to adhere to Unidirectional Data Flow (UDF) principles (https://github.com/google/ground-android/pull/3678), several issues were identified in the polygon drawing logic that could lead to invalid geometry being saved and a confusing user experience.
### Detected Issues
1. **Duplicate Points in Storage:** Incomplete polygons were saving duplicate points (specifically duplicating the last vertex), resulting in 4 points instead of 3 for a triangle.
1. **Incorrect Undo Behavior:** The Undo action would remove a committed vertex even when there was a tentative vertex actively being drawn on the screen.
1. **Missing Validation for Degenerate Shapes:** The proximity check (`isTooClose`) failed to trigger when closing a triangle with only 2 committed points, allowing degenerate lines to be formed.
1. **Premature Button Visibility:** The `COMPLETE` button was displayed for closed shapes that didn't have enough distinct points to form a valid polygon (requires 3 distinct points).
### Proposed Fix
The implementation has been refactored in the current branch to:
- Separate committed vertices from the transient tentative vertex in the ViewModel state.
- Clear the tentative vertex upon commit.
- Enforce a minimum of 3 distinct points for closure validation.
- Update proximity checks to cover all edge cases when closing shapes.
- Introduce `PolygonDrawingSession` class to encapsulate list of committed vertices, transient tentative vertex, redo stack, operations like adding a vertex, undoing, and redoing.
### Steps to Reproduce
1. Start drawing an area task.
2. Place 3 points.
3. Observe that the saved incomplete geometry contains 4 points instead of 3.
Contributor guide
Assessment
This issue has not been assessed yet.