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

[FEATURE] Better support for cartesian coordinate systems #1386

Open
3 tasks done
noelex opened this issue Oct 17, 2022 · 13 comments
Open
3 tasks done

[FEATURE] Better support for cartesian coordinate systems #1386

noelex opened this issue Oct 17, 2022 · 13 comments
Labels
feature This issue requests a new feature Fund Open for a bounty with Polar.sh! P: 4 (far future) S: core Scoped to the core flutter_map functionality

Comments

@noelex
Copy link

noelex commented Oct 17, 2022

What do you want implemented?

I'm currently writing an app which uses laser scanned 2D images as maps, and I would like to allow user to interact with these maps using flutter_map.

The problem is that these maps use cartesian coordiantes (measured in meters) instead of geographical coordinates. Although I can write a custom CRS to project map coordinates to pixels, but LatLng and LatLngBounds won't allow me to specify coordinates beyond 'geographical' boundaries ([-90, 90] for latitude and [-180, 180] for longitude).

Thus I think having a more generic version of 'LatLng' would be very helpful for users which doesn't use geographical coordinate systems.

What other alternatives are available?

There are two ways to workaround this:

  1. Write a transformation which would scale the cartesian coordiantes to fit the 'geographical' range. This is rather error-prone and may incur loss of precision.
  2. Inherit from LatLon and bypass those boundary checks. This is easy to implement and works fine. But there are some methods and properties in LatLon (e.g. longitude and latitudeInRad) makes no sense for cartesian coordinates and may cause confustions.

Can you provide any other information?

No response

Platforms Affected

Android, iOS, Web, Windows, MacOS, Linux, Other

Severity

Annoying: Currently have to use workarounds

Requirements

Bounty

flutter_map uses Polar.sh to provide a bounty on this issue! Contributors who resolve this issue will receive the full amount pledged below, after all fees have been paid.

The flutter_map team funds issues we want to see fixed but are relatively low priority for us, and we do this using your donations! When you donate, 15% goes to the OpenStreetMap Foundation, and the rest is held as donations, some of which we fund these issues with!

Of course, we appriciate any size donation you can make: it supports us directly, and allows us to fund these issues, but if this issue is particularly important to you, you can pledge money directly to this issue using the button below.


Fund with Polar
@noelex noelex added the feature This issue requests a new feature label Oct 17, 2022
@JosefWN
Copy link
Contributor

JosefWN commented Oct 28, 2022

I also work in cartesian coordinates, where flutter_map admittedly struggles a bit. Coincidentally I've also worked with LiDAR mapping a bit, but not with flutter_map specifically. I would say that these are all geographical coordinates, although without an expensive INS attached to your LiDAR they will be in a local coordinate system.

I don't think a more generic LatLng will help much, these coordinates are elliptical, which by definition constrains them in the way that you describe. There are already cartesian CRS such as EPSG:3413 (see examples), which are defined in meters. Like you are suggesting, you could then work in projected coordinates using CustomPoint rather than LatLng.

Here comes the challenging part which you are describing: our coordinates are CustomPoint, but most of flutter_map expects LatLng. What I ended up doing was forking the layers I use in my application to work exclusively in cartesian coordinates.

This has a couple of benefits:

  1. Complexity: Working in cartesian coordinates is typically much easier code-wise, especially if you need to manipulate or derive statistics (such as bounds) from the coordinates in some way.
  2. Performance: No need to reproject continuously from CustomPoint to LatLng and back to CustomPoint, all my coordinates are cartesian and remain so (unless the user wants a specific LatLng for a given CustomPoint).

I also have some rather unique use-cases so in my case forking was probably inevitable anyway. I could have contributed some of my features, but I felt it would only pollute the flutter_map code base. Most people are perfectly fine with LatLng after all.

I think this is an interesting discussion though. Being able to use either LatLng or CustomPoint with flutter_map (layers, MapController etc.) would indeed be nice. I just haven't come up with a suggestion that maintains the fairly lean API flutter_map has but still caters to our cartesian needs as well.

EDIT: Perhaps a first step would be having the layers implemented using CustomPoint (ex. OverlayImageLayerRaw) and then have wrapping layers taking LatLng and projecting to CustomPoint (ex. OverlayImageLayer). That wouldn't cause duplicated code or excessive overhead, to my knowledge it wouldn't break existing functionality, and it would allow for either CustomPoint or LatLng to be supplied.

