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

MGLMapCamera doesn't respect edgePadding on SDK 5.6.0+ #198

Open
maxmamis opened this issue Mar 4, 2020 · 9 comments · May be fixed by #323
Open

MGLMapCamera doesn't respect edgePadding on SDK 5.6.0+ #198

maxmamis opened this issue Mar 4, 2020 · 9 comments · May be fixed by #323

Comments

@maxmamis
Copy link

maxmamis commented Mar 4, 2020

Flying to an MGLMapCamera does not respect insets as expected on SDK version 5.6.0+.

Minimal reproducible code:

let camera = mapView.cameraThatFitsCoordinateBounds(bound, edgePadding: UIEdgeInsets(top: 200, left: 0, bottom: 0, right: 0))
mapView.fly(to: camera, completionHandler: nil)

Expected behavior: zooms to the specified location in the bottom third of the screen. (This works properly on SDK version 5.5.0)
Observed behavior: zooms to the specified location dead center on the screen.

Tested using an iPhone XS running iOS 13.3.1

Screenshots:

5.7.0:
IMG_AB7EA258F9C0-1

5.5.0:
IMG_D0354889178D-1

@chloekraw chloekraw transferred this issue from mapbox/mapbox-gl-native Mar 6, 2020
@mfazekas
Copy link

mfazekas commented Apr 28, 2020

I can confirm that there is an issue with asymmetric edgePadding.

In my testing 5.6.0 works fine too, while 5.7.0, 5.8.0 and 5.9.0.alpha1 is not.
Also users reported from android too. (https://github.com/react-native-mapbox-gl/maps/issues/841)

I've attached a ViewController that reproduces the issue.

In 5.7.0..5.9.0, i get this for pressing up and/or down:
edgePadding:UIEdgeInsets(top: 200.0, left: 0, bottom: 0.0, right: 0)) or edgePadding:UIEdgeInsets(top: 0.0, left: 0, bottom: 200.0, right: 0))
kép

In 5.6.0 i get this for up: edgePadding:UIEdgeInsets(top: 200.0, left: 0, bottom: 0.0, right: 0))
kép

and this for down :edgePadding:UIEdgeInsets(top:0.0, left: 0, bottom: 200.0, right: 0))
kép

import UIKit
import Mapbox

class ViewController: UIViewController, MGLMapViewDelegate {
    var mapView : MGLMapView!
    
    override func viewDidLoad() {
        super.viewDidLoad()
        createUI()
    }
    
