Skip to content

Fix/camera bug fixes #90

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 11 commits into from
May 9, 2025
Merged

Fix/camera bug fixes #90

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
5a347dd
feat: task based camera
Archdoog Apr 29, 2025
5496be1
Merge branch 'main' of github.com:Rallista/swiftui-dsl into fix/camer…
Archdoog Apr 29, 2025
1c1abee
fix: modern concurrency camera
Archdoog Apr 30, 2025
185343a
fix: modern concurrency camera
Archdoog Apr 30, 2025
4247897
Update MapViewCoordinatorCameraTests.swift
Archdoog May 1, 2025
f347a9e
Correcting task sleep
Archdoog May 2, 2025
e0ab880
Adds docs, reverts structure for private
Archdoog May 2, 2025
244d07e
Adds docs, reverts structure for private
Archdoog May 2, 2025
9d075fb
Additional camera fixes and improvements
Archdoog May 6, 2025
e0fd990
Additional camera fixes and improvements
Archdoog May 6, 2025
ae8cbdb
Merge branch 'fix/camera-bug-fixes' of github.com:Rallista/swiftui-ds…
Archdoog May 6, 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
69 changes: 27 additions & 42 deletions Sources/MapLibreSwiftUI/MapViewCoordinator.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@
layer is MLNSymbolStyleLayer
}

for layerSpec in parent.userLayers {

Check warning on line 119 in Sources/MapLibreSwiftUI/MapViewCoordinator.swift

View workflow job for this annotation

GitHub Actions / platform=iOS Simulator,name=iPhone 16,OS=18.1 

main actor-isolated property 'userLayers' can not be referenced from a nonisolated context; this is an error in the Swift 6 language mode
// DISCUSS: What preventions should we try to put in place against the user accidentally adding the same layer twice?
let newLayer = layerSpec.makeStyleLayer(style: mglStyle).makeMLNStyleLayer()

Expand Down Expand Up @@ -182,10 +182,13 @@

// Cancel any existing camera update completion task.
cameraUpdateTask?.cancel()
cameraUpdateContinuation = nil

cameraUpdateTask = Task { @MainActor in
return await withCheckedContinuation { continuation in
// Clean up the continuation if it was already set.
cameraUpdateContinuation?.resume()
cameraUpdateContinuation = nil

Copy link
Contributor

Choose a reason for hiding this comment

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

This looks good!

// Store the continuation to be resumed in mapViewDidBecomeIdle
cameraUpdateContinuation = continuation

Expand Down Expand Up @@ -234,35 +237,27 @@

mapView.userTrackingMode = .follow

mapView.setZoomLevel(zoom, animated: false)
mapView.zoomLevel = zoom
mapView.direction = direction

mapView.minimumPitch = pitch
mapView.maximumPitch = pitch
mapView.minimumPitch = pitchRange.rangeValue.lowerBound
mapView.maximumPitch = pitchRange.rangeValue.upperBound

} else {
mapView.setUserTrackingMode(.follow, animated: animated) {
guard mapView.userTrackingMode == .follow else {
// Exit early if the user tracking mode is no longer set to follow
return
}

mapView.zoomLevel = zoom
mapView.direction = direction

mapView.minimumPitch = pitch
mapView.maximumPitch = pitch
mapView.minimumPitch = pitchRange.rangeValue.lowerBound
mapView.maximumPitch = pitchRange.rangeValue.upperBound
let camera = mapView.camera
camera.heading = direction
camera.pitch = pitch

let altitude = MLNAltitudeForZoomLevel(
zoom,
pitch,
mapView.camera.centerCoordinate.latitude,
mapView.frame.size
)
camera.altitude = altitude
mapView.setCamera(camera, animated: animated)
}
}
case let .trackingUserLocationWithHeading(zoom: zoom, pitch: pitch, pitchRange: pitchRange):
Expand All @@ -272,32 +267,26 @@
// Needs to be non-animated or else it messes up following

mapView.userTrackingMode = .followWithHeading
mapView.setZoomLevel(zoom, animated: false)

mapView.zoomLevel = zoom

mapView.minimumPitch = pitch
mapView.maximumPitch = pitch
mapView.minimumPitch = pitchRange.rangeValue.lowerBound
mapView.maximumPitch = pitchRange.rangeValue.upperBound

} else {
mapView.setUserTrackingMode(.followWithHeading, animated: animated) {
guard mapView.userTrackingMode == .followWithHeading else {
// Exit early if the user tracking mode is no longer set to followWithHeading
return
}

mapView.zoomLevel = zoom

mapView.minimumPitch = pitch
mapView.maximumPitch = pitch
mapView.minimumPitch = pitchRange.rangeValue.lowerBound
mapView.maximumPitch = pitchRange.rangeValue.upperBound
let camera = mapView.camera

let altitude = MLNAltitudeForZoomLevel(
zoom,
pitch,
mapView.camera.centerCoordinate.latitude,
mapView.frame.size
)
camera.altitude = altitude
camera.pitch = pitch
mapView.setCamera(camera, animated: animated)
}
}
case let .trackingUserLocationWithCourse(zoom: zoom, pitch: pitch, pitchRange: pitchRange):
Expand All @@ -307,36 +296,32 @@
// so let's do something else instead.
// Needs to be non-animated or else it messes up following

mapView.setZoomLevel(zoom, animated: false)
mapView.zoomLevel = zoom

mapView.minimumPitch = pitch
mapView.maximumPitch = pitch
mapView.minimumPitch = pitchRange.rangeValue.lowerBound
mapView.maximumPitch = pitchRange.rangeValue.upperBound

} else {
mapView.setUserTrackingMode(.followWithCourse, animated: animated) {
guard mapView.userTrackingMode == .followWithCourse else {
// Exit early if the user tracking mode is no longer set to followWithCourse
return
}

mapView.zoomLevel = zoom

mapView.minimumPitch = pitch
mapView.maximumPitch = pitch
mapView.minimumPitch = pitchRange.rangeValue.lowerBound
mapView.maximumPitch = pitchRange.rangeValue.upperBound

let camera = mapView.camera

let altitude = MLNAltitudeForZoomLevel(
zoom,
pitch,
mapView.camera.centerCoordinate.latitude,
mapView.frame.size
)
camera.altitude = altitude
camera.pitch = pitch
mapView.setCamera(camera, animated: animated)
}
}
case let .rect(boundingBox, padding):
mapView.minimumPitch = 0
mapView.maximumPitch = 0
mapView.direction = 0

