Mapview.content inset are not working as expected on 5.5.0

[pappalar]

Hello, after the changes to the contentInset in 5.5.0 I don't understand how the content inset is used.

Example:

The mapview has a frame width of 300
I set contentInset.left to 100
I set contentInset.right to 50

I would expect that if I zoom to a square MGLRegionBound I would see it inset by 100 on the left and 50 on the right.

However this is not happening.

The mapView FRAME is shifted by those values, so the region bound is not respected.

I see the box 100 px from the left but only some pixel from the right.

Why is that?

It looks to me that the left inset is applied to shift the entire mapview frame and then the region bound is drawn.

Another example is this:

set left to 300px
right to 0px

I would expect to see the region attached to the border on the right, starting 300 px from the left.
Instead, the region is totally out of the screen.

Could you explain me if I understood the inset wrong or this is a bug due to mapbox/mapbox-gl-native#15584 ? thanks

[pappalar]

I provide here an example screen:

Happening:

I would expect both my shape and its bounding box to be inside the contentInset of the map.
Instead they are shifted. I believe this is a bug since using setCamera(withEdgeInset) instead seem to work

Expected:

What you can see here:

• A mapView attached to the superview edges
• a semi-transparent Cyan view on top of the mapview attached to the superview edges
• A red line representing the bounding box of the shape I am trying to display.
• the shape itself as a blueline

What am I doing:

1. Creating a example inset:

let insets = UIEdgeInsets(top: 200,
                            left: 1,
                            bottom: 300,
                            right: 200)

2. Setting the inset on the mapView

mapView.contentInset = insets

3. Updating the cyan view boundaries to match the inset:
   this for me is what I would expect to be the drawable area of the mapview

debugTrailing.constant = mapView.contentInset.right
debugLeading.constant = mapView.contentInset.left
debugTop.constant = mapView.contentInset.top
debugBottom.constant = mapView.contentInset.bottom

4. Display the feature on the map

let box: MGLCoordinateBounds
// blabla magic to drar the coordinate bounds on one of my sources

5. zooming to this feature I prepared by updating the camera:
   Note: I am not using a inset here, I want the map to respect the contentInset for that instead.
   This is needed for me, as sometimes I center around a coordinate instead, I don't have bounds, and I can't provide edge insets to mapView.fly(to: camera, withDuration: animated ? -1.0 : 0.0, completionHandler: nil)

mapView.setVisibleCoordinateBounds(box,
                                           edgePadding: .zero,
                                           animated: animated,
                                           completionHandler: completionHandler)

[pappalar]

Sorry for the triple post, but I would like to provide as much info as I can to be sure to get to the point:

Everything works fine if the EdgeInset are the same on top-bottom or left-right.
Then the content is inset correctly, and the mapview.center does not move.

As soon as there is a difference between top-bottom or left-right.... then the center of the map is not where i would expect it to be.

example:

let's take the frame of the Map

po mapView.frame
(0.0, 0.0, 375.0, 812.0)

Let's apply a content Inset LEFT of width/2 = 187

The result should be that the center of the map is located on x at 375 + 187
and the bounding box is displayed from the center of the screen to the right edge.

instead the bounding box starts at 375+ 187 + 187/2 and the right side of the bounding box is at
mapView.frame.width + 187

This happens also if I use a camera animation by using mapView.cameraThatFitsCoordinateBounds(bounds)

Does this mean that the inset moves the viewPort logically but does not shrink the viewport ?
Is the viewport always as big as the mapView.frame?
if yes, what is the entire point of the content inset?

EDIT:

I manage to make it work the way I expect:
In order to apply a inset of Left = 187
I need to apply a inset of left= 187 and right = 187/2

Then I get the map correctly centered between 187 and the right side of the screen

This shows me that the insets are used to move the map Frame instead of shrinking the viewport and are applied in order one after the other.

Hence this clearly look like a bug.

@julianrex I am creating issues here now,
Should I continue to create them in https://github.com/mapbox/mapbox-gl-native instead?

[pappalar]

Just verified that the content inset are working correctly when creating a camera around a point.
They are not working when the camera is created or set to fit a BoundingBox.

The reason seems to be that the camera to fit the bounding box is calculated correctly respecting the contentInset but then the camera center is calculated wrongly instead.

[RomainQuidet]

I also have the same issue, setCenterCoordinate behave correctly with a contentInset, but setVisibleCoordinates does not ...
Seems to be a bug in computation from mbgl::CameraOptions cameraOptions = self.mbglMap.cameraForLatLngs(latLngs, padding, cameraDirection);

[RomainQuidet]

