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

MapViewPort implementation to listen to raw values of 'camera' #33

Merged
merged 2 commits into from
Apr 16, 2024

Conversation

Archdoog
Copy link
Collaborator

@Archdoog Archdoog commented Apr 8, 2024

  • Adds MapViewPort concept for modifier callback style updates of the MapViewPort (think the actual rawValue of the camera even when the state camera is set to an automatic mode like tracking).

@Archdoog Archdoog requested a review from ianthetechie April 9, 2024 03:14
@Patrick-Kladek
Copy link
Contributor

I'm trying to implement a marker where the underlying map can be moved around to precisely select a coordinate. The same way as Uber allows you to select a pickup/drop-off location or Apple Maps when navigating around in StreetView.

So this PR is greatly appreciated from me. Your concept here is basically the same as:

.unsafeMapViewModifier { mapView in
    mapView.centerCoordinate
}

but exposes it via a Modifier instead of raw accessing the MLNMapView, right? Both solutions only run the callback when the map stops moving - I've tried that before I found this PR.

Now my question do you know how to get continuous updates while the map is moving and can we add this as a parameter to this modifier? Something like this:

enum ViewPortUpdateMode {
    case continuous
    case didChange
}

.onMapViewPortUpdate(mode: .continuous, { mapViewPort in
    print(mapViewPort.center)
})

I don't mind submitting another PullRequest with these changes, however some help would be appreciated.

@Archdoog
Copy link
Collaborator Author

I'm trying to implement a marker where the underlying map can be moved around to precisely select a coordinate. The same way as Uber allows you to select a pickup/drop-off location or Apple Maps when navigating around in StreetView.

So this PR is greatly appreciated from me. Your concept here is basically the same as:

.unsafeMapViewModifier { mapView in
    mapView.centerCoordinate
}

but exposes it via a Modifier instead of raw accessing the MLNMapView, right? Both solutions only run the callback when the map stops moving - I've tried that before I found this PR.

Now my question do you know how to get continuous updates while the map is moving and can we add this as a parameter to this modifier? Something like this:

enum ViewPortUpdateMode {
    case continuous
    case didChange
}

.onMapViewPortUpdate(mode: .continuous, { mapViewPort in
    print(mapViewPort.center)
})

I don't mind submitting another PullRequest with these changes, however some help would be appreciated.

I can definitely see value for configuring a .continuous, .changeDidEnd or even debounced/throttled sequence of updates throughout a map view change event. My main motivation in using the end only was to reduce the potential number of calls per camera change event to reduce CPU usage, etc. Though I didn't actually test it with Xcode instrumentation. (I was playing with a similar thing issue here https://github.com/Rallista/maplibre-compose-playground and found that callback updates continuously dramatically impacted device cpu, from around 10-20% to 100-200%).

I'm all for taking enhancing this PR if we can prove that quality isn't impacted with cpu/power consumption in another PR.

I'm also all for adding additional 'ViewPort' related information in the MapViewPort type. It's meant to be a read only of whatever's needed to correctly understand what the map is displaying 👍

@ianthetechie
Copy link
Collaborator

Agreed; we would definitely welcome PRs to add the functionality you want @Patrick-Kladek as long as they are not creating a persistent overhead (especially important when not actually used by the end dev; obviously if the process inherently creates overhead then we can simply document that in the docstrings; instrument profiling would be appreciated with a PR).

@ianthetechie ianthetechie merged commit c7fc6a0 into maplibre:main Apr 16, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants