bbecquet / Leaflet.PolylineOffset

Apply a relative pixel offset to polylines without changing their coordinates.
MIT License
152 stars 32 forks source link

Fix offset behavior at low zoom levels #1

Open bbecquet opened 10 years ago

bbecquet commented 10 years ago

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

bbecquet commented 9 years ago

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: https://github.com/bbecquet/Leaflet.PolylineDecorator/issues/20

obi068 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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 notifications@github.com 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 https://github.com/bbecquet/Leaflet.PolylineOffset/issues/1#issuecomment-155345710 .

rsa2135 commented 6 years ago

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 commented 6 years ago

Hi, having this issue as well.

image

Does anybody have a clue how to fix it?

Thanks

kino90 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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

LekisS commented 6 years ago

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

creolis commented 6 years ago

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 commented 5 years ago

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