New Features!

[matafokka]

Hello, @henrythasler! As promised, here are new features, improvements and bug fixes:

1. Replaced midpoint algorithm with intermediate point algorithm which greatly improves performance and makes new features possible.
2. Added segmentsNumber and breakPoints options to give the user precise control over segments.
3. steps option is now deprecated in favor of options above.
4. Added moveNoWrapTo option to control where the first point will be.
5. Implemented feature I call natural drawing. It's a line that goes exactly between two points with all revolutions around the Earth. This is intended for the users that are not familiar with what exactly a geodesic line is. This is how it works:

6. Added updateStatisticsAfterRedrawing option. Setting it to false will turn off automatic statistics calculation which will improve performance.
7. Now statistics contains spherical length in radians and meters.
8. Implemented changing line length.
9. Changed internal interfaces to support these features.
10. Fixed line splitting not working when one point has -180 longitude and the other - +180.
11. Fixed edge case when longitude difference is 180 degrees, and absolute values of latitudes are equal (for example, [[80, 0], [40, 180]] coordinates). Previous algorithm handled it okay-ish, but new algorithm and my fix produces П-shaped line as it should be.
12. Added tests for all this stuff.

Henry, I need your help with the following:

1. Please, check out API changes. Some of it is a bit messy, docs require better wording. Can you make it better?
2. Statistics other than spherical lengths doesn't account revolutions when natural drawing is used. I'll look into this issue, but I'll prioritize changing length over it. Please, try to think something about it too.

[lgtm-com[bot]]

This pull request introduces 1 alert when merging 22612454c5763b1a75e533bd6dbcda21ec126032 into e9dc41a653662e8d0bd5d50d0d2dbc73a5e0c7b6 - view on LGTM.com

new alerts:

• 1 for Unused variable, import, function or class

[matafokka]

Some notes on intermediate point algorithm.

1. To follow great circle another way around (i.e. big part), change distance like so: d = Math.PI * 2 - d.
2. Natural drawing is possible with another method - copying lines that follow small and big part one-by-one until we reach the end point. However, it has some issues with antimeridians, and segments' lengths are not constant.
3. It's possible to change length (get intermediate point beyond 100% of the length) to get line with distance higher than 180 degrees. It requires using a big part until 360 degrees, and stacking the parts when length is above 360. Essentially, it creates natural drawing line.
4. It should be possible to change length from the start without swapping points by changing sign of some stuff in the formulas. I'm not doing it since it'll be harder to maintain.

A note in general. Since geodesic line looks like a sine function in WebMercator, it should be possible to create a trig function and draw directly on the canvas. This will produce 100% accurate and smooth line. But this is for another time and another library, if someone would care enough to implement this.

[matafokka]

By the way, can you add a linter, so editors and IDEs won't mess the code up, and it'll be possible to fix in one click or command?

[matafokka]

I found the true cause of why changeLength() doesn't work on [[19, -200], [45, 10 - 720]]. While drawing a small line, I change its length too. However, since new length exceeds 180 degrees, the fraction of last point "resets". I'm gonna work on the fix.

[henrythasler]

You have obviously been very busy. :-D I think this will take a while for me to sort this out.

[matafokka]

You have obviously been very busy. :-D I think this will take a while for me to sort this out.

Sure I was :D

I also fixed changeLength(), yay!

[lgtm-com[bot]]

This pull request introduces 1 alert when merging 85eac2d70442c4a03cb42889125b9126be9cb6f3 into e9dc41a653662e8d0bd5d50d0d2dbc73a5e0c7b6 - view on LGTM.com

new alerts:

• 1 for Unused variable, import, function or class

[lgtm-com[bot]]

This pull request introduces 2 alerts when merging 07db344511ccc3e050dd5081a2bef14ac98f36b9 into e9dc41a653662e8d0bd5d50d0d2dbc73a5e0c7b6 - view on LGTM.com

new alerts:

• 2 for Unused variable, import, function or class

[matafokka]

Goodness, we definitely need a linter for both correct reformatting and this:

2 for Unused variable, import, function or class

[matafokka]

Here you go. I've tried to enforce your style, you may want to edit the rules yourself. By the way, be sure to run npm run lint and check the warnings.

[henrythasler]

Goodness, we definitely need a linter for both correct reformatting and this:

2 for Unused variable, import, function or class

I have SonarCloud to do the linting (besides other things). Not sure what additional benefits eslint provides.

[matafokka]

Not sure what additional benefits eslint provides.

Both IDEA and VSCode messed up formatting. ESLint allowed them to format the code right. That's the main reason to use it.

Most existing formatting issues can be fixed by running one command. I already did that.

It provides a nice list with the issues that can't be fixed automatically, so you can quickly fix it yourself. No need to run cloud-based checks and wait for the report. LGTM, for example, takes 3 minutes, ESLint takes 3 seconds.

If SonarCloud can do the same, please, add such commands, and we can use it instead of ESLint.

[matafokka]

I actually got curious and checked what exactly SonarCloud does. Well, it's neat but different from what's linter for. Linter enforces certain coding style by rules. For example, I've enforced double quotes while SonarCloud didn't say a thing about different quotes throughout the code.

[matafokka]

Sorry for this many comments. I've misread your comment about sonar. Yes, they do have a linter. To compare, eslint has following features that sonarlint seem to lack:

1. Config that can be shared with the repo.
2. CLI tools.

[matafokka]

@henrythasler, hello, sorry to bother you, but have you checked this PR? Please, let me know if you're fine at least with the public API. My project depends on some of these changes, and I want to finally finish it :D

[henrythasler]

Well, I'm not sure right now...

A lot of things were changed with this PR and some are probably very specific for your application (e.g. the natural-drawing feature) or personal taste (es-lint) which is totally OK. This means I'm not comfortable to merge the PR as is but might pick some features in the future (when I have some free time) if needed (and there is demand). Maybe it's best if you finish your project with your current fork and not rely on a release with all these features from my side.

Please do not consider this as a general rejection of your ideas and I surely do not want to diminish the huge effort you put into all of this. Quite the contrary. You helped a great deal to fix the wrapping-issue, brought a lot of great ideas to my attention and I sincerely thank you for this.

[matafokka]

Thank you for such kind words!

I didn't want to reinvent the wheel by forking your project and decided to go with the PR.

Then I'll stick with the GitHub fork for now, but might do a proper fork with npm package and stuff. There're couple of features I'd really like to add:

1. Merge splitting and wrapping with the line building algorithm. I think, it might improve performance a bit.
2. ~Get rid of multilinestrings. It complicates things, and I'm not sure if it's really needed.~ Totally forgot about GeoJSON support.
3. Implement canvas-based drawing instead of using polylines. As I said earlier, we can use trig functions and draw pixels directly. It'll produce way smoother lines than we get with the current approach with any number of points, and it'll be way faster. I don't have enough knowledge to do the math, but if you or someone you know is up for the task, I'll be more than happy to prepare the base for such functionality.

These changes will completely break backwards compatibility, so fork becomes a feasible option. What's your opinion on this?

If I'll go with a proper fork (leaflet-geodesic-experimental? :D), I'll notify you if there will be something interesting. In this case, I'll also look forward to a productive coexistence of the two projects :)

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.