js-markerclusterer icon indicating copy to clipboard operation
js-markerclusterer copied to clipboard

Option for SuperClusterer algorithm bounding box

Open phil-chp opened this issue 2 years ago • 5 comments

Hello @jpoehnelt,

Is there a reason why 2 months ago (#60, src/algorithms/supercluster.ts:96), you switched the bounding box for getClusterer() in the SuperClusterer algorithm from a dynamic [west, south, east, north] to a static [-180, -90, 180, 90]?

We were working on a project, and we had some huge performance issues (tested on Ubuntu 20.04 LTS and macOS 12.1 Monterey).

Running on 2.3k markers, going above 100 clusters on the entire map would literally crash the browser.

Digging through the SuperClusterer algorithm, we found the bounding box set on those previously mentioned static numbers. Changing back those values for the dynamic ones, fixed it.

Correct me if I'm wrong, but it seems relevant to only want the algorithm to operate on currently visible clusters (instead of the entire world).

Thank you, Have a lovely day.

(Related pull request : !135)

phil-chp avatar Dec 08 '21 16:12 phil-chp

There is a trade off here that needs to be analyzed further.

  • The current implementation only computes clusters when the zoom has changed but for the entire globe.
  • The implementation in #135 computes clusters every time the map bounds changes but for a smaller region.

Which is better? Might depend on the specifics of the data. If markers are concentrated in a small area, the current implementation is probably more performant. If the markers are more uniform, probably the latter implementation is better.

It is possible to pass your own algorithm to meet your specific data patterns.

jpoehnelt avatar Dec 08 '21 18:12 jpoehnelt

That's an interesting point, we directly assumed it was a bug because of the browser crash and change in src/algorithms/supercluster.ts:96.

We have made another algorithm as suggested.

I'm interested in you mentioning that the current implementation is more optimized for concentrated data, because, that represents our case.
We were confronted with huge performance issues, using 2.3k markers in an approx. area of 30 km².


If you also feel divided by this, and understand the need for a dynamic bounding box in some cases, maybe it would be appealing to introduce it as a SuperClusterOptions?
(This goes hand-in-hand with extendBoundsToPaddedViewport(), so it doesn't feel too far-fetched to add it here.)

Some sort of :

enum bboxStatus {
    WHOLE_GLOBE,
    DYNAMIC,
    CUSTOM
}

function defineBoundingBox(status: bboxStatus = bboxStatus.WHOLE_GLOBE, custom?: number[]): number[] { ... }

If it were to be implemented and added in the docs, it would save lots of time for future people encountering this problem.

phil-chp avatar Dec 09 '21 15:12 phil-chp

Good suggestion on the configuration option.

jpoehnelt avatar Dec 13 '21 17:12 jpoehnelt

Hi, i have the same performance issue... it would help if this option existed in the supercluster options... Do you plan to add this option in the next future version? thank you

davidpadych avatar Jan 05 '22 21:01 davidpadych

Please submit a pr! 😄

jpoehnelt avatar Jan 05 '22 21:01 jpoehnelt

:tada: This issue has been resolved in version 2.4.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

googlemaps-bot avatar Jul 27 '23 17:07 googlemaps-bot