nitaliano / react-native-mapbox-gl

A Mapbox GL react native module for creating custom maps
Other
2.16k stars 697 forks source link

Fixed erratic zoom behavior when using setCamera with Flight mode #1547

Closed mattijsf closed 5 years ago

mattijsf commented 5 years ago

Fixes https://github.com/nitaliano/react-native-mapbox-gl/issues/1416 and https://github.com/nitaliano/react-native-mapbox-gl/issues/1466

I personally experienced the effect of the bug below using setCamera with Flight Mode but due to the nature of the bug it might also fix other camera related issues.

I found that the existing [RCTMGLUtils clamp] function used in the _makeCamera -> altitudeFromZoom -> getMetersPerPixelAtLatitude was not always returning consistent values. It might be related to the the MIN() MAX() operations on top of NSNumber instances. I'm unsure if the behavior of these macro's is defined when used with NSNumber. Migrating away from this approach fixes some camera issues.

Test Code:

- (void)testRCTMGLUtilsClamp {
  XCTAssertEqual([[RCTMGLUtils clamp:@(14.1) min:@(1) max:@(22)] doubleValue], 14.1);
}

In the above test case the second XCTAssertEqual line fails: RCTUtilsClampTest.m:28: error: -[RCTUtilsClampTest testRCTMGLUtilsClamp] : (([[RCTMGLUtils clamp:@(14.1) min:@(1) max:@(22)] doubleValue]) equal to (14.1)) failed: ("1") is not equal to ("14.1")

 

Video (before / after) Broken clamp: ![flight-mode-broken](https://user-images.githubusercontent.com/706368/54608238-99390200-4a50-11e9-957c-c3765bad4fd9.gif) Fixed clamp: ![flight-mode-fixed](https://user-images.githubusercontent.com/706368/54608270-ab1aa500-4a50-11e9-95c3-ba44d3355974.gif)

 

Example code ```jsx import React from 'react'; import MapboxGL from '@mapbox/react-native-mapbox-gl'; import sheet from '../styles/sheet'; import BaseExamplePropTypes from './common/BaseExamplePropTypes'; import TabBarPage from './common/TabBarPage'; class SetCameraFlight extends React.Component { static SF_OFFICE_LOCATION = [-122.400021, 37.789085]; static DC_OFFICE_LOCATION = [-77.036086, 38.910233]; static propTypes = { ...BaseExamplePropTypes, }; constructor(props) { super(props); this._flyToOptions = [ {label: 'SF', data: SetCameraFlight.SF_OFFICE_LOCATION}, {label: 'DC', data: SetCameraFlight.DC_OFFICE_LOCATION}, ]; this.onFlyToPress = this.onFlyToPress.bind(this); } async onFlyToPress(i, coordinates) { await this.map.setCamera({ zoom: 14.1, duration: 2000, centerCoordinate: coordinates, mode: MapboxGL.CameraModes.Flight, }); } render() { return ( (this.map = ref)} style={sheet.matchParent} /> ); } } export default SetCameraFlight; ```
kristfal commented 5 years ago

Great fix! Thanks a lot. Merging.