teralytics / Leaflet.D3SvgOverlay

Leaflet Plugin: D3 SVG Overlay
MIT License
184 stars 45 forks source link

Remove rounding from latLngToLayerPoint() to preserve sub-pixel accuracy on zoom. #28

Open ResidentMario opened 8 years ago

ResidentMario commented 8 years ago

At the moment, when you assign a pixel coordinate to a geographic coordinate using latLngToLayerPoint() the result is rounded off:

var projectedPoint = _layer.map.project(L.latLng(latLng), zoom)._round();

This rounds the pixel coordinate used to display whatever shape the coordinate is a part of to nearest-pixel accuracy. Now, this is fine when the map is zoomed out, but when you zoom in, what were inconsequential sub-pixel inaccuracies at 1x scale become become a crayon drawing at, say, 1000x scale:

zooming-example-zoomed-in

The fix for this is easy:

var projectedPoint = _layer.map.project(L.latLng(latLng), zoom);// ._round();

Which results in:

zooming-example-zoomed-out

Which is obviously far better.

xEviL commented 8 years ago

@ResidentMario, please create a complete test case for this. There already exists a code that turns off rounding while the layer is being drawn: https://github.com/teralytics/Leaflet.D3SvgOverlay/blob/master/L.D3SvgOverlay.js#L49 This may not work for some reason.

ResidentMario commented 8 years ago

Here's a bl.ocks minimal example documenting this issue:

http://bl.ocks.org/ResidentMario/13e902fd16fca628cc3fc826600bd2fe

This plot overlays the resulting lines by overplotting them on one another as you zoom in and out. The less accurate lines were plotted at lower zoom levels; the more accurate ones at higher zoom levels.

bkrrrr commented 8 years ago

+1, same for me. rounding leads to strange artefacts.

I made a version with d3 v4 support, and deactivated rounding. (I haven't removed the function calls so far)

https://github.com/bkrrr/Leaflet.D3SvgOverlay/commit/002178bc23a195ff50df1ebb7bf961ea772c5e59

What was the basic idea behind rounding? Is it possible to make something like this to a new major like in #25 proposed?

xEviL commented 8 years ago

@ResidentMario, @bkrrr yes this is a legitimate bug. Seems like I reintroduced rounding by mistake in ec19f5fb175710dc025b8fecef9c747156e186d4

Would you do a PR with fixes?

scarabdesign commented 6 years ago

Disabling Leaflet rounding is definitely having an adverse affect on the functioning of Leaflet Draw library. Specifically, when they try to crop a set of polygon points to what is only drawn on the screen, they Point objects are not being rounded after a zoom. So, drawing a point, then zooming in and then trying to draw further points, the execution is getting stuck in a while(true) loop in LineUtil:clipSegment