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

Decide if all functions should always return normalized coordinates (within ±90deg/±180deg range) #46

Open
lukas-h opened this issue Dec 14, 2021 · 7 comments
Assignees
Labels

Comments

@lukas-h
Copy link
Member

lukas-h commented Dec 14, 2021

Relating to the new vector functions #23 – which don't have a "wraparound" conversion built-in. But may also be important more broadly:

I'm not sure if [coordinate normalization] belongs here or not, it sort of depends on the use cases. Is there a use case for getting
positions that are technically out of bounds or should the value we return from any function always be normalized to within the ±90deg/±180deg range?

Originally posted by @baparham in #43 (comment)

@lukas-h
Copy link
Member Author

lukas-h commented Dec 14, 2021

Right now, the coordinate types Position(lng, lat, alt) and BBox have a toSigned method to do the normalization.

But you have to explicitly call it. Nowhere in the library is this done automatically.

@lukas-h
Copy link
Member Author

lukas-h commented Dec 14, 2021

I found some older discussions, where this is mentioned: #4 (comment)

@lukas-h lukas-h added help wanted Extra attention is needed question Further information is requested labels Dec 14, 2021
@baparham
Copy link
Member

After giving this a small amount of thought, I would presume that most use cases center on real world geographical data, in which case, extending beyond the bounds of the standard ±90deg/±180deg range aren't much use.

There is probably a small use case to be argued for allowing non-normalized coordinates, particularly with abstract geo-manipulation, but I would posit that is the exception, not the rule.

With that said, my vote would be to make coordinates always normalized to within the standard range, and potentially provide a way to keep calculations and positions unsigned, i.e. non-normalized. How that is achieved, I don't really know, since normalization is "lossy" in effect, and you can never return to the pre-normalized state, UNLESS you keep that data around as part of the position and bbox objects for some future use, but I think it's better to wait for someone in the community to actually raise that as a valid use case before building it into the implementation.

@lukas-h
Copy link
Member Author

lukas-h commented Dec 15, 2021

100% agree

Maybe this could be done in the constructors of the coordinate types. Similarly to this approach maybe:
https://github.com/flutter-mapbox-gl/maps/blob/01a038d93560bebcf2170dabf32e44b3cdffafcc/mapbox_gl_platform_interface/lib/src/location.dart#L16

@lukas-h
Copy link
Member Author

lukas-h commented Dec 15, 2021

There is probably a small use case to be argued for allowing non-normalized coordinates, particularly with abstract geo-manipulation, but I would posit that is the exception, not the rule.

We could add a constructor Position.unsigned/Position.unbounded or similarly named.
This should flip a boolean flag for “this position is not normalized and should remain so”. With that we could re-implement the vector operations somehow like this:

Position operator +(Position other) => 
shouldRemainUnsigned
? Position.unsigned(lng + other.lng, lat + other.lat)
: Position(lng + other.lng, lat + other.lat);

@lukas-h
Copy link
Member Author

lukas-h commented Dec 19, 2021

I will start with an implementation attempt soon

@lukas-h lukas-h added this to the Improve GeoJSON Core milestone Mar 2, 2022
@lukas-h
Copy link
Member Author

lukas-h commented Mar 2, 2022

The decision was, to keep Position unsigned by default, but it can be converted to a signed coordinate easily!

The imagined behaviour with signed/unsigned is similar to DateTime, which can be local or UTC. The DateTime has conversion functions toLocal() and toUTC(), that return a new object. Internally it uses a boolean to save the state (local = true/false).

CC @baparham @tobrun

@lukas-h lukas-h self-assigned this Mar 10, 2022
lukas-h added a commit that referenced this issue Jun 20, 2022
@lukas-h lukas-h removed help wanted Extra attention is needed question Further information is requested labels Jun 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants