Restyle geoshape/geotrace by seadowg · Pull Request #7017 · getodk/collect

seadowg · 2026-01-09T16:04:25Z

Work towards #6962
Closes #6999
Closes #7019

This restyles traces and shapes in OSM. I also fixed a crash that could happen on rotate (#6999) as it fitted in to some work I needed to do anyway (@grzesiek2010 discussed that in #7019).

Why is this the best possible solution? Were any other approaches considered?

Comments inline.

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

Do we need any specific form for testing your changes? If so, please attach one.

It's worth playing with all map engines to see if there are any performance regressions with adding/removing points with shapes/traces as how they are rendered has been changed across the board. Other than that (and the fixed issue), the style changes are isolated to OSM - Google Maps and Mapbox look a little wonky, but we'll be fixing that in follow-ups. It might be worth just doing a quick pass of things for now as we'll be making further changes to the maps with #6970.

Does this change require updates to documentation? If so, please file an issue here and include the link below.

Before submitting this PR, please make sure you have:

added or modified tests for any new or changed behavior
run ./gradlew connectedAndroidTest (or ./gradlew testLab) and confirmed all checks still pass
added a comment above any new strings describing it for translators
added any new strings with date formatting to DateFormatsTest
verified that any code or assets from external sources are properly credited in comments and/or in the about file.
verified that any new UI elements use theme colors. UI Components Style guidelines

seadowg · 2026-01-09T16:06:06Z

@alyblenkin I've updated the style for OSM. Take a look and let me know if the behaviour/style is as expected, and then I can get working on making it work in Google Maps/Mapbox.

alyblenkin · 2026-01-13T15:01:39Z

Based on our conversation in slack, we want to align more closely with the designs from Web Forms.

https://effortless-dragon-d2ff5f.netlify.app/

seadowg · 2026-01-15T09:54:52Z

@dbemke @srujner @WKobus are you able to have a quick play with traces/shapes? They should look inconsistent right now, but I've made some changes to simplify how they work, and I'm interested to see if it has any performance drawbacks. I'm going to have a go as well. I can end a Google Maps APK if you need it!

srujner · 2026-01-15T10:13:36Z

@seadowg Can it wait until Monday?
We're focusing on testing #7016 right now, and there's a lot to check there.

Google Maps APK would be helpful :)

seadowg · 2026-01-15T10:26:23Z

Absolutely! Not a priority, just something to check before I finish the PR off.

alyblenkin · 2026-01-15T21:20:15Z

Noting down a few things from my chat with Callum:

Points size: They are currently 6×, which feels better than before. They feel appropriate while actively mapping, but appear pretty large when zoomed out. Dynamically resizing markers based on zoom would be a larger lift, so we’re planning to leave this for now.
Invalid state: the most recently placed point will not be highlighted when invalid, since all points will be shown in red. This may be revisited later based on feedback.
Select state: later we want to clearly indicate the selected point so that moving points is more discoverable, like we do with WF.

dbemke · 2026-01-16T11:10:32Z

I'm interested to see if it has any performance drawbacks.

Can you give us a hint where to look for the performance issues - adding points, saving points etc?

seadowg · 2026-01-16T11:35:40Z

Can you give us a hint where to look for the performance issues - adding points, saving points etc?

Adding and removing points with shapes/traces mainly!

seadowg · 2026-01-16T13:05:07Z

geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyFragment.kt

            }
        }