    func createUI() {
        let url = URL(string: "mapbox://styles/mapbox/streets-v11")
        mapView = MGLMapView(frame: view.bounds, styleURL: url)
        mapView.delegate = self
        mapView.setCenter(CLLocationCoordinate2D(latitude: 59.31, longitude: 18.06), zoomLevel: 9, animated: false)
        self.view.addSubview(mapView)
        mapView.translatesAutoresizingMaskIntoConstraints = false
        NSLayoutConstraint.activate([
            mapView.leftAnchor.constraint(equalTo: view.leftAnchor),
            mapView.rightAnchor.constraint(equalTo: view.rightAnchor),
            mapView.topAnchor.constraint(equalTo: view.topAnchor),
            mapView.bottomAnchor.constraint(equalTo: view.bottomAnchor)
        ])
        
        let button1 = UIButton()
        button1.setTitle("up", for: .normal)
        button1.setTitleColor(.systemBlue, for: .normal)
        button1.backgroundColor = .systemBackground
        button1.addTarget(self, action: #selector(up), for: .touchUpInside)

    
        let button2 = UIButton()
        button2.setTitle("down", for: .normal)
        button2.setTitleColor(.systemBlue, for: .normal)
        button2.backgroundColor = .systemBackground
        button2.addTarget(self, action: #selector(down), for: .touchUpInside)
        
        let buttons = UIStackView(arrangedSubviews: [button1, button2])
        buttons.translatesAutoresizingMaskIntoConstraints = false
        buttons.spacing = 32.0
        self.view.addSubview(buttons)
        NSLayoutConstraint.activate([
            buttons.bottomAnchor.constraint(equalTo: self.view.layoutMarginsGuide.bottomAnchor),
            buttons.centerXAnchor.constraint(equalTo: self.view.centerXAnchor)
        ])
    }
    
    func bounds() -> MGLCoordinateBounds {
        let diff = 0.1
        let center = CLLocationCoordinate2D(latitude: 59.31, longitude: 18.06)
        let sw = CLLocationCoordinate2D(latitude: center.latitude - diff, longitude: center.longitude - diff);
        let ne = CLLocationCoordinate2D(latitude: center.latitude + diff, longitude: center.longitude + diff);

        return MGLCoordinateBounds(sw: sw, ne: ne);
    }
    
    func drawBounds(_ style: MGLStyle) {
        let b = bounds()
        let coordinates = [
            b.sw,
            CLLocationCoordinate2D(latitude: b.sw.latitude, longitude: b.ne.longitude),
            b.ne,
            CLLocationCoordinate2D(latitude: b.ne.latitude, longitude: b.sw.longitude),
            b.sw
        ]

        let polyline = MGLPolyline(coordinates: coordinates, count: UInt(coordinates.count))
        let source = MGLShapeSource(identifier: "polyline", shape: polyline, options: nil)
        style.addSource(source)
        
        let layer = MGLLineStyleLayer(identifier: "polyline", source: source)
        layer.lineColor = NSExpression(forConstantValue: UIColor.red)
        layer.lineWidth = NSExpression(forConstantValue: 4.0)
        style.addLayer(layer)
    }

    @objc
    func up() {
        let camera = mapView.camera(mapView.camera.copy() as! MGLMapCamera,
            fitting: bounds(),
            edgePadding:UIEdgeInsets(top: 200.0, left: 0, bottom: 0.0, right: 0))
        mapView.fly(to: camera, withDuration: 1.0, completionHandler: nil)
    }
    
    @objc
    func down() {
        let camera = mapView.camera(
            mapView.camera.copy() as! MGLMapCamera,
            fitting: bounds(),
            edgePadding:UIEdgeInsets(top: 0.0, left: 0, bottom: 200.0, right: 0))
        mapView.fly(to: camera, withDuration: 1.0, completionHandler: nil)
    }
    
    func mapView(_ mapView: MGLMapView, didFinishLoading style: MGLStyle ) {
        drawBounds(style)
    }
}

@ferdicus
Copy link

ferdicus commented Jun 4, 2020

any news on this issue?

@mfazekas
Copy link

mfazekas commented Jun 18, 2020

I think the issue is that previously in the C++ mapbox-gl-native padding on CameraOptions was not used, but padding caused cameraCenter to adjust according to padding.

With the new C++ mapbox-gl-native CameraOptions padding field is now used. But when converting to/from MGLMapCamera it's lost.

The issue seems to be caused by this change:
https://github.com/mapbox/mapbox-gl-native/pull/16067/files

- (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];
}

https://github.com/mapbox/mapbox-gl-native-ios/blob/master/platform/darwin/src/MGLMapCamera.h

https://github.com/mapbox/mapbox-gl-native/blob/ae4a0c3ee3c9aadabb49f607425920ce38a53ec9/include/mbgl/map/camera.hpp#L29-L31

https://github.com/mapbox/mapbox-gl-native/blob/3600dd8a1a4d91290d752d699ed964eab03bb1a5/src/mbgl/map/map.cpp#L213-L230

@astojilj
Copy link
Contributor

@mfazekas
Thanks for analysis. As a result of mapbox/mapbox-gl-native#16067, only specified padding is taken into account when calculating camera that fits geometry.

As you mentioned, the additive charter of inset management here:

- (MGLMapCamera *)camera:(MGLMapCamera *)camera fittingShape:(MGLShape *)shape edgePadding:(UIEdgeInsets)insets {
if (!_mbglMap)
{
return self.residualCamera;
}
mbgl::EdgeInsets padding = MGLEdgeInsetsFromNSEdgeInsets(insets);
padding += MGLEdgeInsetsFromNSEdgeInsets(self.contentInset);

is lost when converting to/from MGLMapCamera it's lost.

Would this approach resolve the issue

https://github.com/mapbox/mapbox-navigation-ios/pull/2471/files#diff-a43bde32226ca771fb8f732e90f4fc6aR1297 ?

In short, mapView.contentInset is set to desired value (or use automatically set value) and .zero (edgePadding: .zero) used in camera fitting or cameraThatFitsShape.

cc @maxmamis @ferdicus

@pappalar
Copy link

I just wanted to understand if #323 should cover this issue.

I think this would also cover #86, as I had to resort to the camera inset since fly does not have a api to set the edge inset directly (while setCamera has)

- (void)flyToCamera:(MGLMapCamera *)camera withDuration:(NSTimeInterval)duration peakAltitude:(CLLocationDistance)peakAltitude completionHandler:(nullable void (^)(void))completion;
- (void)setCamera:(MGLMapCamera *)camera withDuration:(NSTimeInterval)duration animationTimingFunction:(nullable CAMediaTimingFunction *)function edgePadding:(UIEdgeInsets)edgePadding completionHandler:(nullable void (^)(void))completion;

@Bogdan-Belogurov
Copy link

Bogdan-Belogurov commented Sep 22, 2020

Hi, I have the same issue.
Any news about #323?

Environment
Mapbox SDK version: v6.2.0
iOS versions: iOS 14 / 13
Xcode version: Xcode 12

@1ec5
Copy link
Contributor

1ec5 commented Oct 19, 2020

Some of the discussion in mapbox/mapbox-gl-native#15233 may also be relevant.

@maxmamis
Copy link
Author

confirmed that #323 fixes the issue — would love to see it get merged!

@india2sarthak
Copy link

Any updates on this? Still an issue with 6.2.0

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

Successfully merging a pull request may close this issue.

10 participants