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

Define EPSG:4326 projection for eox-map configuration parameters #1093

Closed
santilland opened this issue Jul 15, 2024 · 8 comments · Fixed by #1181
Closed

Define EPSG:4326 projection for eox-map configuration parameters #1093

santilland opened this issue Jul 15, 2024 · 8 comments · Fixed by #1181
Assignees
Labels
enhancement New feature or request map

Comments

@santilland
Copy link
Member

When using different projections the center property and extent functions expect the currently used projection in the map. This makes setting and keeping track of these values somewhat difficult (when switching projections), needing to make sure externally to always re-project properties to the currently used projection.
I would propose to always expect the use of EPSG:4326 for center property, as well as for zoomExtent and related functions where extent or bbox can be set making sure the re-projection to the currently used projection is done internally.

@RobertOrthofer
Copy link
Contributor

This does make sense for special projections, at the moment we have some convenience logic for detecting both EPSG:4326 and EPSG:3857, in order to keep it as simple as possible for the most basic use cases.

I feel like this change might mean a lot of changes in many parts in both code and API, and it also would mean that EOxMap.center would give a different result than EOxMap.map.getCenter(), I'm not sure if this is the expected behaviour. I think a user that has their data in a certain projection is more likely to also set the center in the same system.

Maybe we can offer some additional attributes, like lonLatCenter and lonLatExtent, that only have getters? Or maybe just an exported helper function to offer conversions between the registered projections?

@santilland
Copy link
Member Author

Thanks for the feedback on that, those are some good and important points.
I like the lonLatCenter and lonLatExtent idea, but not sure if we would need only getters, could we not also allow it for setting values?
The helper function could also be interesting, something like, fromNativeMapProj and toNativeMapProj maybe?

@silvester-pari what do you think?

@RobertOrthofer
Copy link
Contributor

true, we could also allow setters, I'm slightly worried about infinite update loops if there is a conversion error between the projections, but we are already handling that issue, albeit by very simple means that might fail at some weird coordinate systems...

@santilland
Copy link
Member Author

Maybe a combination would be the best approach, having lonLatCenter and lonLatExtent as getters only, and have a lonLatCenterToNative and lonLatExtentToNative helper function?

@santilland
Copy link
Member Author

@silvester-pari @RobertOrthofer if we agree on the approach commented on my previous comment it would be great if we can consider looking into this

@RobertOrthofer
Copy link
Contributor

RobertOrthofer commented Jul 22, 2024

i'd vote in favor of this approach, but we could also make it more versatile and allow transformations between any registered coordinate systems. Basically just exporting the vanilla ol functionalities transform and transformExtent, and get the current coordinate identifier via the element

@santilland
Copy link
Member Author

I think exporting transform and transformExtent makes a lot of sense, it would allow us to use any registered/available projection. If @silvester-pari does not oppose i would say let's implement this, so:
Having lonLatCenter and lonLatExtent and exporting the transform and transformExtent.

@silvester-pari
Copy link
Collaborator

Sounds good to me, just a note on exporting methods; let's offer them as a dedicated export and not as an eox-map function/property; same as for getFlatLayersArray() described here: #974 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request map
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants