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

Fix offset behavior at low zoom levels #1

Open
bbecquet opened this issue Oct 6, 2014 · 12 comments
Open

Fix offset behavior at low zoom levels #1

bbecquet opened this issue Oct 6, 2014 · 12 comments
Labels

Comments

@bbecquet
Copy link
Owner

bbecquet commented Oct 6, 2014

When zooming-out, the offset lines get messed up, with bad angles and overlapping segments.

@bbecquet
Copy link
Owner Author

Actually it can happen at all zoom levels:
capture
It isn't visible on this picture, but this polyline contains a lot of tightly spaced points. The bug seems related to a loss of precision when these points are projected as pixel coordinates. Computation should be made on floating point coordinates instead.

See a similar bug that was fixed on Leaflet.PolylineDecorator: bbecquet/Leaflet.PolylineDecorator#20

@obi068
Copy link

obi068 commented Oct 15, 2015

Using a different intersection method helps a little bit.
Patching the offsetPolyline: function with this code fixes all issues for me.

// avoid points inside 2 pixels
var xs = b.x - a.x;
var ys = b.y - a.y;
var dist = Math.sqrt(xs*xs + ys*ys);
if (dist > 2) { original code }
intersection: function(p0, p1, p2, p3) {
    var s10_x = p1.x - p0.x;
    var s10_y = p1.y - p0.y;
    var s32_x = p3.x - p2.x;
    var s32_y = p3.y - p2.y;

    var denom = s10_x * s32_y - s32_x * s10_y;
    if (denom == 0) {return null};
    var denom_positive = denom > 0;

    var s02_x = p0.x - p2.x;
    var s02_y = p0.y - p2.y;

    var s_numer = s10_x * s02_y - s10_y * s02_x;
    if ((s_numer < 0) == denom_positive) {return null};

    var t_numer = s32_x * s02_y - s32_y * s02_x;

    if ((t_numer < 0) == denom_positive) {return null};

    if((s_numer > denom) == denom_positive || (t_numer > denom) == denom_positive) {return null};

    var t = t_numer / denom;
    return L.point(p0.x + (t * s10_x), p0.y + (t * s10_y))
  },

@BartWaardenburg
Copy link
Contributor

Hey @obi068,

I've run into this issue as well. I tried to use your code snippet, but it seems I need to do a couple more changes to the files besides the pieces of code you provided. Do you mind including the entire leaflet.polylineoffset.js file?

Update:
Thanks! I fixed my version by checking for the s1 & s2 objects in the joinLineSegments function. It now runs smoothly on all zoomlevels.

@obi068
Copy link

obi068 commented Nov 11, 2015

HI,

Of course.

You are right there are some other changes as well.
I still have some issues, but its much better than before.

br

gerhard

On Tue, Nov 10, 2015 at 8:37 AM, Bart Waardenburg [email protected]
wrote:

Hey @obi068 https://github.com/obi068,

I've run into this issue as well. I tried to use your code snippet, but it
seems I need to do a couple more changes to the files besides the pieces of
code you provided. Do you mind including the entire
leaflet.polylineoffset.js file?


Reply to this email directly or view it on GitHub
#1 (comment)
.

bbecquet pushed a commit that referenced this issue Jun 28, 2017
* Implemented (part of) the fix as @obi068 suggested

* Fixed bugs introduce in commit 422b577; Initial initial PR#13 created by @BartWaardenburg should be re-reviewed such that it can be merged in Leaflet.PolylineOffset code
@rsa2135
Copy link

rsa2135 commented Nov 26, 2017

Hi, I'm still experiencing this bug at zoom levels < 17

offset-behavior

code:

let lineCoordinates, color, line, route, depth;
routes.features.forEach((feature, idx) => {

  depth = feature.geometry.type == 'MultiLineString' ? 1 : 0;
  color = feature.properties.hex;
  line = feature.properties.route_id;
  route = feature.properties.color;
   
  lineCoordinates = L.GeoJSON.coordsToLatLngs(feature.geometry.coordinates, depth);

  L.polyline(lineCoordinates, {
    color: `#${color}`,
    weight: 2,
    offset: route === "blue" ? -10 : 0
  }).addTo(this.container);
});

Happens when using L.GeoJSON as well.

I've reduced the coordinates collection using Douglas-Peucker's algorithm, but the problem still persists (to a lesser degree).

Any idea for a workaround?
Thanks!

@kino90
Copy link

kino90 commented Dec 4, 2017

Hi, having this issue as well.

image

Does anybody have a clue how to fix it?

Thanks

@kino90
Copy link

kino90 commented Dec 4, 2017

Ok, I managed to patch this with the pull request content and the snippet from @obi068 (thanks a lot).
Will this be pushed to a new working version anytime soon? 👍

@ghost
Copy link

ghost commented Dec 18, 2017

I am waiting for a patch too! @kino90 Could you detail how you did your patch or commit your changes? I'm struggling to find the piece of code related to @obi068 patch

@KirillMetrik
Copy link

Having the latest version (1.1.1) and this is still an issue for me.

@LekisS
Copy link

LekisS commented Jun 12, 2018

@Sijmen PR #18 fixes the issue for me, thanks a lot

@creolis
Copy link

creolis commented Sep 3, 2018

#18 makes things better, but it does not fix the issue for me.
After applying all the mitigations in this thread (including #18), this is the (improved!) result:

screen shot 2018-09-03 at 15 07 50

@ghost
Copy link

ghost commented Jan 18, 2019

@creolis check my last pull request, you may find it useful

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

8 participants