Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Padding error on iOS: Unable to calculate appropriate zoom level for bounds #1032

Closed
naftalibeder opened this issue Sep 17, 2020 · 13 comments · Fixed by #1057
Closed

Padding error on iOS: Unable to calculate appropriate zoom level for bounds #1032

naftalibeder opened this issue Sep 17, 2020 · 13 comments · Fixed by #1057

Comments

@naftalibeder
Copy link
Collaborator

Description

There is a bug in Mapbox that causes an error on iOS when a cumulative padding of 64 or greater is applied to the camera. In addition, padding does not have any effect on the way bounds are centered on the screen on iOS.

Steps to reproduce

  1. Clone this repo.
  2. Configure and run it as described in readme.md.

Expected behavior

The padding values should influence the way the bounds are centered in the map view, and not cause crashes.

Other information

This issue is referenced in #841 and #956 as well.

Versions

  • Platform: iOS
  • Device: iPhone 11, iPhone SE, iPhone 6S
  • Emulator/ Simulator: both
  • OS: iOS 13.7, iOS 14.0
  • react-native-mapbox-gl Version: 8.1.0-rc.4
  • React Native Version: 0.63.2
@naftalibeder
Copy link
Collaborator Author

@mfazekas @ferdicus I was wondering if there was any way of moving this forward, as it has prevented us from upgrading to the newest version.

It's a common action (adding padding to the map), and it's reproducible on the most recent versions of React Native and Mapbox in the simplest example I could put together.

Is there any more information I can provide, or another way of using this feature that would avoid the problem? Any discussion on this issue would be very helpful. Thanks!

@ferdicus
Copy link
Member

Hey @naftalibeder,
it's not like there is an unwillingness to investigate this issue.
However we're basically some people working on/ off in our spare time on this repo.
It's simply a time issue unfortunately, there is too much to do and too little people to assign it to 😞.

@mfazekas
Copy link
Contributor

@naftalibeder Does going back to older Mapbox version (say 5.6.0) fixes the issue?!

https://github.com/react-native-mapbox-gl/maps/issues/841#issuecomment-651727563

@naftalibeder
Copy link
Collaborator Author

@mfazekas Yes, we've been locked onto version 7.0.8 for quite some time, as it's the highest version we're able to use without problems.

@mfazekas
Copy link
Contributor

@naftalibeder plese see the link in my comment, above it describes how to use older (5.6.0) Mapbox IOS libs with newer ReactNAtiveMapboxGL versions (8.1.0.rc2 or later).

The padding seems to be introduced in 5.7.0 of iOS libraries and it seems to be hard/impossible to work around in React Native wrappers. I'll need to look this exact bug though

@naftalibeder
Copy link
Collaborator Author

naftalibeder commented Sep 30, 2020

@mfazekas Thanks for pointing that out - I think I remember trying that solution at one point and it not working, but I will try to reproduce it on the minimal example project I created for this issue. Will update!

@naftalibeder
Copy link
Collaborator Author

@mfazekas Unfortunately I'm not even able to complete the installation with that modification - if I follow the recommendations, I oscillate between these two errors:

[!] CocoaPods could not find compatible versions for pod "@react-native-mapbox-gl-mapbox-static":
  In snapshot (Podfile.lock):
    @react-native-mapbox-gl-mapbox-static (= 5.8.0, ~> 5.8)

  In Podfile:
    react-native-mapbox-gl (from `../node_modules/@react-native-mapbox-gl/maps`) was resolved to 8.1.0-rc.4, which depends on
      @react-native-mapbox-gl-mapbox-static (~> 5.6.0)


You have either:
 * changed the constraints of dependency `@react-native-mapbox-gl-mapbox-static` inside your development pod `react-native-mapbox-gl`.
   You should run `pod update @react-native-mapbox-gl-mapbox-static` to apply changes you've made.

and

[!] CocoaPods could not find compatible versions for pod "Mapbox-iOS-SDK":
  In snapshot (Podfile.lock):
    Mapbox-iOS-SDK (= 5.9.0, ~> 5.8)

  In Podfile:
    react-native-mapbox-gl (from `../node_modules/@react-native-mapbox-gl/maps`) was resolved to 8.1.0-rc.4, which depends on
      Mapbox-iOS-SDK (~> 5.6.0)


You have either:
 * changed the constraints of dependency `Mapbox-iOS-SDK` inside your development pod `react-native-mapbox-gl`.
   You should run `pod update Mapbox-iOS-SDK` to apply changes you've made.

Feel free to try the installation yourself on the linked branch: https://github.com/TruckMap/mapbox-example-padding/tree/downgrade-mapbox-gl

@mfazekas
Copy link
Contributor

mfazekas commented Oct 2, 2020

@naftalibeder you need to remove the Podfile.lock then pod install

@naftalibeder
Copy link
Collaborator Author

Thanks for the tip, I had thought pod deintegrate removed the lockfile but I was mistaken!

So after running the app with the older Mapbox library, I experienced the same pattern of errors, except that instead of throwing a redbox error, the app simply crashes the main thread with the error libc++abi.dylib: terminating with uncaught exception of type std::domain_error.

And for the padding configurations that do not cause a crash, the map still does not respond to the padding setting.

@mfazekas
Copy link
Contributor

mfazekas commented Oct 5, 2020

I've submitted a fix at, you should be able to test it:

npm install --save react-native-mapbox-gl/maps#mfazekas/fix-boudnsPadding-ios

@naftalibeder
Copy link
Collaborator Author

naftalibeder commented Oct 5, 2020

Thank you so much, this is a big improvement. It has fixed the crash, but now I am finding a different set of issues. Specifically:

  1. The padding seems to affect the attribution and info icons, moving them away from the edge of the map by the exact padding amount. I believe they should stay in place?
  2. Rather than the map contents centering inside of the area bounded by the padding, the contents are simply offset by the padding amount. For instance, when paddingBottom is set to half of the map height, that should mean the 'active' area is the top half of the map, and therefore a centered item moves to be offset from the top of the screen by 25%. Instead, the centered item simply rests at the exact top edge of the screen.
  3. When the padding equals or exceeds the dimensions of the map, the library throws an error. This is clearly intended, but I was wondering if it was possible to silently max out the padding instead (as in min(cumulativePadding, mapHeight)). This would probably be a simple fix, but I'm not sure where to do it.

You can see this in action here, and I've updated the repo.

@mfazekas
Copy link
Contributor

mfazekas commented Oct 6, 2020

@naftalibeder i've updated the PR so that it uses edgePadding instead of contentInset. This should address most issues noted here.

Used this code as test app:
https://github.com/react-native-mapbox-gl/maps/pull/1057#issuecomment-704475765

@naftalibeder
Copy link
Collaborator Author

@mfazekas Left a note on the PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants