Skip to content

Feat/mapviewcamera ops update #102

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Sep 10, 2025
Merged

Feat/mapviewcamera ops update #102

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
09677e3
Added setDirection and pan camera operations
Archdoog Sep 6, 2025
7357814
Added setDirection and pan camera operations
Archdoog Sep 6, 2025
f1abcf9
Added setDirection and pan camera operations
Archdoog Sep 6, 2025
9b752cb
Reverted MapViewProxy
Archdoog Sep 10, 2025
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
11 changes: 10 additions & 1 deletion Package.resolved
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions Package.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ let package = Package(
// Macros
.package(url: "https://github.com/swiftlang/swift-syntax.git", "509.0.0" ..< "602.0.0"),
// Testing
.package(url: "https://github.com/apple/swift-numerics.git", from: "1.1.0"),
.package(url: "https://github.com/Kolos65/Mockable.git", from: "0.3.1"),
.package(url: "https://github.com/pointfreeco/swift-snapshot-testing", from: "1.18.3"),
// Macro Testing
Expand Down Expand Up @@ -88,6 +89,8 @@ let package = Package(
name: "MapLibreSwiftUITests",
dependencies: [
"MapLibreSwiftUI",
.product(name: "MapLibre", package: "maplibre-gl-native-distribution"),
.product(name: "Numerics", package: "swift-numerics"),
.product(name: "Mockable", package: "Mockable"),
.product(name: "SnapshotTesting", package: "swift-snapshot-testing"),
]
Expand Down
172 changes: 153 additions & 19 deletions Sources/MapLibreSwiftUI/Extensions/MapViewCamera/MapViewCameraOperations.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,20 @@
import CarPlay
import Foundation
import OSLog

private let logger = Logger(subsystem: "MapLibreSwiftUI", category: "MapViewCameraOperations")

@MainActor
public extension MapViewCamera {
// MARK: Zoom

/// Set a new zoom for the current camera state.
///
/// - Parameter newZoom: The new zoom value.
mutating func setZoom(_ newZoom: Double) {
/// - Parameters:
/// - newZoom: The new zoom value.
/// - proxy: An optional map view proxy, this allows the camera to convert from .rect/.showcase to centered.
/// Allowing zoom from the user's current viewport.
mutating func setZoom(_ newZoom: Double, proxy: MapViewProxy? = nil) {
switch state {
case let .centered(onCoordinate, _, pitch, pitchRange, direction):
state = .centered(onCoordinate: onCoordinate,
Expand All @@ -20,19 +28,30 @@ public extension MapViewCamera {
state = .trackingUserLocationWithHeading(zoom: newZoom, pitch: pitch, pitchRange: pitchRange)
case let .trackingUserLocationWithCourse(_, pitch, pitchRange):
state = .trackingUserLocationWithCourse(zoom: newZoom, pitch: pitch, pitchRange: pitchRange)
case .rect:
return
case .showcase:
return
case .rect, .showcase:
// This method requires the proxy.
guard let proxy else {
logger.debug("Cannot setZoom on a .rect or .showcase camera without a proxy")
return
}

state = .centered(onCoordinate: proxy.centerCoordinate,
zoom: newZoom,
pitch: MapViewCamera.Defaults.pitch,
pitchRange: .free,
direction: proxy.direction)
}

lastReasonForChange = .programmatic
}

/// Increment the zoom of the current camera state.
///
/// - Parameter newZoom: The value to increment the zoom by. Negative decrements the value.
mutating func incrementZoom(by increment: Double) {
/// - Parameters:
/// - newZoom: The value to increment the zoom by. Negative decrements the value.
/// - proxy: An optional map view proxy, this allows the camera to convert from .rect/.showcase to centered.
/// Allowing zoom from the user's current viewport.
mutating func incrementZoom(by increment: Double, proxy: MapViewProxy? = nil) {
switch state {
case let .centered(onCoordinate, zoom, pitch, pitchRange, direction):
state = .centered(onCoordinate: onCoordinate,
Expand All @@ -51,10 +70,18 @@ public extension MapViewCamera {
state = .trackingUserLocationWithHeading(zoom: zoom + increment, pitch: pitch, pitchRange: pitchRange)
case let .trackingUserLocationWithCourse(zoom, pitch, pitchRange):
state = .trackingUserLocationWithCourse(zoom: zoom + increment, pitch: pitch, pitchRange: pitchRange)
case .rect:
return
case .showcase:
return
case .rect, .showcase:
// This method requires the proxy.
guard let proxy else {
logger.debug("Cannot incrementZoom on a .rect or .showcase camera without a proxy")
return
}

state = .centered(onCoordinate: proxy.centerCoordinate,
zoom: proxy.zoomLevel + increment,
pitch: MapViewCamera.Defaults.pitch,
pitchRange: .free,
direction: proxy.direction)
}

lastReasonForChange = .programmatic
Expand All @@ -64,8 +91,11 @@ public extension MapViewCamera {

/// Set a new pitch for the current camera state.
///
/// - Parameter newPitch: The new pitch value.
mutating func setPitch(_ newPitch: Double) {
/// - Parameters:
/// - newPitch: The new pitch value.
/// - proxy: An optional map view proxy, this allows the camera to convert from .rect/.showcase to centered.
/// Allowing zoom from the user's current viewport.
mutating func setPitch(_ newPitch: Double, proxy: MapViewProxy? = nil) {
switch state {
case let .centered(onCoordinate, zoom, _, pitchRange, direction):
state = .centered(onCoordinate: onCoordinate,
Expand All @@ -79,14 +109,118 @@ public extension MapViewCamera {
state = .trackingUserLocationWithHeading(zoom: zoom, pitch: newPitch, pitchRange: pitchRange)
case let .trackingUserLocationWithCourse(zoom, _, pitchRange):
state = .trackingUserLocationWithCourse(zoom: zoom, pitch: newPitch, pitchRange: pitchRange)
case .rect:
return
case .showcase:
return
case .rect, .showcase:
// This method requires the proxy.
guard let proxy else {
logger.debug("Cannot setPitch on a .rect or .showcase camera without a proxy")
return
}

state = .centered(onCoordinate: proxy.centerCoordinate,
zoom: proxy.zoomLevel,
pitch: newPitch,
pitchRange: .free,
direction: proxy.direction)
}

lastReasonForChange = .programmatic
}

// TODO: Add direction set
/// Set the direction of the camera.
///
/// This will convert a rect and showcase camera to a centered camera while rotating.
/// Tracking user location with heading and course will ignore this behavior.
///
/// - Parameters:
/// - newDirection: The new camera direction (0 - North to 360)
/// - proxy: An optional map view proxy, this allows the camera to convert from .rect/.showcase to centered.
/// Allowing zoom from the user's current viewport.
mutating func setDirection(_ newDirection: Double, proxy: MapViewProxy? = nil) {
switch state {
case let .centered(onCoordinate: onCoordinate, zoom: zoom, pitch: pitch, pitchRange: pitchRange, _):
state = .centered(
onCoordinate: onCoordinate,
zoom: zoom,
pitch: pitch,
pitchRange: pitchRange,
direction: newDirection
)
case let .trackingUserLocation(zoom: zoom, pitch: pitch, pitchRange: pitchRange, _):
state = .trackingUserLocation(zoom: zoom, pitch: pitch, pitchRange: pitchRange, direction: newDirection)
case .trackingUserLocationWithHeading:
logger.debug("Cannot setPitch while .trackingUserLocationWithHeading")
case .trackingUserLocationWithCourse:
logger.debug("Cannot setPitch while .trackingUserLocationWithCourse")
case .rect, .showcase:
// This method requires the proxy.
guard let proxy else {
logger.debug("Cannot setDirection on a .rect or .showcase camera without a proxy")
return
}

state = .centered(onCoordinate: proxy.centerCoordinate,
zoom: proxy.zoomLevel,
pitch: MapViewCamera.Defaults.pitch,
pitchRange: .free,
direction: newDirection)
}

lastReasonForChange = .programmatic
}

// MARK: Car Play

/// Pans the camera for a CarPlay view.
///
/// - Parameters:
/// - newPitch: The new pitch value.
/// - proxy: An optional map view proxy, this allows the camera to convert from .rect/.showcase to centered.
/// Allowing zoom from the user's current viewport.
mutating func pan(_ direction: CPMapTemplate.PanDirection, proxy: MapViewProxy? = nil) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We could create our own direction enum here, but I figured CarPlay is always available to import. Thoughts?

var currentZoom: Double?
var currentCenter: CLLocationCoordinate2D?

switch state {
case let .centered(onCoordinate: onCoordinate, zoom: zoom, _, _, _):
currentZoom = zoom
currentCenter = onCoordinate
case let .trackingUserLocation(zoom: zoom, _, _, _),
let .trackingUserLocationWithHeading(zoom: zoom, _, _),
let .trackingUserLocationWithCourse(zoom: zoom, _, _):
currentZoom = zoom
case .rect, .showcase:
break
}

let zoom = currentZoom ?? proxy?.zoomLevel ?? MapViewCamera.Defaults.zoom
let center = currentCenter ?? proxy?.centerCoordinate ?? MapViewCamera.Defaults.coordinate

// Adjust +5 for desired sensitivity
let sensitivity: Double = 5
// Pan distance decreases exponentially with zoom level
// At zoom 0: ~5.6 degrees, at zoom 10: ~0.0055 degrees, at zoom 20: ~0.000005 degrees
let basePanDistance = 360.0 / pow(2, zoom + sensitivity)
let latitudeDelta = basePanDistance
let longitudeDelta = basePanDistance / cos(center.latitude * .pi / 180)

let newCenter: CLLocationCoordinate2D? = switch direction {
case .down:
CLLocationCoordinate2D(latitude: center.latitude - latitudeDelta, longitude: center.longitude)
case .up:
CLLocationCoordinate2D(latitude: center.latitude + latitudeDelta, longitude: center.longitude)
case .left:
CLLocationCoordinate2D(latitude: center.latitude, longitude: center.longitude - longitudeDelta)
case .right:
CLLocationCoordinate2D(latitude: center.latitude, longitude: center.longitude + longitudeDelta)
default:
nil
}

guard let newCenter else {
return
}

self = .center(newCenter, zoom: zoom)
lastReasonForChange = .programmatic
}
}
3 changes: 2 additions & 1 deletion Sources/MapLibreSwiftUI/Models/MapCamera/MapViewCamera.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ import MapLibre
/// The SwiftUI MapViewCamera.
///
/// This manages the camera state within the MapView.
public struct MapViewCamera: Hashable, Equatable, Sendable, CustomStringConvertible {
@MainActor
public struct MapViewCamera: Hashable, Equatable, Sendable, @preconcurrency CustomStringConvertible {
public enum Defaults {
public static let coordinate = CLLocationCoordinate2D(latitude: 0, longitude: 0)
public static let zoom: Double = 10
Expand Down
84 changes: 60 additions & 24 deletions Sources/MapLibreSwiftUI/Models/MapViewProxy.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,46 +18,82 @@ import MapLibre
@MainActor
public struct MapViewProxy: Hashable, Equatable {
/// The current center coordinate of the MapView
public var centerCoordinate: CLLocationCoordinate2D {
mapView.centerCoordinate
}
public let centerCoordinate: CLLocationCoordinate2D
Copy link
Collaborator

Choose a reason for hiding this comment

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

So, a few discussion points here (that we want input from @hactar on). We changed this to improve testability (constructing this required a full MapLibre map object, which isn't really possible to configure for testing properly), and @Archdoog thinks that current usage patterns would not be holding on to the proxy long term. The proxy is recreated every time the region changes internally.

However, as written it's technically possible that some attribute of the map view change underneath, and the proxy is then out of sync. This is not great. But the usage pattern is that you keep getting proxies via the onMapViewProxyUpdate modifier.

So there seems to be an argument for making this an immutable struct and making it clear that this is not guaranteed to be valid after onMapViewProxyUpdate completes. It might also be worth changing the name if this is how we land (i.e. MapViewState or something... idk naming is hard).

As an aside, we can also make the initializers internal.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Looking at https://developer.apple.com/documentation/swiftui/geometryproxy, we might consider actually building a custom private/internal struct that can be used instead of the mapView on the proxy. Its job would be to hold the data required to calculate the frame to lat-long and other similar functionality. I think would let us snapshot the advanced functionality without holding a reference to the whole MapView. Bonus, it'd also further enhance testability.

Copy link
Collaborator

Choose a reason for hiding this comment

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

So lets start with the easy part: I tested my bigger app against the branch and after some adjustments found no regressions, so all is good here.

But that said, I'm not sure how ideal this new solution is:

The proxy currently only covers a small part (the stuff I needed) of what might need to be exposed via it to get feature parity with MapLibre. The previous solution lazily "set" the properties - only the properties the dev actually uses were read out of MapLibre. Now, all properties have to be read out for every update of the proxy. Currently it is 7, but it could become a lot more if we go for MapLibre feature parity. Does this have a measurable performance impact, especially when ProxyUpdateMode is set to .realtime? If it doesn't yet, will it when the proxy has 20 values, or 40? I don't know - but the current lazy get solution would definitely not suffer here.

Secondly, now that the mapView is optional, all functions that use it have to have optional return values, making the proxy more difficult to use in code: everything needs to be unwrapped. In non test usage, you will always have an MLNMapView - making usage on call site worse just for testing isn't ideal.

If I see it correctly you don't want to test the proxy itself, but need to provide a proxy for other camera related tests, without having a map system in place. Wouldn't a better alternative be to create a MapViewProxyProtocol, then keep the MapViewProxy as it currently is, but also create a MockMapViewProxy that can be initialized with values as needed? The MockMapViewProxy wouldn't have to have an private let mapView: MLNMapView in it at all while it would still be a requirement for the real MapViewProxy?

Copy link
Collaborator

@hactar hactar Sep 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Looking at https://developer.apple.com/documentation/swiftui/geometryproxy, we might consider actually building a custom private/internal struct that can be used instead of the mapView on the proxy. Its job would be to hold the data required to calculate the frame to lat-long and other similar functionality. I think would let us snapshot the advanced functionality without holding a reference to the whole MapView. Bonus, it'd also further enhance testability.

I'm all for not having a reference to MLNMapView if possible - but how would you go about solving the second use case of MapViewProxy: calling into functions like convert that MLNMapView provides?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was a good discussion to do some more research on. Outcome was:

  1. Since the MLNMapView already exists as a ref type, there's little cost to attaching it to the MapViewProxy. This is basically what you said @hactar and given the potential to extend further map view proxy features makes sense.
  2. We already solved this problem for the camera. I've used this concept and renamed the camera's map view protocol to MLNMapViewRepresentable. This is essentially the protocol that exposes any MLNMapView param or function we want to inject and test on an object.

This has been fixed and now we've got the original MapViewProxy but with my test suite.


/// The current zoom value of the MapView
public var zoomLevel: Double {
mapView.zoomLevel
}
public let zoomLevel: Double

/// The current compass direction of the MapView
public var direction: CLLocationDirection {
mapView.direction
}
public let direction: CLLocationDirection

public var visibleCoordinateBounds: MLNCoordinateBounds {
mapView.visibleCoordinateBounds
}
/// The visible coordinate bounds of the MapView
public let visibleCoordinateBounds: MLNCoordinateBounds

public var mapViewSize: CGSize {
mapView.frame.size
}
/// The size of the MapView
public let mapViewSize: CGSize

public var contentInset: UIEdgeInsets {
mapView.contentInset
}
/// The content inset of the MapView
public let contentInset: UIEdgeInsets

/// The reason the view port was changed.
public let lastReasonForChange: CameraChangeReason?

private let mapView: MLNMapView
/// The underlying MLNMapView (only used for functions that require it)
private let mapView: MLNMapView?

public func convert(_ coordinate: CLLocationCoordinate2D, toPointTo: UIView?) -> CGPoint {
mapView.convert(coordinate, toPointTo: toPointTo)
/// Convert a coordinate to a point in the MapView
/// - Parameters:
/// - coordinate: The coordinate to convert
/// - toPointTo: The view to convert the point to (usually nil for the MapView itself)
/// - Returns: The CGPoint representation of the coordinate
public func convert(_ coordinate: CLLocationCoordinate2D, toPointTo: UIView?) -> CGPoint? {
guard let mapView else {
return nil
}
return mapView.convert(coordinate, toPointTo: toPointTo)
}

public init(mapView: MLNMapView,
lastReasonForChange: CameraChangeReason?)
{
/// Initialize with an MLNMapView (captures current values)
/// - Parameters:
/// - mapView: The MLNMapView to capture values from
/// - lastReasonForChange: The reason for the last camera change
public init(mapView: MLNMapView, lastReasonForChange: CameraChangeReason?) {
centerCoordinate = mapView.centerCoordinate
zoomLevel = mapView.zoomLevel
direction = mapView.direction
visibleCoordinateBounds = mapView.visibleCoordinateBounds
mapViewSize = mapView.frame.size
contentInset = mapView.contentInset
self.lastReasonForChange = lastReasonForChange
self.mapView = mapView
}

/// Initialize with explicit values (useful for testing)
/// - Parameters:
/// - centerCoordinate: The center coordinate
/// - zoomLevel: The zoom level
/// - direction: The compass direction
/// - visibleCoordinateBounds: The visible coordinate bounds
/// - mapViewSize: The size of the map view
/// - contentInset: The content inset
/// - lastReasonForChange: The reason for the last camera change
public init(
centerCoordinate: CLLocationCoordinate2D,
zoomLevel: Double,
direction: CLLocationDirection,
visibleCoordinateBounds: MLNCoordinateBounds,
mapViewSize: CGSize = CGSize(width: 320, height: 568),
contentInset: UIEdgeInsets = .zero,
lastReasonForChange: CameraChangeReason? = nil
) {
self.centerCoordinate = centerCoordinate
self.zoomLevel = zoomLevel
self.direction = direction
self.visibleCoordinateBounds = visibleCoordinateBounds
self.mapViewSize = mapViewSize
self.contentInset = contentInset
self.lastReasonForChange = lastReasonForChange
mapView = nil
}
}

Expand Down
Loading
Loading