ground-android icon indicating copy to clipboard operation
ground-android copied to clipboard

Ghost-locks when moving the map and flickering "Add data collection site" card

Open jo-spek opened this issue 1 year ago • 5 comments

Relating partially to issue https://github.com/google/ground-android/issues/2727:

On my personal Android Device (Xiaomi Poco X3 NFC, Android 12 SKQ1.211019.001, GROUND build 0.1.9-openforis) in this survey, the map is randomly locked to the most recent place. Only when moving away and holding my finger on the touchscreen a little longer does the map move along and not jump back to the previous place. However, then the new place is sort of "ghost-locked". This makes sliding the map around for geolocation almost impossible. All the while, the "Add data collection site" card does randomly appear and disappear. See the video for better understanding.

The same did not happen on any other of my surveys, however, some of the 'smoothness' of gliding over the map is gone and there are frequent small 'jumpbacks' (for lack of a better word). They are only annoying and do not inhibit the work, but diminish the user experience.

https://github.com/user-attachments/assets/c6231977-8d52-4e56-8f1c-d0a1d18d895f

jo-spek avatar Sep 17 '24 14:09 jo-spek

So I just figured this happens the further I am away from the "center of action", i.e. where most mapping takes place. I was on the survey that takes place in Ghana but was testing it in Germany where the app just wouldn't work.

Faking my GPS to be close to Kumasi, the survey worked just fine, but coming closer to the Côte d'Ivoire border the misbehavior started again.

jo-spek avatar Sep 17 '24 18:09 jo-spek

I verified that this is happening when new sites (LOI) are received from the server. Two primary issues to investigate:

  • The viewport shouldn't be automatically updated when updates are received after the initial survey, jobs, and sites are loaded.
  • The viewport was reset multiple times before allowing me to pan again. This is likely due to the remote change triggering multiple updates. We should make sure only one update is triggered and applied.
  • WWe should also verify that multiple remote updates are applied serially rather than concurrently, and that there's some debounce behavior to ensure we don't thrash the network or device when they occur.

gino-m avatar Sep 19 '24 02:09 gino-m

@shobhitagarwal1612 you were looking into the viewport, do you have any observations to add here?

jcqli avatar Sep 25 '24 14:09 jcqli

I'm not sure why that is happening. Will try repro'ing it locally to understand what's happening under the hood

shobhitagarwal1612 avatar Sep 25 '24 15:09 shobhitagarwal1612

I'm not sure why that is happening. Will try repro'ing it locally to understand what's happening under the hood

Sounds good. Be sure to try with dev Firebase and not local emu, since Cloud Messaging isn't supported there.

gino-m avatar Sep 25 '24 15:09 gino-m

Initial debugging reveals that GoogleMapsFragment.setFeatures() is called 5 times when an LOI is modified in the remote datastore:

2024-11-11 16:17:54.337  3951-3951  GoogleMapsFragment      com.google.android.ground            V  setFeatures() called with 5 features
2024-11-11 16:17:54.712  3951-3951  GoogleMapsFragment      com.google.android.ground            V  setFeatures() called with 5 features
2024-11-11 16:17:54.720  3951-3951  GoogleMapsFragment      com.google.android.ground            V  setFeatures() called with 5 features
2024-11-11 16:17:54.726  3951-3951  GoogleMapsFragment      com.google.android.ground            V  setFeatures() called with 6 features
2024-11-11 16:17:54.732  3951-3951  GoogleMapsFragment      com.google.android.ground            V  setFeatures() called with 6 features

While SurveyRepository.loadAndSyncSurveyWithRemote is called only once as expected. We may need to debounce or otherwise isolate changes to the local db to prevent the UI from getting thrashed.

gino-m avatar Nov 11 '24 21:11 gino-m

Just verified that this isn't occurring in the latest codebase. @jo-spek can you please re-open if this occurs again?

gino-m avatar Nov 11 '24 22:11 gino-m

I just observed this again at HEAD right after new LOIs were synced. It appears that as the UI catches up with newly synced remote changes the map gets forced back to the previous viewport. Probably related to #2831.

gino-m avatar Nov 16 '24 14:11 gino-m

I believe #2835 may have fixed this. @jo-spek @jabramowitz5 Can you keep in eye out in case this recurs?

gino-m avatar Nov 21 '24 21:11 gino-m