General improvements

[matafokka]

Edit: #58 fix was removed in favor of upstream fix.

Changes in this PR:

• Refactored code. Mainly, replaced forEach() with for-of loop which should increase performance on really underpowered devices, especially while continuously redrawing.
• Now project can be built on Windows, but hash sums won't be generated.

[matafokka]

Oh, and be sure to check todo in the tests, I've uncommented one with wrapping, it works, but you might want to change it to do something different

[matafokka]

@henrythasler, hello, and merry Christmas! I started working on implementing other features I mentioned. I checked geodesy library and wondered what's the difference between midpoint and intermediate point. Intermediate point is actually the same that arc.js implemented and I mentioned. With equal number of points, there's no difference in end results!

But intermediate point is faster, simpler, can bring more features to your library, and right API won't hurt backwards compatibility, so it's feasible to replace midpoint with it.

Furthermore, I believe, it's possible to integrate splitting and wrapping with intermediate point to achieve O(N) complexity instead of O(2N). Also, we can integrate latlngExpressionArraytoLatLngArray() functionality to the same algorithm to even further improve performance. Plus, it'll allow us to keep reference to the original points passed to setLatLngs().

I don't think that anyone uses aforementioned functions, so it's feasible to remove them for ~2x better performance and reduced library size.

Applying the same logic, it's feasible to merge all functions from types-helper into one. Remember: with continuous or long-running operations, function calls do noticeably impact performance.

So, I'll start with replacing midpoint with intermediate point and implementing features. Please, let me know what do you think about simplifying internal structure. I'd suggest doing so and releasing a major version with tiny edge-case breaking changes.

[henrythasler]

Hi @matafokka, thanks a lot for taking the time to help with this. I just grabbed your branch and did some local testing. I like your improvements (foreach -> for-of). The wrapMultiLineString() function in the testing-nosplit.html example looks a bit off though. Edit: I should have read the comment in the issue first that explains that it does not work with two points... sorry.

I remember I had this same problem in an earlier iteration. I will have a deeper look at your code.

Edit2: Increasing the steps still produces artefacts:

I prefer to keep this topic separated from other performance improvements and changes if you don't mind. Makes it easier for me to keep track of things.

[matafokka]

Hey, @henrythasler, thank you for looking into this!

I remember I had this same problem in an earlier iteration. I will have a deeper look at your code.

On your image, red line has 6 vertices, but blue - only 3. Wrong number of steps might be the problem. Can you send me your demo? Mine works just fine.

Edit2: Increasing the steps still produces artefacts

I solved it by implementing intermediate point and setting like 100 vertices. It works fine with up to 10K vertices in total on my potato laptop.

Though, the more vertices the more line looks jagged due to Leaflet's simplification. I'm still not sure what the best value is.

I prefer to keep this topic separated from other performance improvements and changes

Ok, then I suggest dealing with the first edit and ignoring second for now.

[henrythasler]

Can you send me your demo?

see docs/testing-nosplit.html

I solved it by implementing intermediate point and setting like 100 vertices. It works fine with up to 10K vertices in total on my potato laptop.

Increasing the vertices does not mitigate the issue. It only makes it harder to find it:

The existing solution does not show this behavior but has issues in other situations: existing	proposed
bad	good
good	bad

This means if we could combine both methods in a clever way we might be able to solve the issue.

[matafokka]

Second case, 1000 points, intermediate point algorithm, multiple variants of the same problem, red line is split:

LatLngs: [43, -468], [47.989921667414194, -287.92968750000006]:

LatLngs: [43, -468], [51.83577752045248, -288.28125000000006]:

Midpoint algorithm, 8 steps, 513 vertices, LatLngs: [[43, -468], [52.696361078274485, -288.28125000000006]]

Is this good?

But! I just found out, when difference between lngs is exactly 180 degrees, weird stuff happens:

This doesn't happen in your demo, do you know, what might be wrong?

[matafokka]

On second thought, adding more points to solve the issue is really unacceptable. I think, I know what's wrong. I'll try to fix it ASAP.

[henrythasler]

Please do also keep in mind, that the latest branch to address this issue is feature/better-wrap-next

[henrythasler]

I'll clean it up my branch a bit and merge it to master. Then you could integrate your other improvements (e.g. Win-build) in a new PR.

[matafokka]

Ok, thank you! Oh, should I also integrate intermediate point, API changes to support it and feature I call natural drawing (geodesic line exactly from point to point with all required revolutions, looks like sine graph)?

[henrythasler]

I'd appreciate if you just integrate

Refactored code. Mainly, replaced forEach() with for-of loop which should increase performance on really underpowered devices, especially while continuously redrawing. Now project can be built on Windows, but hash sums won't be generated.

so I can make a release with the bugfix and your improvements these days. I have yet to understand the other ideas you have so maybe this can then be another PR, if needed.

[matafokka]

Hello, done! Though, coverage check doesn't seem to correctly check types-helper.ts:49. Even IDEA didn't suggest to simplify that branches for some reason. Should I revert back this line?

[henrythasler]

Hello, done! Though, coverage check doesn't seem to correctly check types-helper.ts:49. Even IDEA didn't suggest to simplify that branches for some reason. Should I revert back this line?

Thanks. I'm not sure what you mean. Coverage for this file is 100%.

[matafokka]

True, either this stuff bugged when I checked earlier, or I'm going blind :D Either way, everything's ok then.