Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[core] center still offset incorrectly with content insets #15344

Closed
chloekraw opened this issue Aug 9, 2019 · 6 comments
Closed

[core] center still offset incorrectly with content insets #15344

chloekraw opened this issue Aug 9, 2019 · 6 comments
Assignees
Labels
bug Core The cross-platform C++ core, aka mbgl

Comments

@chloekraw
Copy link
Contributor

chloekraw commented Aug 9, 2019

Customer reported a bug with the center being offset incorrectly when setting the camera with the release-oolong branch. Likely related to #15106, maybe #14991.

We expected #15130 to resolve this regression but this fix has already been backported to oolong in #15176.

Customer suspects it's from this part of their code:

  mbgl::CameraOptions cameraOptions;
  cameraOptions.center = MGLLatLngFromLocationCoordinate2D(centerCoordinate);
  cameraOptions.padding = MGLEdgeInsetsFromNSEdgeInsets(insets);
  cameraOptions.zoom = zoomLevel;
  if (direction >= 0) {
    cameraOptions.bearing = direction;
  }

Problems reported with the map:

  • try to set the center of the map to the current location and notice the current location is slightly off-center
  • tell the camera to position on the map bounds of a specific city. map bounds have the same offset issue
  • center the map to current location. there's the same slightly off-center offset
  • try again to center the map on current location. when it properly positions the map to center, the offset bug goes away.
@chloekraw chloekraw added bug Core The cross-platform C++ core, aka mbgl beta blocker Blocks the next beta release labels Aug 9, 2019
@astojilj
Copy link
Contributor

astojilj commented Aug 9, 2019

@chloekraw

Based on the problem description, this code behaves as expected: padding moves all content, and center (and perspective center if pitch is used) to content area defined by padding.
Padding is also persistent property of camera - once it is set, it is active until other, or zero, padding is set.

#15130 was about offset in selecting annotations, it didn't affect how content is rendered (rendering offset).

Padding on iOS is used always because of top bar - so that the center is in visible area. Then, if there are other UI elements overlaid on sides, e.g. turn by turn navigation, padding is used to center the content to non occluded area of screen.

What might be causing issue is that, insets are defined in logical screen coordinates, as opposite to using physical - the customer in this case might have just the vale that is too high. Without knowing the value set in code above, it is hard to tell.

@chloekraw
Copy link
Contributor Author

chloekraw commented Aug 9, 2019

Thanks @astojilj. I’ll email them and make sure they see it.

Let’s say that they had this same code and the same implementation back when they were on release-mojito. If they notice a difference in behavior now, why might that be?

@astojilj
Copy link
Contributor

astojilj commented Aug 9, 2019

@chloekraw

#14664 introduced persistent nature of padding - earlier, set padding was reset to 0 after the transition and new center was calculated (padding was baked into center coordinate). As padding defines content area, asymmetric viewport center and vanishing point, it had to be set as persistent.

After #14664, center coordinate and padding are separate, and if there is a need to reset padding, there should be an explicit call from client code to do it. Note that padding, as other camera properties are, is animated when using easeTo/flyTo.

@chloekraw
Copy link
Contributor Author

Thanks @astojilj. Closing this ticket, will re-open if it's needed.

@chloekraw chloekraw removed the beta blocker Blocks the next beta release label Aug 13, 2019
@fabian-guerra
Copy link
Contributor

I'm re-opening this issue because as of 5.0.0 setting MGLMapView.contentInset effectively moved MGLMapView.centerCoordinate accordingly. Stated in our documentation:

When the value of this property is equal to `UIEdgeInsetsZero`, viewport
properties such as `centerCoordinate` assume a viewport that matches the map
view’s frame. Otherwise, those properties are inset, excluding part of the
frame from the viewport. For instance, if the only the top edge is inset, the
map center is effectively shifted downward.

Starting 5.1.0 this is no longer true making it a regression.

@astojilj
Copy link
Contributor

astojilj commented Sep 5, 2019

When the value of this property is equal to UIEdgeInsetsZero, viewport
properties such as centerCoordinate assume a viewport that matches the map
view’s frame. Otherwise, those properties are inset, excluding part of the
frame from the viewport. For instance, if the only the top edge is inset, the
map center is effectively shifted downward.

@fabian-guerra @chloekraw @1ec5 Let's move this to another issue and not reopen this one, please.

@fabian-guerra that content above doesnt mean that value in centerCoordinate gets altered when you change contentInset. It is just rendered on different place.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Core The cross-platform C++ core, aka mbgl
Projects
None yet
Development

No branches or pull requests

3 participants