diff --git a/src/mbgl/map/map.cpp b/src/mbgl/map/map.cpp
index 649e0e321e..e0f63bc0e8 100644
--- a/src/mbgl/map/map.cpp
+++ b/src/mbgl/map/map.cpp
@@ -182,13 +182,12 @@ CameraOptions cameraForLatLngs(const std::vector<LatLng>& latLngs, const Transfo
     // Calculate the bounds of the possibly rotated shape with respect to the viewport.
     ScreenCoordinate nePixel = {-INFINITY, -INFINITY};
     ScreenCoordinate swPixel = {INFINITY, INFINITY};
-    double viewportHeight = size.height;
     for (LatLng latLng : latLngs) {
         ScreenCoordinate pixel = transform.latLngToScreenCoordinate(latLng);
         swPixel.x = std::min(swPixel.x, pixel.x);
         nePixel.x = std::max(nePixel.x, pixel.x);
-        swPixel.y = std::min(swPixel.y, viewportHeight - pixel.y);
-        nePixel.y = std::max(nePixel.y, viewportHeight - pixel.y);
+        swPixel.y = std::min(swPixel.y, pixel.y);
+        nePixel.y = std::max(nePixel.y, pixel.y);
     }
     double width = nePixel.x - swPixel.x;
     double height = nePixel.y - swPixel.y;
@@ -212,21 +211,9 @@ CameraOptions cameraForLatLngs(const std::vector<LatLng>& latLngs, const Transfo
 
     // Calculate the center point of a virtual bounds that is extended in all directions by padding.
     ScreenCoordinate centerPixel = nePixel + swPixel;
-    ScreenCoordinate paddedNEPixel = {
-        padding.right() / minScale,
-        padding.top() / minScale,
-    };
-    ScreenCoordinate paddedSWPixel = {
-        padding.left() / minScale,
-        padding.bottom() / minScale,
-    };
-    centerPixel = centerPixel + paddedNEPixel - paddedSWPixel;
     centerPixel /= 2.0;
 
-    // CameraOptions origin is at the top-left corner.
-    centerPixel.y = viewportHeight - centerPixel.y;
-
-    return CameraOptions().withCenter(transform.screenCoordinateToLatLng(centerPixel)).withZoom(zoom);
+    return CameraOptions().withCenter(transform.screenCoordinateToLatLng(centerPixel)).withPadding(padding).withZoom(zoom);
 }
 
 CameraOptions Map::cameraForLatLngs(const std::vector<LatLng>& latLngs, const EdgeInsets& padding, optional<double> bearing, optional<double> pitch) const {

[RomainQuidet]

Seems to me that CameraOptions cameraForLatLngs(const std::vector<LatLng>& latLngs, const Transform& transform, const EdgeInsets& padding) method in map.cpp is doing wrong geomatic computation.
By simplifying it, I manage to get a correct behavior with setVisibleCoordinates methods group.

[RomainQuidet]

So the issue is located in mbgl-core, which is on another repo now... What is the process here for a pull request ?

[pappalar]

@RomainQuidet I would say open a PR to the core repo and link this issue and say it's fixing it.

I guess tough the new change should not break the SDK tests here.
Or maybe it should break it to show the behaviour is wrong ;)

However If it is a core issue I guess similar problems should popup in the android and web sdks.

I wonder if the iOS layer on top of is also not reading the data wrong comign from the core

[RomainQuidet]

PR sent, we'll see what's going on ;) My tests show a correct behavior of our app with this patch.

[pappalar]

@RomainQuidet @astojilj I see the PR was merged!

When we can expect this in a SDK Release?

Thanks

[RomainQuidet]

yes, really cool they accepted the correction. It's now on master branch of mapbox-gl-native submodule, it needs to be taken by iOS devs on this project.
I don't know about the timing ... 5.6.0 is in beta state, I don't know if they are allowed to change it now.

[julianrex]

This is awesome @RomainQuidet @racer1988 - thanks so much for submitting that PR, and thanks @astojilj for landing this in gl-native!

When we can expect this in a SDK Release?

I don't know about the timing ... 5.6.0 is in beta state, I don't know if they are allowed to change it now.

This will be in 5.7.0 which is due early February IIRC.

[chloekraw]

Thanks for your help with this investigation @RomainQuidet @racer1988! The fix will also be available in pre-releases in January for you to test and confirm the issue is resolved. Expect an alpha release the 2nd week of January and a beta the 4th week of January. cc @zugaldia

[mfazekas]

We're now facing a regression that might be caused by this fix when doing flyTo with edgePadding on ios:

See #198

I think that the cause is that when converting CameraOptions to MGLMapCamera edgePadding is ignored:

mapbox-gl-native-ios/platform/ios/src/MGLMapView.mm

Lines 4011 to 4025 in a139216

	- (MGLMapCamera *)cameraForCameraOptions:(const mbgl::CameraOptions &)cameraOptions
	{
	if (!_mbglMap)
	{
	return self.residualCamera;
	}
	mbgl::CameraOptions mapCamera = self.mbglMap.getCameraOptions();
	CLLocationCoordinate2D centerCoordinate = MGLLocationCoordinate2DFromLatLng(cameraOptions.center ? *cameraOptions.center : *mapCamera.center);
	double zoomLevel = cameraOptions.zoom ? *cameraOptions.zoom : self.zoomLevel;
	CLLocationDirection direction = cameraOptions.bearing ? mbgl::util::wrap(*cameraOptions.bearing, 0., 360.) : self.direction;
	CGFloat pitch = cameraOptions.pitch ? *cameraOptions.pitch : *mapCamera.pitch;
	CLLocationDistance altitude = MGLAltitudeForZoomLevel(zoomLevel, pitch, centerCoordinate.latitude, self.frame.size);
	return [MGLMapCamera cameraLookingAtCenterCoordinate:centerCoordinate altitude:altitude pitch:pitch heading:direction];
	}

Before the fix in this pr edgePadding was not used in the C++ CameraOptions but center was adjusted accordingly, this was converted to MGLMapCamera just fine. But now that CameraOptions uses padding - CameraOptions to MGLMapCamera no longer works correctly when edge padding was used.

[zugaldia]

@mfazekas thanks for the report. Could you open a separate ticket to track this potential regression?

[mfazekas]

@zugaldia FYI there is issue #198 which is the regression.

[astojilj]

@RomainQuidet do you have a proposal how to reconcile this with #198?

[RomainQuidet]

Hi @astojilj I'm sorry I don't work on my customer's project involving mapbox anymore. Anyway if I have some free time I'll check both issues and the code.

[astojilj]

Hi @astojilj I'm sorry I don't work on my customer's project involving mapbox anymore. Anyway if I have some free time I'll check both issues and the code.

No worries, we'll fix it. Thanks again.