mapView.setVisibleCoordinateBounds(boundingBox,
edgePadding: padding,
animated: animated,
Expand Down Expand Up @@ -401,7 +386,7 @@
MainActor.assumeIsolated {
// regionIsChangingWith is not called for the final update, so we need to call updateViewProxy
// in both modes here.
updateViewProxy(mapView: mapView, reason: reason)

Check warning on line 389 in Sources/MapLibreSwiftUI/MapViewCoordinator.swift

View workflow job for this annotation

GitHub Actions / platform=iOS Simulator,name=iPhone 16,OS=18.1 

sending 'self' risks causing data races; this is an error in the Swift 6 language mode

guard let changeReason = CameraChangeReason(reason) else {
// Invalid state - we cannot process this camera change.
Expand Down
12 changes: 6 additions & 6 deletions Tests/MapLibreSwiftUITests/MapViewCoordinator/MapViewCoordinatorCameraTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,8 @@ final class MapViewCoordinatorCameraTests: XCTestCase {
.setCalled(1)

verify(maplibreMapView)
.setZoomLevel(.value(10), animated: .value(false))
.called(1)
.zoomLevel(newValue: .value(10))
.setCalled(1)
}

@MainActor func testUserTrackingWithCourseCameraUpdate() async throws {
Expand Down Expand Up @@ -219,8 +219,8 @@ final class MapViewCoordinatorCameraTests: XCTestCase {
.setCalled(1)

verify(maplibreMapView)
.setZoomLevel(.value(10), animated: .value(false))
.called(1)
.zoomLevel(newValue: .value(10))
.setCalled(1)
}

@MainActor func testUserTrackingWithHeadingUpdate() async throws {
Expand Down Expand Up @@ -264,8 +264,8 @@ final class MapViewCoordinatorCameraTests: XCTestCase {
.setCalled(1)

verify(maplibreMapView)
.setZoomLevel(.value(10), animated: .value(false))
.called(1)
.zoomLevel(newValue: .value(10))
.setCalled(1)
}

// TODO: Test Rect & Showcase once we build it!
Expand Down
Loading