-
-
Notifications
You must be signed in to change notification settings - Fork 860
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
feat: allow polylines & polygons to cross world boundary #1969
base: master
Are you sure you want to change the base?
Conversation
Impacted files * `crs.dart`: new methods `getHalfWorldWidth` and `projectList` * `painter.dart`: refactored using pre-computed `List<Double>` * `polygon.dart`: added an example around longitude 180 * `polyline.dart`: added an example around longitude 180 * `polyline_layer.dart`: we don't cull polylines that go beyond longitude 180 * `projected_polygon.dart`: using new method `Projection.projectList` * `projected_polyline.dart`: using new method `Projection.projectList`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the pull request @monsieurtanuki! I did some quick testing and your added examples work like intended (thanks for adding them).
Some additional thoughts:
The main use case of line wrapping are paths that cross the -180/180 longitude for example when we want to display flights. We need to figure out a solution when the user follows a polyline or polygon accross the world border. Currently the line suddenly disapears. This, of course, is not in scope for this this pull request. (:
line.wrapping.mp4
Oh it's a bug then! Probably related to the "teleportation" issue. |
I actually don't think it's an bug. The line just moves to the new world that comes into center. So the line is still active but basically on the other side of the world. If you plan to look into this as well how it can get improved that's awesome of couse. (: |
@josxha Your description is correct: it's the "teleportation" issue (I can see a unique polyline jumping from world to world on the same zoom 0 map) but when the zoom is high - the "other" world is not visible. I still think it's a bug, probably not a new bug, but a fair user expectation is to see at least one polyline. Working on it. The current PR is relatively small (7 files changed), so I'll see how review-able it would become when including that bug fix. |
Impacted files: * `offsets.dart`: new method `getAddedWorldWidth`, used to add/subtract a world width in order to display visible polylines * `painter.dart`: minor fix, as now we may unproject coordinates from the wrong world
@josxha Fixed! One visible copy of the polyline, with a small or a large zoom. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM in general! Some documentation about what the whole 'half world width' stuff is about would be nice. I'll leave to @josxha to give final review :) Thanks!
Just a thought, what happens with |
Nothing. |
Impacted files: * `crs.dart`: replaced "half world width" with "world width", in order to avoid answering to the question "why HALF?" * `offsets.dart`: now we display the occurrence closer to the screen center; minor refactoring * `painter.dart`: minor fix regarding side-effects on `_metersToStrokeWidth` * `polyline_layer.dart`: now computes the limits projected from -180 and 180 instead of "half world width" * `projected_polyline.dart`: moved code to `polyline_layer.dart`
@JaffaKetchup @josxha I've just pushed some changes that make the code (somehow) more readable. /// Returns the width of the world in geometry coordinates.
///
/// Is used at least in 2 cases:
/// * my polyline crosses longitude 180, and I somehow need to "add a world"
/// to the coordinates in order to display a continuous polyline
/// * when my map scrolls around longitude 180 and I have a marker in this
/// area, the marker may be projected a world away, depending on the map being
/// centered either in the 179 or the -179 part - again, we can "add a world"
double getWorldWidth() {
final (x0, _) = projectXY(const LatLng(0, 0));
final (x180, _) = projectXY(const LatLng(0, 180));
return 2 * (x0 > x180 ? x0 - x180 : x180 - x0);
} |
Sorry, I won't find time to review it this week. 😶 I'll try to review it next week if no one gets ahead of me. (: |
The review should probably be put on hold until the fix for #1976 is coded (by me) (like, today), reviewed and merged. |
What
Polyline
Results In Unexpected Lines #1338)Screenshots
Impacted files
crs.dart
: new methodsgetHalfWorldWidth
andprojectList
painter.dart
: refactored using pre-computedList<Double>
polygon.dart
: added an example around longitude 180polyline.dart
: added an example around longitude 180polyline_layer.dart
: we don't cull polylines that go beyond longitude 180projected_polygon.dart
: using new methodProjection.projectList
projected_polyline.dart
: using new methodProjection.projectList