Use DisplayLink to prevent crashes in willResignActive

[reezer]

We've experienced frequent crashes while tracking routes, when the app goes into background, right after willResignActive is called.

This change instantiates DisplayLink to control rendering, preventing the crash. It is based off what Mapbox does, but more minimal to not require copying various 6.x changes and only touch the relevant parts, mostly what's required to instantiate and use CADisplayLink.

After this change no more crashes have been observed.

[petr-pokorny-1]

Looking good, thanks for contributing!

[juhieta]

Hi @reezer and thanks for the PR, this is important fix and these seem to be similar crashes as noted in issue #70.

There seems to be some problem with this PR when tested with device (in my case iPhone 12). In simulator I didn't see this issue:

If you run the SDK demo app in device, put the app to background (don't close it) and bring it back to foreground, moving the map doesn't work anymore. I managed to fix this by taking few more changes from the related Mapbox PR. I'll try to make change request to your branch.

[juhieta]

@reezer I created the following PR to your branch: reezer#1

It includes few more changes from the Mapbox PR and seems to work correctly in my testing.

[reezer]

@juhieta I will look into this as soon as I have time. In fact we also had problems when the app come back into foreground and I have some local changes for displayLink as well.

Also I locally tried to remove the Snapshotting-Code, since that also seems to be another source of crashes and removal was at least discussed in mapbox bugs as well. So far this seems to be a way of working around those issues.

[juhieta]

Thanks for update @reezer. Yes the snapshotting seems to cause crashing if it's done when app is in background. I guess the changes in the Mapbox PR 432 were aimed to remove those crashes too as the snapshotting is then only done when app is active. So we should be able to keep the snapshotting with those code changes. Of course we could in any case consider if snapshotting is necessary overall.

[reezer]

Had a chance to look at it. Thanks.

Yes, I looked at the changes in Mabox PR 432, but crashes still seems to happen to some people with recent versions of mapbox, which is why I removed the snapshots locally. So far this seems to work, but it still requires more testing.

[juhieta]

Thanks, and yes I'll also test this more now. I did actually remove the snapshots locally too as they caused some unwanted side effect for our case (if base map is changed to something very different, you can get a snapshot of completely different map when bringing to foreground if the snapshot delay is long enough). Maybe in future the snapshots should be made optional at least (and possibly disabled by default).

[paulsUsername]

Any ideas when this could be baked into a release?

[juhieta]

In my tests I have not seen crashes with this PR, so I would suggest this to be merged. However, as this limits the opportunities for the app to take map view snapshots, this can cause the map to "jump" more often when brought to foreground. Therefore, another PR to disable the snapshotting completely could be considered after this is merged.

@roblabs have you had time for reviewing this?

[reezer]

After testing from many people on the phone I can confirm that there are no more crashes with these changes.

[petr-pokorny-1]

Thanks guys!

[juhieta]

However, as this limits the opportunities for the app to take map view snapshots, this can cause the map to "jump" more often when brought to foreground. Therefore, another PR to disable the snapshotting completely could be considered after this is merged.

I now created new PR #83 as a suggestion to disable map snapshotting for iOS.

[petr-pokorny-1]

It looks like this change broken ios UI tests - platform/ios/platform/ios/Integration Tests/MGLMapViewPendingBlockTests.m, testSetCenterCoordinatePauseRendering because it is using MGLMapView pauseRendering selector which was removed.

Exception NSException * "-[MGLMapView pauseRendering:]: unrecognized selector sent to instance 0x7fe5da035200" 0x0000600003ea3120

[juhieta]

Thanks @petr-pokorny-1, I can have a look!

[juhieta]

This should fix it @petr-pokorny-1: #88

[juhieta]

Btw the ios-ci -builds for PRs seem to fail instantly with error:

No runner matching the specified labels was found: macos-11.0

For example, https://github.com/maplibre/maplibre-gl-native/actions/runs/860113172

[petr-pokorny-1]

Btw the ios-ci -builds for PRs seem to fail instantly with error:

No runner matching the specified labels was found: macos-11.0

For example, https://github.com/maplibre/maplibre-gl-native/actions/runs/860113172

I see - 11.0 should be available but it isnt for some reason. I have set it to macos-10.15 and launched CI build again