[New] Generate offline map

[clee088]

Description

This PR implements Generate offline map in Maps category.
URL to README: URL

Linked Issue(s)

• common-samples/issues/3624

How To Test

1. Pan around the map to the area of interest you would like to be taken offline.
2. Tap the "Generate Offline Map" button and wait.
3. Tap the cancel button to test cancellation.

Screenshots

generate-offline-map-demo.mp4

To Discuss

1. Temporary job progress view
2. Viewpoint/Area of Interest: The viewpoint of the offline map is not centered, similar to the download vector tiles sample. The method of displaying the area of interest could also be improved--sometimes the red box doesn't show when the sample loads.
3. Map rotation is disabled for similar reasons as the download vector tiles sample.

[yo1995]

As we briefly mentioned, view model could be better for this sample.
I think we may need to jot down the criteria/threshold in the wiki on when to make it a view model, after finish reviewing the recent PRs.

[yo1995]

offline-map-tiles.mp4

To other reviewers: There seems to be an API bug for the generated offline map, will need 👀 from other reviewers. When the map is zoomed out with 2 fingers, it sometimes shows a larger extent than the red box. It seems that additional basemap tiles are included.

[yo1995]

issue logged at swift/issues/2008

[yo1995]

It seems sth is holding up the model when the view disappears/sample quits. When I enter the sample multiple times, it holds multiple instances of the model. I'll need to take another look tomorrow.

[yo1995]

It seems sth is holding up the model when the view disappears/sample quits. When I enter the sample multiple times, it holds multiple instances of the model. I'll need to take another look tomorrow.

I have experimented for quite a while and still could not figure out the root cause.

3 samples that uses view models currently:

• Find route: It doesn't release the view model even if the sample view has disappeared
• Generate offline map: same
• Show device location: same

Set surface placement mode: The view model is released

My observation is that the view model in Set surface placement mode sample doesn't have any async task involved. So I'm guessing something is off in the async part of these samples. I'll move on and keep reviewing the other parts of the code.

[clee088]

I think the root cause of the memory leak may lie within the alert modifier; more specifically, the isShowingAlert Boolean. When commenting out the .alert(...) modifier, the view model seems to be released from memory (although only when another sample is navigated to). Also, when swapping out the $model.isShowingAlert with a state Boolean, the view model also is deallocated. I'm not exactly sure why this is the cause so I'll look more into it for the rest of the day.

Also, I was wondering when is the observable object supposed to be released from memory? When I return to the list of samples, the view model is still in memory until I either navigate to the same sample, where it is released and initialized again, or I navigate to a different sample.

[yo1995]

I think the root cause of the memory leak may lie within the alert modifier; more specifically, the isShowingAlert Boolean. When commenting out the .alert(...) modifier, the view model seems to be released from memory (although only when another sample is navigated to). Also, when swapping out the $model.isShowingAlert with a state Boolean, the view model also is deallocated. I'm not exactly sure why this is the cause so I'll look more into it for the rest of the day.

Hmm this is quite 🤯 . Thanks for pointing this out. I'll give it another try on Xcode 14 beta.

Also, I was wondering when is the observable object supposed to be released from memory? When I return to the list of samples, the view model is still in memory until I either navigate to the same sample, where it is released and initialized again, or I navigate to a different sample.

This is an expected behavior, but not ideal. IMO It's due to the split screen navigation style. When exit from a sample, the view is not released until a new sample is selected. (think of an iPad) We discussed about this before in the old sample viewer but couldn't figure out a better way. 😕

[yo1995]

I think the root cause of the memory leak may lie within the alert modifier; more specifically, the isShowingAlert Boolean. When commenting out the .alert(...) modifier, the view model seems to be released from memory (although only when another sample is navigated to). Also, when swapping out the $model.isShowingAlert with a state Boolean, the view model also is deallocated. I'm not exactly sure why this is the cause so I'll look more into it for the rest of the day.

With some workarounds of the build phase bug, I tested it on Xcode 14 and iOS 16, and the models are no longer held. So I'm happy to let it slide for now as an iOS bug 😬

[yo1995]

LGTM with the comments above and below. Don't forget to solve merge conflict.

[clee088]

I also prefer handling the removal of temporary files in the view model, especially with the sample README and source code viewer. There seems to be a work around of the alert memory leak by receiving changes of the view model's error, and setting a local state variable to whether or not the error equals nil. This allows temporary files to be removed when the view model is released from memory.

Another thing I saw is that the generate offline map job is not cancelled when the view disappears, causing the view model to exist until the task is complete, even if the sample is not being viewed. Ideally the cancellation code would go in the deinit, but I can't figure out/don't know if it is allowed to have a task within the deinit. For now, it's implemented in the onDisappear modifier, which will cause the download task the cancel when the README viewer is presented while downloading.

[yo1995]

Thanks for the efforts! 💯

[philium]

For now, it's implemented in the onDisappear modifier, which will cause the download task the cancel when the README viewer is presented while downloading.

When the sample info view is implemented, if the visibility state of that view were passed down through the environment, then a check for that could be added here:

guard !isSampleInfoViewVisible else { return }

[philium]

That still seems bright to me.