@JaffaKetchup
Copy link
Member

@JosefWN Good description of the situation, thanks :)

We are always happy to see new PRs, but as you said yourself, this would probably 'pollute' this project quite a bit; we are trying to keep things simple here for now.

Just as a current example, take #1390. However great it might end up being (and is right now) (we are still discussing over on Discord), if it grows too big or starts to do too much for the end developer, it might be decided unsuitable for the core library here.
The current AttributionWidget strikes a good balance between providing for those devs that just 'want to get on with their life :D' and keeping things minimal. However, it does not cater in any way to a more advanced scenario, like #1390 is attempting to do.

It is a very similar situation here. Yes, it would be nice to have this. But would it add significant amounts of code: yes. This situation is further weighed down by this being a fairly niche use-case (I think you'll agree), compared to the new attribution system having potential for usage in/on all applications.

Of course, a plugin might be a good way to go about this, but this is getting into deep discussion territory. So I'd better invite @ibrierley, @moonag, and @TesteurManiak along! Sorry guys :D

@TesteurManiak
Copy link
Collaborator

Well, I'm not objective for this kind of thing as I've always used LatLng on the different projects I've been working on and never felt the need to use cartesian coordinates. Like @JaffaKetchup said I feel like this is a niche use-case.
I think that if a feature only answers very specific needs then it might be better off as a separated plugin. If the current flutter_map public API is not enough to create such plugin I guess that we could try to adapt it for an easier usage.

@ibrierley
Copy link
Collaborator

I was trying to figure if CRS.simple could be used (or extended) in some ways...has anyone tried this (my suspicion would be something like a bounds/LL issue would get in the way like the original post mentioned, but it could be interesting to see if theres a way there...).

It may be helpful to have a specific example people can look at and ponder.

@mootw
Copy link
Collaborator

mootw commented Oct 28, 2022

We could look at creating an extension for LatLng that allows working with meters or a constructor for latlng that takes meters. I am not super familiar with multiple CRS. Or this might be something you can do in your own project using either of these methods?

@JosefWN
Copy link
Contributor

JosefWN commented Oct 28, 2022

We could look at creating an extension for LatLng that allows working with meters or a constructor for latlng that takes meters.

Not sure how that would work without every LatLng also holding a reference to the CRS, which feels messy. I also think it would be confusing to work with elliptical coordinates as if they were planar?

@ibrierley
Copy link
Collaborator

I suspect those with more experience on CRS side will need to expand a little more of give specific examples of things that don't work.

I.e my understanding is that a custom crs basically has projection/unprojection and transformations. So with the original post, what is the issue with having a custom crs that is essentially CRS.simple, but with a transformation that scales by a factor of say 1000 (to allow the values to fit within a LatLng max +/-180,90 ? (it sure feels like bending something into a solution not quite right, but I'd like to understand nontheless).

Again, I may well be misunderstanding some core concepts, but it may be good to discuss those, so it gives us some ideas of potential solutions (or if it simply can't be without too much complexity, at least its been considered reasonably). I have some suspicions where some of the flaws are, but it would be good to hear from those who have tried and got stuck what issues are hit.

@JosefWN
Copy link
Contributor

JosefWN commented Nov 22, 2022

Other than the bugs I have reported relating to planar projections (such as polar stereographic projection), I think my main gripe is the API's are only intended for LatLng rather than CustomPoint, which my whole app is built around. Working in LatLng is not really feasible since I use projection-specific math quite extensively, so I would have to continuously unproject and reproject a number of times every frame.

Working only with CustomPoint still causes a bunch of needless projections which convolutes my code, also becoming non-trivial from a performance perspective. Remember: I have to project to plug things into the layer as LatLng, which then unprojects what I just projected, this is roughly twice the performance overhead over working in LatLng, rather than enjoying the performance benefits that should be achieved by working natively with the CRS.

One thing we could do to get around that is to have two tiers of layers as I think I suggested before. Take the polyline layer for example, we could have a PolylineLayer (LatLng) and a PolylineProjectedLayer (CustomPoint).

Ultimately, since everything is being projected at the end of the day, the bulk of the code would be in the PolylineProjectedLayer. The PolylineLayer would only project the coordinates, and return a PolylineProjectedLayer in its build method. I could then use PolylineProjectedLayer in my code, sidestepping the needless projections altogether.

I don't know if it's feasible, but just food for thought...

@ibrierley
Copy link
Collaborator

I'm still a bit unsure about what the problem is with using a custom crs like crs.simple to get around reprojections ?

@JosefWN
Copy link
Contributor

JosefWN commented Dec 13, 2022

What you are suggesting can potentially work in the most trivial use cases, but it's working around the API:s, not with them. In my use case I have an actual CRS where this would not work, either. At least far from trivially (since there are occasions where I actually need a LatLng to present to the user).

Maybe I'm convoluting this issue, but what triggered me was the issue author's need to use LatLng without having a LatLng. My problem isn't so much with the CRS itself but what comes next.

The challenge I think I share with LiDAR data is that we process a lot of points, and we are not necessarily talking LatLng now, but more typically CustomPoint (be it in CRS.simple or in any other planar CRS). My point sets are more modest when compared to the millions and millions of points that LiDAR point clouds can contain, only in the tens of thousands, but they can potentially change frame-by-frame, processing millions of points over just a couple of seconds, of which I'm only interested in the points in the context currently requested by the user.

I'm working in maritime, in ice infested waters. Not just the vessels are moving, but the entire context, even the "ground" (the ice). Very non-linear tracks (hard to simplify) which stretch with ice drift, satellite imagery is moving and so on. The user can then move back and forth by scrolling a timeline, to see what conditions were like, say, 5 hours ago, and what they will be like 10 hours from now. This makes it possible for the user to continuously see on the screen how conditions change over time, which creatives a more intuitive understanding of weather, wind, ocean currents, pressure gradients etc. and their impact on the vessels and the offshore operation at large.

At first thought, the fact that the points are already projected is great, right? Because under the hood, every layer is using (scaled) projected points to draw things on the map... but if I want to use a PolylineLayer, how do I do that with all my CustomPoints? Caching can work to a lesser degree, but the code to manage the caches would be quite complex for me since my setting is very dynamic. Since I can't cache every possible outcome of user input, I would have to employ multiple caching strategies, and even then I wouldn't feasibly be able to cover all cases, which is why I've leaned toward optimizing the performance instead.

With LatLng-based API like we have now, I'm forced to unproject to LatLng and then the layer projects back to CustomPoint again. But I don't have any LatLng in my code base.

That being said, it's not impossible of course. Just pay the price of unprojections and reprojections. In my case, it's pretty tough however, since my layers are continuously and dynamically moving on the map, and there is quite a bit of math that is far easier to do in a cartesian coordinate system. Even without the unprojections and reprojections I'm optimizing performance every few months as the code base is growing, because I'm constantly on the brink of the frame budget on the highest-end tablets and mid-end laptops for more advanced use-cases.

I'm now maintaining 12 layers/plugins in my code, at least half of which are forked from flutter_map either because they don't support polar CRS (such as the scale bar and the image layer), and/or because they don't support CustomPoint input. The AnimatedMapController mentioned in #1263 (comment) also doesn't currently support polar CRS, since it's relying on LatLngBounds. Essentially anything that does will not work in at least a polar CRS because the LatLngBounds make assumptions about the CRS that don't hold under this type of CRS, such as north being "up" and west being "left".

I do agree that me and @noelex's use cases are somewhat niche, and what I said in the tile layer discussion applies here too: weighing a smaller change that could help a few, albeit a lot, against the cost it would have for everyone else. So I'm not entirely sure we need to change anything.

All I'm saying is that there are ways to make my life much easier, which as a result would also make at least some other peoples lives easier and more hack-free, assuming that they work with large point-sets in a planar CRS.

@ibrierley
Copy link
Collaborator

I think what would be useful is a kind of minimal case that highlights the problem and can't be done easily with the existing crs options. Some basic code which says "I have to do this, but its inefficient and too complex".

Then it's a bit easier to ponder the existing code and if it could be either bent or rewritten to deal with it.

@github-actions

This comment was marked as off-topic.

@github-actions github-actions bot added the Stale label Jan 14, 2023
@github-actions

This comment was marked as off-topic.

@JaffaKetchup JaffaKetchup reopened this Jan 19, 2023
@JaffaKetchup JaffaKetchup added Fund Open for a bounty with Polar.sh! S: core Scoped to the core flutter_map functionality labels Jul 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature This issue requests a new feature Fund Open for a bounty with Polar.sh! P: 4 (far future) S: core Scoped to the core flutter_map functionality
Projects
Status: To do
Development

No branches or pull requests

6 participants