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

Content inset is off on the NavigationMapView #2353

Closed
JThramer opened this issue Mar 18, 2020 · 11 comments
Closed

Content inset is off on the NavigationMapView #2353

JThramer opened this issue Mar 18, 2020 · 11 comments
Labels
bug Something isn’t working P0 Critical priority.

Comments

@JThramer
Copy link
Contributor

Simulator Screen Shot - iPhone 11 Pro Max - 2020-03-17 at 23 52 57

Content inset is off in phones with nonzero safe-area. This causes logo and info button to be offset high by the height of the safe-area. Regression suspected to have been introduced by Map-SDK 5.6.0. Recommend bisect to confirm.

Mapbox Navigation SDK version: master

/cc @mapbox/navigation-ios

@JThramer JThramer added the bug Something isn’t working label Mar 18, 2020
@JThramer JThramer added this to the v1.0.0 milestone Mar 18, 2020
@JThramer JThramer added P0 Critical priority. Injection Work labels Mar 18, 2020
@JThramer
Copy link
Contributor Author

Issue repros back to 0.26.0, suspect that issue may be related to an iOS update, not a map update.

@JThramer
Copy link
Contributor Author

Tested back to iPhone X, 11.0, issue still reproduces.

@1ec5
Copy link
Contributor

1ec5 commented Mar 25, 2020

Regression suspected to have been introduced by Map-SDK 5.6.0. Recommend bisect to confirm.
Issue repros back to 0.26.0

If this issue was introduced by the map SDK, it would’ve been in a much older version of the map SDK to have reproduced back to navigation SDK v0.26.0.

@Udumft
Copy link
Contributor

Udumft commented Mar 30, 2020

Some thoughts that might be helpful: debugging view hierarchy shows that both logo image and info button have only width and height constraints, but no position alignment. Such constraints are configured inside Maps SDK via next code:

https://github.com/mapbox/mapbox-gl-native-ios/blob/171ff9484a48c39151a4bb793911889996f54156/platform/ios/src/MGLMapView.mm#L879-L896

At the same time debug does not show any broken constraints messages. I think it is a good place to start searching.
Tested on sample app with bare Maps SDK MGLMapView

@JThramer
Copy link
Contributor Author

Interesting. Considering that these are both postitional constraints, it makes me wonder if something is broken in Maps. cc @1ec5

@Udumft
Copy link
Contributor

Udumft commented Apr 1, 2020

I noticed that compiling Maps SDK via XCode v11.4 results in constraints above to exist, while same source code compiled with XCode 11.3.1 "skips" position constraints for some reason. So at this point yes, I think the issue is somewhere inside Maps SDK, not Navigation SDK on that matter.
Unfortunately we cannot just move to XCode 11.4 because one of the 3rd party dependencies fails to compile on that version: SnappyShrimp. It seems that issue is related with Apple updating some of the testing libs and thus creating errors with linking them by default. I managed to make it compile and submitted an issue and a PR for resolving this inside SnappyShrimp.

With everything compiled under XCode 11.4 I can confirm that constraints exist, but their values still seems not correct (vertical offsets are too big):
Screen Shot 2020-04-01 at 14 10 43
I think it would be good to ask someone from Maps SDK team to take a look on that to understand where we should search the issue.

Anyway, we need to be able to compile our SDKs under XCode 11.4, and until SnapyShrimp incorporates the compilation fix, we can do next workaround:

  1. > carthage update --no-build
  2. Add custom "Other linker flag" -weak-lXCTestSwiftSupport and a custom "Library Search Path" $(PLATFORM_DIR)/Developer/usr/lib to checked out SnappyShrimp project settings (solution based on this thread).
  3. > carthage build --platform ios

@1ec5 1ec5 modified the milestones: v1.0.0, v0.x next Apr 14, 2020
@1ec5
Copy link
Contributor

1ec5 commented Apr 15, 2020

/cc @mapbox/maps-ios

