From 24edfeb19dd2666574684942958825b07810bc80 Mon Sep 17 00:00:00 2001
From: Naftali Beder <naftali@naftalibeder.com>
Date: Tue, 13 Feb 2024 08:45:39 -0600
Subject: [PATCH] Fix double padding on camera when setting bounds (#3354)

* Zero padding when it's already baked into center/zoom from bounds

* Apply bounds camera directly to prevent padding being applied twice

* Update example to allow granular control over padding

* Scale down padding if it surpasses map size

* Remove instant set on zero duration to allow dynamic duration in flyTo

* Enable setting min and max zoom limits

* Allow removing map zoom limits and avoid unneeded zoom comparisons

* Remove unused import
---
 example/src/examples/V10/CameraAnimation.tsx | 289 +++++++++++--------
 ios/RNMBX/RNMBXCamera.swift                  |  99 +++----
 2 files changed, 210 insertions(+), 178 deletions(-)

diff --git a/example/src/examples/V10/CameraAnimation.tsx b/example/src/examples/V10/CameraAnimation.tsx
index 1e1b6185f..faafa9a67 100644
--- a/example/src/examples/V10/CameraAnimation.tsx
+++ b/example/src/examples/V10/CameraAnimation.tsx
@@ -1,9 +1,8 @@
-import { Divider, Text } from '@rneui/base';
+import { CheckBox, Divider, Slider, Text } from '@rneui/base';
 import {
   Camera,
   CameraAnimationMode,
   CameraBounds,
-  CameraPadding,
   CircleLayer,
   Logger,
   MapView,
@@ -36,80 +35,48 @@ const initialCoordinate: Coordinate = {
   longitude: -73.984638,
 };
 
-const zeroPadding: CameraPadding = {
-  paddingTop: 0,
-  paddingBottom: 0,
-  paddingLeft: 0,
-  paddingRight: 0,
-};
-const evenPadding: CameraPadding = {
-  paddingTop: 40,
-  paddingBottom: 40,
-  paddingLeft: 40,
-  paddingRight: 40,
-};
-const minZoomLevel = 8;
-const maxZoomLevel = 16;
-
-const randPadding = (): CameraPadding => {
-  const randNum = () => {
-    const items = [0, 150, 300];
-    return items[Math.floor(Math.random() * items.length)];
-  };
-
-  return {
-    paddingTop: randNum(),
-    paddingBottom: randNum(),
-    paddingLeft: randNum(),
-    paddingRight: randNum(),
-  };
-};
-
 const toPosition = (coordinate: Coordinate): Position => {
   return [coordinate.longitude, coordinate.latitude];
 };
 
+const rand = () => Math.random() * 0.008;
+
 const CameraAnimation = () => {
-  const [animationMode, setAnimationMode] =
-    useState<CameraAnimationMode>('moveTo');
+  const [easing, setEasing] = useState<CameraAnimationMode>('easeTo');
   const [coordinates, setCoordinates] = useState<Coordinate[]>([
     initialCoordinate,
   ]);
-  const [padding, setPadding] = useState<CameraPadding>(zeroPadding);
-
-  const paddingDisplay = useMemo(() => {
-    return `L ${padding.paddingLeft} | R ${padding.paddingRight} | T ${padding.paddingTop} | B ${padding.paddingBottom}`;
-  }, [padding]);
+  const [paddingLeft, setPaddingLeft] = useState(0);
+  const [paddingRight, setPaddingRight] = useState(0);
+  const [paddingTop, setPaddingTop] = useState(0);
+  const [paddingBottom, setPaddingBottom] = useState(0);
+  const [minZoom, setMinZoom] = useState<number | undefined>(undefined);
+  const [maxZoom, setMaxZoom] = useState<number | undefined>(undefined);
 
-  const move = useCallback(
-    (_animationMode: CameraAnimationMode, shouldCreateMultiple: boolean) => {
-      setAnimationMode(_animationMode);
-
-      if (shouldCreateMultiple) {
-        const _centerCoordinate = {
-          latitude: initialCoordinate.latitude + Math.random() * 0.2,
-          longitude: initialCoordinate.longitude + Math.random() * 0.2,
-        };
-        const _coordinates = Array(10)
-          .fill(0)
-          .map((_) => {
-            return {
-              latitude: _centerCoordinate.latitude + Math.random() * 0.2,
-              longitude: _centerCoordinate.longitude + Math.random() * 0.2,
-            };
-          });
-        setCoordinates(_coordinates);
-      } else {
-        setCoordinates([
-          {
-            latitude: initialCoordinate.latitude + Math.random() * 0.2,
-            longitude: initialCoordinate.longitude + Math.random() * 0.2,
-          },
-        ]);
-      }
-    },
-    [],
-  );
+  const move = useCallback((kind: 'center' | 'bounds') => {
+    if (kind === 'bounds') {
+      const _centerCoordinate = {
+        latitude: initialCoordinate.latitude + rand(),
+        longitude: initialCoordinate.longitude + rand(),
+      };
+      const _coordinates = Array(10)
+        .fill(0)
+        .map((_) => {
+          return {
+            latitude: _centerCoordinate.latitude + rand(),
+            longitude: _centerCoordinate.longitude + rand(),
+          };
+        });
+      setCoordinates(_coordinates);
+    } else if (kind === 'center') {
+      setCoordinates([
+        {
+          latitude: initialCoordinate.latitude + rand(),
+          longitude: initialCoordinate.longitude + rand(),
+        },
+      ]);
+    }
+  }, []);
 
   const features = useMemo((): Feature<Point>[] => {
     return coordinates.map((p) => {
@@ -152,19 +119,101 @@ const CameraAnimation = () => {
     }
   }, [coordinates]);
 
-  const locationDisplay = useMemo(() => {
-    if (coordinates.length > 1) {
-      const ne = centerOrBounds.bounds?.ne.map((n) => n.toFixed(3));
-      const sw = centerOrBounds.bounds?.sw.map((n) => n.toFixed(3));
-      return `ne ${ne} | sw ${sw}`;
-    } else if (coordinates.length === 1) {
-      const lon = coordinates[0].longitude.toFixed(4);
-      const lat = coordinates[0].latitude.toFixed(4);
-      return `lon ${lon} | lat ${lat}`;
-    } else {
-      throw new Error('invalid location passed');
-    }
-  }, [coordinates, centerOrBounds]);
+  const easingCheckBox = useCallback(
+    (value: CameraAnimationMode, label: string) => {
+      const isChecked = value === easing;
+      return (
+        <View
+          style={{
+            flex: 0,
+            flexDirection: 'row',
+            alignItems: 'center',
+          }}
+        >
+          <CheckBox
+            checked={isChecked}
+            center={true}
+            onIconPress={() => setEasing(value)}
+            containerStyle={{
+              backgroundColor: 'transparent',
+              marginRight: -4,
+            }}
+          />
+          <Text
+            style={{
+              flex: 0,
+              color: isChecked ? colors.primary.blue : undefined,
+            }}
+          >
+            {label}
+          </Text>
+        </View>
+      );
+    },
+    [easing],
+  );
+
+  const paddingCounter = useCallback(
+    (value: number, setValue: (value: number) => void, label: string) => {
+      return (
+        <View style={{ flex: 1, paddingHorizontal: 10 }}>
+          <View style={{ flex: 0, alignItems: 'center' }}>
+            <Text>{label}</Text>
+            <Text style={{ fontWeight: 'bold' }}>{`${Math.round(value)}`}</Text>
+          </View>
+          <Slider
+            thumbStyle={{
+              backgroundColor: 'black',
+              width: 15,
+              height: 15,
+            }}
+            value={value}
+            minimumValue={0}
+            maximumValue={500}
+            onSlidingComplete={(_value) => setValue(_value)}
+          />
+        </View>
+      );
+    },
+    [],
+  );
+
+  const zoomLimitCounter = useCallback(
+    (
+      value: number | undefined,
+      setValue: (value?: number) => void,
+      label: string,
+    ) => {
+      return (
+        <View style={{ flex: 1, paddingHorizontal: 10 }}>
+          <View style={{ flex: 0, alignItems: 'center' }}>
+            <Text>{label}</Text>
+            <Text style={{ fontWeight: 'bold' }}>{`${
+              value ?? 'Not set'
+            }`}</Text>
+          </View>
+          <Slider
+            thumbStyle={{
+              backgroundColor: 'black',
+              width: 15,
+              height: 15,
+            }}
+            value={value}
+            minimumValue={-1}
+            maximumValue={20}
+            onSlidingComplete={(_value) => {
+              if (_value < 0) {
+                setValue(undefined);
+              } else {
+                setValue(Math.round(_value));
+              }
+            }}
+          />
+        </View>
+      );
+    },
+    [],
+  );
 
   return (
     <>
@@ -172,11 +221,16 @@ const CameraAnimation = () => {
         <Camera
           {...centerOrBounds}
           zoomLevel={12}
-          minZoomLevel={minZoomLevel}
-          maxZoomLevel={maxZoomLevel}
-          padding={padding}
+          minZoomLevel={minZoom}
+          maxZoomLevel={maxZoom}
+          padding={{
+            paddingTop,
+            paddingBottom,
+            paddingLeft,
+            paddingRight,
+          }}
           animationDuration={800}
-          animationMode={animationMode}
+          animationMode={easing}
         />
 
         {features.map((feature) => {
@@ -192,53 +246,39 @@ const CameraAnimation = () => {
       <SafeAreaView>
         <View style={styles.sheet}>
           <View style={styles.content}>
-            <Text style={styles.fadedText}>centerCoordinate</Text>
+            <Text style={styles.sectionText}>Coordinate</Text>
             <View style={styles.buttonRow}>
-              <Button title="Flight" onPress={() => move('flyTo', false)} />
-              <Button title="Ease" onPress={() => move('easeTo', false)} />
-              <Button title="Linear" onPress={() => move('linearTo', false)} />
-              <Button title="Instant" onPress={() => move('moveTo', false)} />
+              <Button title="Center" onPress={() => move('center')} />
+              <Button title="Bounds" onPress={() => move('bounds')} />
             </View>
 
             <Divider style={styles.divider} />
 
-            <Text style={styles.fadedText}>bounds</Text>
-            <View style={styles.buttonRow}>
-              <Button title="Flight" onPress={() => move('flyTo', true)} />
-              <Button title="Ease" onPress={() => move('easeTo', true)} />
-              <Button title="Linear" onPress={() => move('linearTo', true)} />
-              <Button title="Instant" onPress={() => move('moveTo', true)} />
+            <Text style={styles.sectionText}>Easing</Text>
+            <View style={[styles.buttonRow, { marginBottom: -6 }]}>
+              {easingCheckBox('easeTo', 'Ease')}
+              {easingCheckBox('linearTo', 'Linear')}
+              {easingCheckBox('flyTo', 'Fly')}
+              {easingCheckBox('moveTo', 'Move')}
             </View>
 
             <Divider style={styles.divider} />
 
-            <Text style={styles.fadedText}>padding</Text>
-            <View style={styles.buttonRow}>
-              <Button
-                title="Zero"
-                onPress={() => {
-                  setPadding(zeroPadding);
-                }}
-              />
-              <Button
-                title="Even"
-                onPress={() => {
-                  setPadding(evenPadding);
-                }}
-              />
-              <Button
-                title="Random"
-                onPress={() => {
-                  setPadding(randPadding());
-                }}
-              />
+            <Text style={styles.sectionText}>Padding</Text>
+            <View style={[styles.buttonRow, { marginTop: 6 }]}>
+              {paddingCounter(paddingTop, setPaddingTop, 'Top')}
+              {paddingCounter(paddingBottom, setPaddingBottom, 'Bottom')}
+              {paddingCounter(paddingLeft, setPaddingLeft, 'Left')}
+              {paddingCounter(paddingRight, setPaddingRight, 'Right')}
             </View>
 
             <Divider style={styles.divider} />
 
-            <Text style={styles.fadedText}>info</Text>
-            <Text>position: {locationDisplay}</Text>
-            <Text>padding: {paddingDisplay}</Text>
+            <Text style={styles.sectionText}>Zoom limits</Text>
+            <View style={[styles.buttonRow, { marginTop: 6 }]}>
+              {zoomLimitCounter(minZoom, setMinZoom, 'Min')}
+              {zoomLimitCounter(maxZoom, setMaxZoom, 'Max')}
+            </View>
           </View>
         </View>
       </SafeAreaView>
@@ -251,22 +291,23 @@ const styles = StyleSheet.create({
     flex: 1,
   },
   sheet: {
-    paddingTop: 10,
     paddingHorizontal: 10,
+    marginBottom: -10,
   },
   content: {
     padding: 10,
   },
+  sectionText: {
+    fontSize: 10,
+    color: 'gray',
+  },
   buttonRow: {
     flex: 0,
     flexDirection: 'row',
-    justifyContent: 'space-around',
+    justifyContent: 'space-evenly',
   },
   divider: {
-    marginVertical: 10,
-  },
-  fadedText: {
-    color: 'gray',
+    marginVertical: 8,
   },
 });
 
diff --git a/ios/RNMBX/RNMBXCamera.swift b/ios/RNMBX/RNMBXCamera.swift
index 7c172fa7b..40ec5de31 100644
--- a/ios/RNMBX/RNMBXCamera.swift
+++ b/ios/RNMBX/RNMBXCamera.swift
@@ -40,15 +40,16 @@ struct CameraUpdateItem {
       if let center = camera.center {
         try center.validate()
       }
+
       switch mode {
-        case .flight:
+      case .flight:
         map.mapView.camera.fly(to: camera, duration: duration)
-        case .ease:
+      case .ease:
         map.mapView.camera.ease(to: camera, duration: duration ?? 0, curve: .easeInOut, completion: nil)
-        case .linear:
+      case .linear:
         map.mapView.camera.ease(to: camera, duration: duration ?? 0, curve: .linear, completion: nil)
-        default:
-          map.mapboxMap.setCamera(to: camera)
+      default:
+        map.mapboxMap.setCamera(to: camera)
       }
     }
   }
@@ -259,14 +260,12 @@ open class RNMBXCamera : RNMBXMapComponentBase {
         logged("RNMBXCamera._updateMaxBounds._toCoordinateBounds") {
           options.bounds = try self._toCoordinateBounds(maxBounds)
         }
+      } else {
+        options.bounds = nil
       }
-      if let minZoomLevel = self.minZoomLevel {
-        options.minZoom = minZoomLevel.CGFloat
-      }
-      if let maxZoomLevel = self.maxZoomLevel {
-        options.maxZoom = maxZoomLevel.CGFloat
-      }
-
+      options.minZoom = self.minZoomLevel?.CGFloat
+      options.maxZoom = self.maxZoomLevel?.CGFloat
+      
       logged("RNMBXCamera._updateMaxBounds") {
         try map.mapboxMap.setCameraBounds(with: options)
       }
@@ -371,11 +370,12 @@ open class RNMBXCamera : RNMBXMapComponentBase {
     if (stop.isEmpty) {
       return nil
     }
+    
     var zoom: CGFloat?
     if let z = stop["zoom"] as? Double {
       zoom = CGFloat(z)
     }
-    
+
     var pitch: CGFloat?
     if let p = stop["pitch"] as? Double {
       pitch = CGFloat(p)
@@ -386,20 +386,22 @@ open class RNMBXCamera : RNMBXMapComponentBase {
       heading = CLLocationDirection(h)
     }
     
-    let padding = UIEdgeInsets(
+    var padding: UIEdgeInsets = UIEdgeInsets(
       top: stop["paddingTop"] as? Double ?? 0,
       left: stop["paddingLeft"] as? Double ?? 0,
       bottom: stop["paddingBottom"] as? Double ?? 0,
       right: stop["paddingRight"] as? Double ?? 0
     )
-
-    var center: LocationCoordinate2D?
-    if let feature: String = stop["centerCoordinate"] as? String {
-      
+    
+    var camera: CameraOptions?
+    
+    if let feature = stop["centerCoordinate"] as? String {
       let centerFeature : Turf.Feature? = logged("RNMBXCamera.toUpdateItem.decode.cc") { try
         JSONDecoder().decode(Turf.Feature.self, from: feature.data(using: .utf8)!)
       }
       
+      var center: LocationCoordinate2D?
+      
       switch centerFeature?.geometry {
       case .point(let centerPoint):
         center = centerPoint.coordinates
@@ -407,7 +409,16 @@ open class RNMBXCamera : RNMBXMapComponentBase {
         Logger.log(level: .error, message: "RNMBXCamera.toUpdateItem: Unexpected geometry: \(String(describing: centerFeature?.geometry))")
         return nil
       }
-    } else if let feature: String = stop["bounds"] as? String {
+      
+      camera = CameraOptions(
+        center: center,
+        padding: padding,
+        anchor: nil,
+        zoom: zoom,
+        bearing: heading,
+        pitch: pitch
+      )
+    } else if let feature = stop["bounds"] as? String {
       let collection : Turf.FeatureCollection? = logged("RNMBXCamera.toUpdateItem.decode.bound") { try
         JSONDecoder().decode(Turf.FeatureCollection.self, from: feature.data(using: .utf8)!) }
       let features = collection?.features
@@ -436,55 +447,35 @@ open class RNMBXCamera : RNMBXMapComponentBase {
         #else
         let bounds = CoordinateBounds(southwest: sw, northeast: ne)
         #endif
-        let camera = map.mapboxMap.camera(
+
+        camera = map.mapboxMap.camera(
           for: bounds,
           padding: padding,
-          bearing: heading ?? map.cameraState.bearing,
-          pitch: pitch ?? map.cameraState.pitch
+          bearing: heading ?? map.mapboxMap.cameraState.bearing,
+          pitch: pitch ?? map.mapboxMap.cameraState.pitch
         )
-
-        if let _center = camera.center, let _zoom = camera.zoom {
-          center = _center
-          zoom = _zoom
-        }
       }
     }
 
-    let duration: TimeInterval? = {
-      if let d = stop["duration"] as? Double {
-        return toSeconds(d)
-      }
+    guard let camera = camera else {
       return nil
-    }()
-    
-    let mode: CameraMode = {
-      if let m = stop["mode"] as? String, let m = CameraMode(rawValue: m) {
-        return m
-      }
-      return .flight
-    }()
-    
-    if let z1 = minZoomLevel, let z2 = CGFloat(exactly: z1), zoom ?? 100 < z2 {
-      zoom = z2
     }
 
-    if let z1 = maxZoomLevel, let z2 = CGFloat(exactly: z1), zoom ?? 0 > z2 {
-      zoom = z2
+    var duration: TimeInterval?
+    if let d = stop["duration"] as? Double {
+      duration = toSeconds(d)
+    }
+    
+    var mode: CameraMode = .flight
+    if let m = stop["mode"] as? String, let m = CameraMode(rawValue: m) {
+      mode = m
     }
 
-    let result = CameraUpdateItem(
-      camera: CameraOptions(
-        center: center,
-        padding: padding,
-        anchor: nil,
-        zoom: zoom,
-        bearing: heading,
-        pitch: pitch
-      ),
+    return CameraUpdateItem(
+      camera: camera,
       mode: mode,
       duration: duration
     )
-    return result
   }
   
   func _updateCamera() {