search

Geovation / labelgun

🏷️ reducing label clutter across mapping libraries
https://geovation.github.io/labelgun/
113 stars 10 forks source link

_hideShownCollisions should not be required in update() #2

Closed JamesLMilner closed 7 years ago

JamesLMilner commented 7 years ago

For one reason or another we have to call this._hideShownCollisions() in order to hide labels that shouldn't be shown (I'd say it effects about 5% of labels). Not sure what the root cause of this is, but obviously a hack and decreases performance so would be A+++ to get rid of.

tomchadwin commented 7 years ago

Are you able to describe a reliable way to see this happening, using the Leaflet example, for instance?

JamesLMilner commented 7 years ago

Hey @tomchadwin thanks for request for clarification. It's been a while since I've replicated this, but if I remember correctly:

1) Remove this._hideShownCollisions() from the library source code: https://github.com/Geovation/labelgun/blob/master/src/labelgun.js#L213 2) Rebuild with Webpack 3) Use the rebuilt Labelgun library iwith the Leaflet example 4) Zoom in and out in a random fashion and you will eventually see the bug appear; labels will overlap

I'll see if I can write a failing test case for this at some point as it's a pretty irritating bug!

JamesLMilner commented 7 years ago

This has been fixed in the change of algorithm in 5.0.0