Issues with new algorithm

Geovation / labelgun

🏷️ reducing label clutter across mapping libraries

https://geovation.github.io/labelgun/

113 stars 10 forks source link

Issues with new algorithm #26

Closed tomchadwin closed 7 years ago

tomchadwin commented 7 years ago

I'm not sure the new algorithm is as successful as the old. I'm getting frequent overlaps:

2017-09-06 13_10_12-index html

I would also say that some plausible candidates for labelling have no label:

2017-09-06 13_12_28-index html

JamesLMilner commented 7 years ago

can you confirm the version you're using? All unit tests are passing and all examples seem okay on 5.0.1:

https://geovation.github.io/labelgun/examples/leaflet/index.html

tomchadwin commented 7 years ago

Master: https://github.com/Geovation/labelgun/blob/master/lib/labelgun.js

JamesLMilner commented 7 years ago

Mmm, strange. Can you link me the code?

tomchadwin commented 7 years ago

I'll upload a zip. It's got a lot of cruft, as it's generated from QGIS. I don't think the cruft's to blame, though...

tomchadwin commented 7 years ago

q2w_lg.zip

Zooming in and then back out again gives me the overlap.

JamesLMilner commented 7 years ago

@tomchadwin I examined your code, looks like you're using zoomstart, you want to be using zoomend (as this is the location of the labels at rest). I tried it and it worked as expected :) Also I updated the version to 6.0.0, making some minor changes to the API, the one that would effect you is destroy() becomes reset().

tomchadwin commented 7 years ago

Ah, I see! A daft mistake, with hindsight. I was trying to:

sync the fade transition nicely
leave zoomend free because I already use it for scale-dependent layer visibility

I'll change back to zoomend. Sorry for the noise.

tomchadwin commented 6 years ago

Just to confirm - all working. It also now works in QtWebkit, so your latest work means it's transpiling perfectly too, as far as I can tell. Thanks so much!