+    private val viewModel: GeoPolyViewModel by viewModels {


We can probably move a bunch more of the Fragment's state in to this (or another) ViewModel, but I think this is a good start for now: we can continue improving this as part of #6970.

seadowg · 2026-01-16T13:08:01Z

maps/src/main/java/org/odk/collect/maps/MapFragment.kt

     */
    fun hasCenter(): Boolean

+    fun supportsDraggablePolygon(): Boolean {


This lets us implement in just OSM while not completely breaking other engines. I think we should follow up with Mapbox/Google Maps implementations as that allows us to keep the change sets small and do the work in parallel if we can.

seadowg · 2026-01-16T13:09:21Z

maps/src/main/java/org/odk/collect/maps/MapFragment.kt

-     * If there are no vertices, does nothing.
-     */
-    fun removePolyLineLastPoint(featureId: Int)
+    fun updatePolygon(featureId: Int, polygonDescription: PolygonDescription)


For the moment, updatePolyLine and updatePolygon aren't optimized at all, but I thought it would be good to have it set up so we can work on optimizations within the map engine code without risk of merge conflicts with work happening in GeoPolyFragment.

seadowg · 2026-01-16T13:17:02Z

maps/src/main/java/org/odk/collect/maps/LineDescription.kt

    private val strokeColor: String? = null,
    val draggable: Boolean = false,
-    val closed: Boolean = false
+    @Deprecated("Use PolygonDescription instead") val closed: Boolean = false


Our line/polygon API for MapFragment was in a weird state where you could add draggable or static lines, static polygons and draggale or static closed lines. We got away with the latter because we didn't need to fill them, but now we do they need to be proper polygons under the hood.

dbemke · 2026-01-16T13:30:50Z

I think I found an Easter egg !!! Hidden point in the geotrace (on all devices).

Steps to reproduce:

Open All question types form in the demo project.
Tap "get line" in geotrace.
Select e.g. placement by tappping and add some point.
Tap pause and remove the points except from one
Tap the back button and discard changes.
Open the map again - the hidden point is there ;)

The way points are entered and removed doesn't matter. I haven't found it on the master version or the store version.

seadowg · 2026-01-16T13:42:17Z

geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyViewModel.kt

+    private var accuracyThreshold: Int = 0
+
+    init {
+        viewModelScope.launch {


We've talked off and on about whether to use coroutines directly like this (as opposed to through our Scheduler interface) - especially with ViewModels as it gives us viewModelScope. I can't immediately think of a benefit to using Scheduler here as this work is dispatched on the UI thread, so there's no need to integrate with IdlingResource for Espresso. That said, as far as I can tell, there are no tests for the automatic location recording. I'll backfill those as a follow up and see how this fits in.

seadowg · 2026-01-16T14:48:26Z

I think I found an Easter egg !!! Hidden point in the geotrace (on all devices).

I can't reproduce. Could you double-check the steps? Is the point you see always in the same place?

grzesiek2010 · 2026-01-16T21:51:35Z

maps/src/main/java/org/odk/collect/maps/MapConsts.kt

 object MapConsts {
-    const val DEFAULT_STROKE_COLOR = -65536 // color-int representation of #ffff0000
+    @JvmField
+    val DEFAULT_STROKE_COLOR = Color.parseColor("#3e9fcc")


There is a nice toColorInt ext fun which you can use here and below: Color.parseColor("#3e9fcc") -> #3e9fcc".toColorInt().

That function returns Int? rather than Int - I'm guessing it's for dealing with user input rather than static strings.

That function returns Int? rather than Int

No, it returns Int:

@ColorInt public inline fun String.toColorInt(): Int = Color.parseColor(this)

and under the hood it does the same thing.

I've added that change in #7024 so that merging this pr is not blocked.

Ah looks like we've shadowed it:

collect/androidshared/src/main/java/org/odk/collect/androidshared/utils/ColorUtils.kt

Line 7 in 3daae3e

fun String.toColorInt() = try {

I can rename that.

osmdroid/src/main/java/org/odk/collect/osmdroid/OsmDroidMapFragment.java

maps/src/main/java/org/odk/collect/maps/markers/MarkerIconCreator.kt

maps/src/main/java/org/odk/collect/maps/markers/MarkerIconDescription.kt

grzesiek2010 · 2026-01-17T00:01:35Z

location/src/main/java/org/odk/collect/location/tracker/ForegroundServiceLocationTracker.kt

            val interval = intent.getLongExtra(EXTRA_UPDATE_INTERVAL, -1)
            locationClient.setUpdateIntervals(
                interval,
-                interval / 2


Should we simplify #setUpdateIntervals as now we alwasy pass the same values?

I can follow up with that as I'd like to investigate white it was the way it was.

mapbox/src/main/java/org/odk/collect/mapbox/MapboxMapFragment.kt

geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyViewModel.kt

grzesiek2010 · 2026-01-17T00:53:28Z

I think I found an Easter egg !!! Hidden point in the geotrace (on all devices).

I can't reproduce. Could you double-check the steps? Is the point you see always in the same place?

Yes it was adding a point in the center of the map. I've solved it in #7016 so just rebase.

osmdroid/src/main/java/org/odk/collect/osmdroid/OsmDroidMapFragment.java

dbemke · 2026-01-19T07:12:58Z

Could you double-check the steps? Is the point you see always in the same place?

It always the same place. I reproduced it also in All widgets form. Steps are on OSM and I edit one step to leave one point.

seadowg · 2026-01-19T14:39:38Z

@dbemke I've rebased so that issue should be fixed now!

dbemke · 2026-01-21T12:53:49Z

Tested with Success!

Verified on devices with Android 10

Verified cases:

issues Crash after rotating the screen with automatic location recording on in geoshape and geotrace #6999 Fix crash after rotating the screen with automatic location recording on in geoshape and geotrace #7019 don't occur
shapes/traces not intersecting and incremental=true in OSM, Google Maps, Mapbox
quick checks in USGS and Carto
adding and removing points
snackbars
error messages
regression checks in geoshape and geotrace

WKobus · 2026-01-21T12:57:20Z

Tested with Success!

Verified on devices with Android 16

seadowg mentioned this pull request Jan 13, 2026

Fix crash after rotating the screen with automatic location recording on in geoshape and geotrace #7019

Closed

6 tasks

seadowg force-pushed the trace-style branch from f3ae61f to cf962f0 Compare January 14, 2026 17:54

seadowg commented Jan 16, 2026

View reviewed changes

seadowg requested a review from grzesiek2010 January 16, 2026 14:48

seadowg marked this pull request as ready for review January 16, 2026 14:48

grzesiek2010 requested changes Jan 17, 2026

View reviewed changes

grzesiek2010 reviewed Jan 18, 2026

View reviewed changes

osmdroid/src/main/java/org/odk/collect/osmdroid/OsmDroidMapFragment.java Outdated Show resolved Hide resolved

seadowg force-pushed the trace-style branch from a3c8918 to 58baa6f Compare January 19, 2026 09:29

seadowg mentioned this pull request Jan 19, 2026

Implement new geoshape/trace style for Google/Mapbox #7024

Merged

6 tasks

seadowg force-pushed the trace-style branch from 914a761 to 593abb4 Compare January 19, 2026 10:55

seadowg added 4 commits January 19, 2026 14:09

Change default color for lines in maps

736ef94

Simplify MapIconCreator interface

36fad66

Add convenience extension for create marker icon bitmaps

ecd7772

Add new specific icon for line points

c825cc2

seadowg added 18 commits January 19, 2026 14:09

Fix whitespace issue

91b0bea

Add updatePolyLine method

6ae1adb

Add updatePolygon method

d0281d4

Simplify feature description constructor calls

2774e36

Highlight last point with different color

055f227

Update fake

f3147e3

Clean up unneeded code

3e07038

Break up single line

d508feb

Rename method

974898f

Rename class

a63222a

Remove double save to cache

f3c1845

Correct whitespace

46f0155

Fix formatting

16eabc9

Correct typo

2164907

Use isNotEmpty

72a978f

Use last()

f6ded2d

Remove redundant let

35ae10e

Remove redundant alpha setting

e6d579d

seadowg force-pushed the trace-style branch from 2bb4858 to e6d579d Compare January 19, 2026 14:09

Only update map when points updated

0b63d63

seadowg requested a review from grzesiek2010 January 19, 2026 14:39

grzesiek2010 approved these changes Jan 19, 2026

View reviewed changes

grzesiek2010 merged commit 3daae3e into getodk:master Jan 19, 2026
7 checks passed

grzesiek2010 added the needs testing label Jan 19, 2026

seadowg deleted the trace-style branch January 21, 2026 13:21

dbemke added behavior verified and removed needs testing labels Jan 21, 2026

Conversation

seadowg commented Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why is this the best possible solution? Were any other approaches considered?

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

Do we need any specific form for testing your changes? If so, please attach one.

Does this change require updates to documentation? If so, please file an issue here and include the link below.

Before submitting this PR, please make sure you have:

Uh oh!

seadowg commented Jan 9, 2026

Uh oh!

alyblenkin commented Jan 13, 2026

Uh oh!

seadowg commented Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

srujner commented Jan 15, 2026

Uh oh!

seadowg commented Jan 15, 2026

Uh oh!

alyblenkin commented Jan 15, 2026

Uh oh!

dbemke commented Jan 16, 2026

Uh oh!

seadowg commented Jan 16, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dbemke commented Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

seadowg commented Jan 16, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

seadowg Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

grzesiek2010 commented Jan 17, 2026

Uh oh!

Uh oh!

dbemke commented Jan 19, 2026

Uh oh!

seadowg commented Jan 19, 2026

Uh oh!

Uh oh!

dbemke commented Jan 21, 2026

Uh oh!

WKobus commented Jan 21, 2026

Uh oh!

Reviewers

Assignees

Labels

seadowg commented Jan 9, 2026 •

edited

Loading

seadowg commented Jan 15, 2026 •

edited

Loading

dbemke commented Jan 16, 2026 •

edited

Loading

seadowg Jan 20, 2026 •

edited

Loading