rorystephenson / supercluster_dart

A port of MapBox's javascript supercluster library for fast marker clustering.
ISC License
2 stars 3 forks source link

SuperclusterMutable.load not replacing elements #3

Closed Robbendebiene closed 1 year ago

Robbendebiene commented 1 year ago

I think there is a mistake in the docs. It is stated that SuperclusterMutable.load will replace existing points.

However since this uses the rbush load method under the hood it is actually just a method for inserting multiple points at once.

// Bulk load elements (empty in this case, so here it's a no-op). tree.load([]);

from: https://pub.dev/packages/rbush

I've also tested this.

rorystephenson commented 1 year ago

I think there is a mistake in the docs. It is stated that SuperclusterMutable.load will replace existing points.

However since this uses the rbush load method under the hood it is actually just a method for inserting multiple points at once.

~Thanks for pointing that out! I agree it's incorrect. Do you think it would be clearer if it says something like:~

~Bulk load points and form clusters. Existing points/clusters are discarded. This may take some time with large amounts of points. If you wish to add/remove some points you should use insert/remove which will only modify affected points/clusters and is much faster.~

EDIT: I've just re-read your comment and I realised that you're pointing out that existing points are not actually removed 🤦 . That was not intended. I suspect this is probably broken as the way the clustering algorithm works all affected clusters need to be rebuilt from the bottom layer up when new clusters may be formed (as can happen if you add a point close to an existing one which causes a cluster to be created).

I'm going to fix the implementation to match the documentation.

rorystephenson commented 1 year ago

@Robbendebiene This should be fixed now in 2.1.1. Please let me know if there is still an issue.