Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix camera method and address deprecations #3602

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,19 @@ package com.rnmapbox.rnmbx.components.camera

import android.animation.Animator
import android.content.Context
import com.rnmapbox.rnmbx.utils.GeoJSONUtils.toPointGeometry
import com.rnmapbox.rnmbx.utils.GeoJSONUtils.toLatLng
import com.rnmapbox.rnmbx.utils.GeoJSONUtils.toLatLngBounds
import com.rnmapbox.rnmbx.utils.LatLngBounds
import com.rnmapbox.rnmbx.components.mapview.RNMBXMapView
import com.mapbox.maps.CameraOptions
import com.facebook.react.bridge.ReadableMap
import com.mapbox.geojson.FeatureCollection
import com.mapbox.geojson.Point
import com.mapbox.maps.CameraOptions
import com.mapbox.maps.EdgeInsets
import com.mapbox.maps.ScreenCoordinate
import com.rnmapbox.rnmbx.components.camera.constants.CameraMode
import com.rnmapbox.rnmbx.components.mapview.RNMBXMapView
import com.rnmapbox.rnmbx.utils.GeoJSONUtils.toLatLng
import com.rnmapbox.rnmbx.utils.GeoJSONUtils.toLatLngBounds
import com.rnmapbox.rnmbx.utils.GeoJSONUtils.toPointGeometry
import com.rnmapbox.rnmbx.utils.LatLng
import com.rnmapbox.rnmbx.utils.LatLngBounds

class CameraStop {
private var mBearing: Double? = null
Expand Down Expand Up @@ -124,14 +126,22 @@ class CameraStop {
if (mLatLng != null) {
builder.center(mLatLng!!.point)
} else if (mBounds != null) {
val coordinates = listOf<Point>(
mBounds!!.toBounds().northeast,
mBounds!!.toBounds().southwest
)
val tilt = if (mTilt != null) mTilt!! else currentCamera.pitch
val bearing = if (mBearing != null) mBearing!! else currentCamera.bearing

val boundsCamera = map.cameraForCoordinateBounds(
mBounds!!.toBounds(),
val boundsCamera = map.cameraForCoordinates(
coordinates,
CameraOptions.Builder()
.pitch(tilt)
.bearing(bearing)
.build(),
cameraPaddingEdgeInsets,
bearing,
tilt
null,
ScreenCoordinate(0.0, 0.0)
)
builder.center(boundsCamera.center)
builder.anchor(boundsCamera.anchor)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,11 @@ class CameraUpdateItem(
}
}
val map = mMap.get()
if (map == null) {
if (map == null || mCameraUpdate.center == null) {
isCameraActionCancelled = true
return
}

val animationOptions = MapAnimationOptions.Builder();

// animateCamera / easeCamera only allows positive duration
Expand Down
58 changes: 39 additions & 19 deletions ios/RNMBX/RNMBXCamera.swift
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,18 @@ struct CameraUpdateItem {
if let center = camera.center {
try center.validate()
}

switch mode {
case .flight:
map.mapView.camera.fly(to: camera, duration: duration)
case .ease:
map.mapView.camera.ease(to: camera, duration: duration ?? 0, curve: .easeInOut, completion: nil)
case .linear:
map.mapView.camera.ease(to: camera, duration: duration ?? 0, curve: .linear, completion: nil)
default:
map.mapboxMap.setCamera(to: camera)

map.withMapView { mapView in
switch mode {
naftalibeder marked this conversation as resolved.
Show resolved Hide resolved
case .flight:
mapView.camera.fly(to: camera, duration: duration)
case .ease:
mapView.camera.ease(to: camera, duration: duration ?? 0, curve: .easeInOut, completion: nil)
case .linear:
mapView.camera.ease(to: camera, duration: duration ?? 0, curve: .linear, completion: nil)
default:
mapView.mapboxMap.setCamera(to: camera)
}
}
}
}
Expand Down Expand Up @@ -87,8 +89,10 @@ open class RNMBXMapComponentBase : UIView, RNMBXMapComponent {
}

func withMapView(_ callback: @escaping (_ mapView: MapView) -> Void) {
withRNMBXMapView { mapView in
callback(mapView.mapView)
withRNMBXMapView { map in
map.withMapView { mapView in
callback(mapView)
}
}
}

Expand Down Expand Up @@ -447,17 +451,33 @@ open class RNMBXCamera : RNMBXMapComponentBase {

withMapView { map in
#if RNMBX_11
let bounds = [sw, ne]
do {
let bounds: [CLLocationCoordinate2D] = [sw, ne]
camera = try map.mapboxMap.camera(
for: bounds,
camera: .init(cameraState: .init(
center: .init(),
padding: .zero,
zoom: zoom ?? 0,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've found that I have more consistant results by using zoom: .zero. If zoom is set on this camera is seems to effect the positioning.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mfazekas @naftalibeder I'll aim to make a demo this afternoon of why i've highlighted this.. but when moving the camera with setCamera from a centerCoordinates position to setCamera with bounds it seems that zoom is applied to the final position. Adding this zoom: .zero looks to fix this problem thou I need to look at why.

bearing: heading ?? map.mapboxMap.cameraState.bearing,
pitch: pitch ?? map.mapboxMap.cameraState.pitch
)),
coordinatesPadding: padding,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we use coordinatePadding and not camera padding?

maxZoom: nil,
offset: nil
)
} catch {
Logger.log(level: .error, message: "RNMBXCamera.toUpdateItem: Failed to build camera configuration: \(error)")
}
#else
let bounds = CoordinateBounds(southwest: sw, northeast: ne)
#endif

camera = map.mapboxMap.camera(
for: bounds,
padding: padding,
bearing: heading ?? map.mapboxMap.cameraState.bearing,
pitch: pitch ?? map.mapboxMap.cameraState.pitch
)
#endif
}
} else {
camera = CameraOptions(
Expand Down Expand Up @@ -528,8 +548,8 @@ open class RNMBXCamera : RNMBXMapComponentBase {
return false
}

map.mapView.viewport.removeStatusObserver(self)
return super.removeFromMap(map, reason:reason)
map._mapView.viewport.removeStatusObserver(self)
return super.removeFromMap(map, reason: reason)
}
}

Expand Down Expand Up @@ -568,12 +588,12 @@ extension RNMBXCamera : ViewportStatusObserver {
return "compass"
case .course:
return "course"
case .some(let bearing):
case .some(_):
return "constant"
case .none:
return "normal"
}
} else if let state = state as? OverviewViewportState {
} else if let _ = state as? OverviewViewportState {
return "overview"
} else {
return "custom"
Expand Down
Loading