@captainbarbosa
Copy link
Contributor

captainbarbosa commented Apr 15, 2020

I spent some time on this today, but couldn't find any solid clues. Here is what know as of now:

  1. I can reproduce the issue in the example nav app on Xcode 11.3 on an 11 Pro Max. And, the weirder thing is, the Xcode 11.3 version doesn't show those extra constraints (Safe Area.bottom = self.bottom +122) that @Udumft saw in their screenshot above:

Screen Shot 2020-04-15 at 2 41 00 PM

  1. I don’t see any constraints coming from the Navigation SDK's BottomBannerView that could be influencing this position (that I know of) … but I probably don’t know enough how the bottom banner’s height is determined to be sure.

  2. I also looked at this in Reveal.app to see if we could um, reveal, more information, didn't find anything helpful.

So somehow, the logo/info icon are getting extra spacing added between the banner and their views, but I have no clue where this extra spacing is coming from.

Some things that could be helpful for the Maps SDK team to continue looking into:

  • It would be helpful, I think, to know how the Navigation SDK calculates the height of the bottom banner view, so we could try to reproduce in a pure Maps SDK application.
  • Does the Navigation SDK make use of MGLMapView's contentInset property? As far as I could tell, it was always at zero for me in the Example app while displaying the navigation view. But maybe I missed something?

@1ec5 1ec5 removed this from the v0.40.0 milestone Apr 24, 2020
@1ec5
Copy link
Contributor

1ec5 commented Apr 30, 2020

Probably related to the failure in mapbox/mapbox-gl-native-ios#167.

@1ec5
Copy link
Contributor

1ec5 commented Apr 30, 2020

Does the Navigation SDK make use of MGLMapView's contentInset property? As far as I could tell, it was always at zero for me in the Example app while displaying the navigation view. But maybe I missed something?

Yes:

mapView.contentInset = contentInset(forOverviewing: false)
func contentInset(forOverviewing overviewing: Bool) -> UIEdgeInsets {
let instructionBannerHeight = topBannerContainerView.bounds.height
let bottomBannerHeight = bottomBannerContainerView.bounds.height
// Inset by the safe area to avoid notches.
var insets = mapView.safeArea
insets.top += instructionBannerHeight
insets.bottom += bottomBannerHeight
if overviewing {
insets += NavigationMapView.courseViewMinimumInsets
let routeLineWidths = MBRouteLineWidthByZoomLevel.compactMap { $0.value.constantValue as? Int }
insets += UIEdgeInsets(floatLiteral: Double(routeLineWidths.max() ?? 0))
} else if mapView.tracksUserCourse {
// Puck position calculation - position it just above the bottom of the content area.
var contentFrame = mapView.bounds.inset(by: insets)
// Avoid letting the puck go partially off-screen, and add a comfortable padding beyond that.
let courseViewBounds = mapView.userCourseView.bounds
// If it is not possible to position it right above the content area, center it at the remaining space.
contentFrame = contentFrame.insetBy(dx: min(NavigationMapView.courseViewMinimumInsets.left + courseViewBounds.width / 2.0, contentFrame.width / 2.0),
dy: min(NavigationMapView.courseViewMinimumInsets.top + courseViewBounds.height / 2.0, contentFrame.height / 2.0))
assert(!contentFrame.isInfinite)
let y = contentFrame.maxY
let height = mapView.bounds.height
insets.top = height - insets.bottom - 2 * (height - insets.bottom - y)
}
return insets
}

But it probably gets overridden as soon as the puck is laid out, due to mapbox/mapbox-gl-native#15233.

@1ec5
Copy link
Contributor

1ec5 commented Oct 11, 2021

This is specific to navigation SDK v1.x with map SDK v6.x; it’s likely that the relevant code has been rewritten at this point, but feel free to reopen if the issue persists.

@1ec5 1ec5 closed this as completed Oct 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn’t working P0 Critical priority.
Projects
None yet
Development

No branches or pull requests

5 participants