-
Notifications
You must be signed in to change notification settings - Fork 91
Increase precision of points, lines and shapes by translating them clo… #138
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
Increase precision of points, lines and shapes by translating them clo… #138
Conversation
|
Hello, |
|
Hi @kkdd. The solution in this pull request doesn't change the shaders in any way and will have a negligible impact on performance as it is only changing a few lines of code. It should be enough for most use cases. Whatever the case, this is what I am using to plot high precision points, so thought I'd share it with the group. |
|
Hello, RayLarone. Thank you for your comment. |
|
This PR breaks unit tests. Could you fix those, and commit? Then I can merge. |
|
Hi @robertleeplummerjr. Sure, although apologies, I'm a data analyst not a programmer so maybe I'm not fully understanding the unit testing. What do you mean by this pull request breaks unit tests? When I clone this branch, run How can I see which unit tests need addressing so I can fix the issues? |
|
Perhaps I had a premature failure when I tested it. I'm away this week but next I'll look into merging. |
|
@robertleeplummerjr please look into merging again :) |
|
The build and tests run without errors for me. |
|
If the map is initialised at [0, 0], for example, rendered with glify, and then panTo the shapes at [50, 0] far from [0, 0], the mapCenterPixels won't be updated and the polygons will be distorted. Are we sure map.getCenter() is the right way, if glify won't update and re-render? For optimisation, If any shapes are visible in the map, the centre point should be close to the shapes themselves. |
|
@shenzhuxi Yes, you are correct. I suggested this method with the assumption it is unlikely people would need centimetre precision for layers spanning multiple countries/continents (although it's not out of the question). And having the the origin at map.getCentre() was better than where it was by default. A better approach would probably be to have a different origin at the centre of each layer so layers plotted in different parts of the world will retain a local precision. I have subsequently found out that deck gl has identified a similar problem and includes a solution with different coordinate systems like "lnglat_offsets" (https://deck.gl/docs/developer-guide/coordinate-systems) where you are required to input the "coordinateOrigin" yourself. Maybe something to model this off. |
|
@RayLarone We tested using each shape's centre point instead of map.getCenter() and it seems work fine. |
|
@shenzhuxi
|
…ser to the Pixel Origin.
See this issue (#137) for an explanation of the code.
This branch aims to resolve #137, #54 and #119.
This is using the map center (
map.getCenter()) as a best guess at where the "precision center" should be located.Other options could be to use the data/geojson centroid, or to pass in a LatLng pair as an argument.