feat: add support for custom offline maps #318

achou11 · 2024-05-06T19:45:45Z

Closes #243

Notes:

decision tree for which map's style.json to serve (EDIT: when map initially loads/renders):

Remaining items:

on my Pixel 2 Android 11, zooming out eventually causes the app to crash. have only tried this with one example map so not sure if it's specific to this map or a general issue. i get the following some cryptic logs from adb but not sure how to debug:

NODEJS-MOBILE: exiting due to SIG_DFL handler for signal 11

backtrace:
  #00 pc 000000000004e0f4  /apex/com.android.runtime/lib64/bionic/libc.so (abort+180) (BuildId:   
  #01 pc 00000000000050b8  /system/bin/app_process64 (art::SignalChain::Handler(int, siginfo*, void*)+1120) (BuildId:   
  #02 pc 00000000000008b0  [vdso] (__kernel_rt_sigreturn)
  #03 pc 0000000000118c3c  [anon:scudo:primary]

One thing to try next: upload same map to Mapeo Mobile and see if same crash happens.

EDIT: I'm no longer getting the crash for mysterious reasons...

attempting to test this on my Pixel 6 Android 14 is very difficult due to storage permission changes introduced in Android 13+14. Basically almost impossible without using adb and even then, it requires some complicated workarounds that I have yet to figure out (think other members of our team have done it before). EDIT: will not test on pixel 6 for now

gmaclennan · 2024-05-07T15:45:44Z

Some initial ideas for debugging (you may have already tried): turn on logging for fastify so you can see what requests are being handled, so you can trace back what is happening up to the crash.

achou11 · 2024-05-13T21:31:11Z

For reasons unknown, I'm no longer getting the crash. Didn't do anything special...

ErikSin

The use of useMapStyle() feels unnecessary with the tanstack query.

I think it would be better to use a suspense query (The map is the main feature, so i think it makes sense to block the entire app while it is loading)

And then we could create a custom hook for invalidating the query when the user goes offline/online

// custom hook
function useInvalidateMapUrlQuery(){
     const {isInternetReachable} = useNetInfo();
     const queryClient = useQueryClient()
     useEffect(()=>{
          queryClient.invalidateQuery({queryKey:[MAP_STYLE_URL_KEY]})
     },[queryClient, isInternetReachable])
}

// inside map screen:
const MapScreen = () => {
     const {data:mapStyleUrl} = useMapStyleUrl() //this is a suspense query, so no loading needs to be taken into account
     useInvalidateMapUrlQuery()

     // rest of component
}

The code feels a little cleaner if we are just utilizing tanstack query.

The other thing that we should maybe do is only change to the offline map if there isnt an online map already loaded. Aka, if the user is online and has a map loaded and then goes offline, don't reload the map with offline one, just keep the online one (even though we wont have any zoom) The online map at the farthest zoom feels better than using an offline map.

Basically the only time we should serve an offline map is when they open the app initally without any access to the internet.

ErikSin · 2024-05-13T23:41:13Z

src/backend/src/app.js

+  // Register maps plugins
+  fastify.register(MapeoStaticMapsFastifyPlugin, {
+    prefix: 'static',
+    staticRootDir: staticStylesDir,
+  })
+  fastify.register(MapeoOfflineFallbackMapFastifyPlugin, {
+    prefix: 'fallback',
+    styleJsonPath: join(fallbackMapPath, 'style.json'),
+    sourcesDir: join(fallbackMapPath, 'dist'),
+  })
+  fastify.register(MapeoMapsFastifyPlugin, {
+    prefix: 'maps',
+    defaultOnlineStyleUrl: DEFAULT_ONLINE_MAP_STYLE_URL,
+  })
+


Can you explain what this is for?

it's to add maps functionality to the fastify instance that we use for serving things over http. the functionality is separate into 3 plugins:

static: provides ability to statically serve custom offline maps, similar to how Mapeo Mobile works

fallback: provides ability to statically serve a directory that acts as the fallback map (which are set up differently that custom offline maps). this is mostly to accommodate for the fallback map structure that's used by Mapeo Mobile.

maps: currently acts as the main resolver for the user-facing API e.g. resolving the appropriate style.json based on existence of static/fallback maps. in the future, this will probably hold other functionality related to features that are more similar to the Map Manager in Mapeo Mobile.

achou11 · 2024-05-14T16:24:03Z

The other thing that we should maybe do is only change to the offline map if there isnt an online map already loaded.

not entirely sure which map you're referring to when saying "offline", but for custom background maps, the legacy behavior is that if there's a custom background map loaded (e.g. a map that exists in files/styles/default folder in android app storage), then that takes first priority, even if you're online. if you're talking about switching from online tiles to the offline fallback map, that would require fixes on the backend side and probably better to do as a follow-up if needed.

Regarding the proposed approach for adjusting the setup for the query, your suggestion as is will not work because the URL returned by the api is stable during the lifetime of the backend server and will not change based on internet connectivity. Basically it will always point to http://localhost:<some_port>/maps/style.json. Only invalidating the query will return the same URL, but what we need is a way for the style.json resource to be re-requested by the map library, which is what the current solution of using a search param does in this case. the search param is ignored by the server on the backend - it's basically used to tell the mapping library to re-request the style.json resource.

Agree that the query can probably be switched to a suspense query, but the useMapStyle() hook as it's implemented otherwise still feels appropriate

achou11 · 2024-05-14T16:32:03Z

The other thing that we should maybe do is only change to the offline map if there isnt an online map already loaded. Aka, if the user is online and has a map loaded and then goes offline, don't reload the map with offline one, just keep the online one (even though we wont have any zoom) The online map at the farthest zoom feels better than using an offline map.

Agree that there's probably improvements to the UX around this stuff. but this PR is mostly focused on porting over the existing behavior from Mapeo Mobile, which is described by the decision tree above

achou11 · 2024-05-14T17:04:26Z

hm switching to use a suspense query doesn't seem to work for me 🤔 end up with just a blank white space where the map's supposed to be. maybe due to the mapping library not playing well with suspense, or where our suspense boundary is?

achou11 · 2024-05-14T17:33:40Z

The other thing that we should maybe do is only change to the offline map if there isnt an online map already loaded.

not entirely sure which map you're referring to when saying "offline", but for custom background maps, the legacy behavior is that if there's a custom background map loaded (e.g. a map that exists in files/styles/default folder in android app storage), then that takes first priority, even if you're online. if you're talking about switching from online tiles to the offline fallback map, that would require fixes on the backend side and probably better to do as a follow-up if needed.

Regarding the proposed approach for adjusting the setup for the query, your suggestion as is will not work because the URL returned by the api is stable during the lifetime of the backend server and will not change based on internet connectivity. Basically it will always point to http://localhost:<some_port>/maps/style.json. Only invalidating the query will return the same URL, but what we need is a way for the style.json resource to be re-requested by the map library, which is what the current solution of using a search param does in this case. the search param is ignored by the server on the backend - it's basically used to tell the mapping library to re-request the style.json resource.

Agree that the query can probably be switched to a suspense query, but the useMapStyle() hook as it's implemented otherwise still feels appropriate

I misspoke on the first part here. Without any custom basemap, I thought the behavior when going from online to offline was that the fallback offline map would immediately be displayed in the app. Based on testing Mapeo Mobile, this is incorrect, and what you suggested is what actually happens in this case (again, wasn't sure what you mean by "offline"). Seems like the fallback offline map is never dynamically displayed i.e. when network state changes while app is active. It seems that it's only ever displayed when the map initially renders.

I think what I was trying to improve with this change was the opposite situation: what if you're offline but then you become online? (again, no custom basemaps) Seems like it would be a nice feature to be able to dynamically switch to the online map in that case, but that's not what happens in Mapeo Mobile currently, so maybe that would be a follow-up

This is an easy fix in this PR, as it basically means I can get rid of the network-related parts of the changes, which will simplify the PR as a side effect!

ErikSin

Great, everything is working

This is out of scope of this PR, but we should check when the user goes back online and serve that online map without forcing them to close the app.

ErikSin · 2024-05-14T21:29:47Z

For the posterity I didn't actually test this with a custom map...For the MVP release that doesn't feel like a priority so I will make note it for the QA testers

achou11 · 2024-05-14T21:56:37Z

For the posterity I didn't actually test this with a custom map...For the MVP release that doesn't feel like a priority so I will make note it for the QA testers

also for posterity, i tested this on my Pixel 2 Android 11 using some custom maps from the Mapeo testing kit (which we have in Google Drive)

achou11 force-pushed the 243/offline-maps branch 4 times, most recently from 2546868 to d17e176 Compare May 13, 2024 21:27

achou11 marked this pull request as ready for review May 13, 2024 21:47

achou11 requested review from gmaclennan and ErikSin and removed request for gmaclennan May 13, 2024 21:49

ErikSin requested changes May 14, 2024

View reviewed changes

achou11 added 8 commits May 14, 2024 13:09

WIP

0babcf8

fix path

2d729d4

update map screen

7de8f45

update messages

7b2c9ad

refetch url when internet access changes

89e692d

fix static plugin init

517f222

simplify usage

52c5541

revert unintentional change in message

070ebcd

achou11 force-pushed the 243/offline-maps branch from f79cdc0 to 070ebcd Compare May 14, 2024 17:15

achou11 added 2 commits May 14, 2024 13:18

fix formatting errors

92ce896

add comment to explain useMapStyle() hook

e8e545b

remove internet-based url changing

7f559f1

achou11 requested a review from ErikSin May 14, 2024 17:44

update InsetMapView to use style url query hook

5fe8e20

achou11 force-pushed the 243/offline-maps branch from 82847fd to 5fe8e20 Compare May 14, 2024 20:34

fix path for fallback map

903fc22

ErikSin approved these changes May 14, 2024

View reviewed changes

achou11 merged commit f8219a2 into main May 14, 2024
3 checks passed

achou11 deleted the 243/offline-maps branch May 14, 2024 21:47

achou11 mentioned this pull request May 14, 2024

Should attempt to display online map when user regains internet connection #350

Open

achou11 mentioned this pull request Oct 15, 2024

chore: update core and ipc to v2.0.0 #796

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add support for custom offline maps #318

feat: add support for custom offline maps #318

achou11 commented May 6, 2024 •

edited

Loading

gmaclennan commented May 7, 2024

achou11 commented May 13, 2024

ErikSin left a comment

ErikSin May 13, 2024

achou11 May 14, 2024 •

edited

Loading

achou11 commented May 14, 2024 •

edited

Loading

achou11 commented May 14, 2024

achou11 commented May 14, 2024

achou11 commented May 14, 2024 •

edited

Loading

ErikSin left a comment

ErikSin commented May 14, 2024

achou11 commented May 14, 2024 •

edited

Loading

feat: add support for custom offline maps #318

feat: add support for custom offline maps #318

Conversation

achou11 commented May 6, 2024 • edited Loading

gmaclennan commented May 7, 2024

achou11 commented May 13, 2024

ErikSin left a comment

Choose a reason for hiding this comment

ErikSin May 13, 2024

Choose a reason for hiding this comment

achou11 May 14, 2024 • edited Loading

Choose a reason for hiding this comment

achou11 commented May 14, 2024 • edited Loading

achou11 commented May 14, 2024

achou11 commented May 14, 2024

achou11 commented May 14, 2024 • edited Loading

ErikSin left a comment

Choose a reason for hiding this comment

ErikSin commented May 14, 2024

achou11 commented May 14, 2024 • edited Loading

achou11 commented May 6, 2024 •

edited

Loading

achou11 May 14, 2024 •

edited

Loading

achou11 commented May 14, 2024 •

edited

Loading

achou11 commented May 14, 2024 •

edited

Loading

achou11 commented May 14, 2024 •

edited

Loading