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

Remove workarounds adding content inset when setting map camera #2145

Closed
1ec5 opened this issue Jun 1, 2019 · 8 comments · Fixed by #2211
Closed

Remove workarounds adding content inset when setting map camera #2145

1ec5 opened this issue Jun 1, 2019 · 8 comments · Fixed by #2211
Labels
bug Something isn’t working op-ex Refactoring, Tech Debt or any other operational excellence work.

Comments

@1ec5
Copy link
Contributor

1ec5 commented Jun 1, 2019

Once mapbox/mapbox-gl-native#14813 lands and we upgrade to map SDK v5.1.0, NavigationMapView.userAnchorPoint and possibly other methods need to be updated to stop adding the map view’s contentInset to the edge padding when setting the map’s camera.

/ref #1651 (comment)
/cc @mapbox/navigation-ios @fabian-guerra @d-prukop

@1ec5 1ec5 added op-ex Refactoring, Tech Debt or any other operational excellence work. blocked: upstream labels Jun 1, 2019
@1ec5 1ec5 added this to the v0.35.0 milestone Jun 1, 2019
@1ec5
Copy link
Contributor Author

1ec5 commented Jun 20, 2019

In rudimentary testing, NavigationMapView.userAnchorPoint and NavigationMapView.updateCourseTracking(location:camera:animated:) seem to be behaving fine after upgrading to map SDK v5.1.0. That’s a bit of a surprising outcome, given that there was obviously a bug in the map SDK previously and it got fixed in v5.1.0.

@1ec5
Copy link
Contributor Author

1ec5 commented Jun 27, 2019

Fixed in #2134.

@1ec5 1ec5 closed this as completed Jun 27, 2019
@1ec5
Copy link
Contributor Author

1ec5 commented Jul 17, 2019

I must not’ve been paying close enough attention, but NavigationMapView.updateCourseTracking(location:camera:animated:) is not quite correct. Upgrading from map SDK v5.0.0 to v5.1.0 causes the map to pivot around the center-bottom of the map instead of the puck’s center.

@1ec5 1ec5 reopened this Jul 17, 2019
@1ec5 1ec5 modified the milestones: v0.35.0, v0.36.0 Jul 17, 2019
@1ec5 1ec5 added the bug Something isn’t working label Jul 17, 2019
@1ec5
Copy link
Contributor Author

1ec5 commented Jul 26, 2019

Possibly related to mapbox/mapbox-gl-native#15232.

@1ec5
Copy link
Contributor Author

1ec5 commented Jul 26, 2019

I must not’ve been paying close enough attention, but NavigationMapView.updateCourseTracking(location:camera:animated:) is not quite correct. Upgrading from map SDK v5.0.0 to v5.1.0 causes the map to pivot around the center-bottom of the map instead of the puck’s center.

This is probably mapbox/mapbox-gl-native#15233.

@1ec5 1ec5 removed this from the v0.36.0 milestone Jul 27, 2019
astojilj added a commit that referenced this issue Aug 12, 2019
Decouple dependency content insets -> anchor point -> padding/content insets broken after mapbox/mapbox-gl-native#14664 introduced animated interpolation for padding change.

Remove the need to specify center for puck view - use the approach where content inset is keeping it centered.

Take safeArea into account when calculating contentInsets.

Addresses: mapbox/mapbox-gl-native#15232, mapbox/mapbox-gl-native#15233

Related to: #2165,

Fixes: #2145
@astojilj
Copy link
Contributor

Once mapbox/mapbox-gl-native#14813 lands and we upgrade to map SDK v5.1.0, NavigationMapView.userAnchorPoint and possibly other methods need to be updated to stop adding the map view’s contentInset to the edge padding when setting the map’s camera.

/ref #1651 (comment)
/cc @mapbox/navigation-ios @fabian-guerra @d-prukop

@1ec5, @fabian-guerra

Since mapbox/mapbox-gl-native@4026451 is already in 5.1, it requires described update.

As padding is added to content inset, current anchor point calculation is out of sync with mapbox/mapbox-gl-native#14813 and effectively adds content insets twice: once in anchor point calculation and once in setCamera - defined in mapbox/mapbox-gl-native#12818. This is a cause of having the puck jumping down - working on fix for this in #2211.

astojilj added a commit that referenced this issue Sep 9, 2019
Decouple dependency content insets -> anchor point -> padding/content insets broken after mapbox/mapbox-gl-native#14664 introduced animated interpolation for padding change.

Remove the need to specify center for puck view - use the approach where content inset is keeping it centered.

Take safeArea into account when calculating contentInsets.

Addresses: mapbox/mapbox-gl-native#15232, mapbox/mapbox-gl-native#15233

Related to: #2165,

Fixes: #2145
@noway
Copy link

noway commented Sep 26, 2019

In my experience, the puck is detaches from the route while jumping up, although when it's down it's too low as well, so the puck sort of either being too low (which is not too bad cause I can offset that myself I think) or too high and detached.

Either way, is there any immediate workaround for puck jumping up/down? I'm using "mapbox/mapbox-navigation-ios" "v0.37.0" and "https://www.mapbox.com/ios-sdk/Mapbox-iOS-SDK.json" "5.4.0" via Cathage. We were using mapbox navagtion 0.34 with mapbox ios sdk 5.0.0, but 5.0.0 fails to compile with Swift 5.1 now (which is forced with Xcode 11.0). Having to update to 0.37, this is a critical bug which blocks any releases we can do to our app. I set my bound using:

self->_navigationViewController.view.frame = CGRectMake(0, 0, self.bounds.size.width, self.bounds.size.height);

Update: workaround to this is to adopt this unmerged PR: #2211 (comment)

1ec5 pushed a commit that referenced this issue Oct 2, 2019
Decouple dependency content insets -> anchor point -> padding/content insets broken after mapbox/mapbox-gl-native#14664 introduced animated interpolation for padding change.

Remove the need to specify center for puck view - use the approach where content inset is keeping it centered.

Take safeArea into account when calculating contentInsets.

Addresses: mapbox/mapbox-gl-native#15232, mapbox/mapbox-gl-native#15233

Related to: #2165,

Fixes: #2145
@1ec5
Copy link
Contributor Author

1ec5 commented Oct 2, 2019

Once #2211 lands, #2248 will supersede the original problem tracked in this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn’t working op-ex Refactoring, Tech Debt or any other operational excellence work.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants