Clustering breaks when using maxzoom <= clusterMaxZoom

[NikkyAI]

maplibre-gl-js version: 3.1.0

browser: firefox

Steps to Trigger Behavior

1. have a high enough maxzoom on your map, lets assume 24 or 25
2. enable clustering with a clusterMaxZoom of 18 or higher on a geojson source
3. add data that has enough nearby points

Link to Demonstration

https://jsfiddle.net/axL71c8q/6/

Expected Behavior

clusters disappear/expand into actual data once zoomed in further than clusterMaxZoom

Actual Behavior

clusters stay around no matter how far you zoom in,

clustering on indoor maps is impossible in its current state

[HarelM]

clusterMaxZoom is intended to define when to stop clustering. If you would like to have the cluster in higher zoom level you need to increase the cluterMaxZoom.
What am I missing?

[NikkyAI]

it keeps clustering even past the set clusterMaxZoom level once its set to 18 or higher, lets say i want to set it to 20, i would expect clustering to stop at 21 or 22, but it does not
once clustering is set to 18 or higher.. it never stops clustering no matter how far you zoom in

in the example the clusterMaxZoom is set to 18, but no matter how far you zoom in it never unclusters.. once you set it back to 17 or lower it will behave as expected

[HarelM]

Ahh, I see the issue now.
This is strange.
Looking at the code it might be a supercluster issue or some problem passing the right value to the worker. Feel free to console log the data passed to the worker here:

maplibre-gl-js/src/source/geojson_worker_source.ts

Line 300 in 6794451

function getSuperclusterOptions({superclusterOptions, clusterProperties}: { superclusterOptions?: any; clusterProperties?: any}) {

maplibre-gl-js/src/source/geojson_source.ts

Line 178 in 6794451

maxZoom: options.clusterMaxZoom !== undefined ? options.clusterMaxZoom : this.maxzoom - 1,

I also see some problem with the typings that can be improved.
A PR would be more than welcome!

[NikkyAI]

i adapted a jsfiddle i found that used supercluster and openlayers map to the latest version of supercluster and the same sample data, its not showing this bug: https://jsfiddle.net/0z5gsq63/25/

i have to admit i am not feeling at home working with javascript and typescript.. we are using this entire library from kotlinjs.. and i am more used to backend work anyways, but i will try my best to track down the root of the issue anyways

[HarelM]

Let me know if you need any guidance doing so.
Generally speaking, I would start with adding console.log in the places I highlighted above and see what values you get.
From the example provided it doesn't look like a supercluster issue, so the main suspect is passing the data to the worker and initializing the supercluster correctly.

[NikkyAI]

i think i found the culprit

in geojson source the maxzoom property

maplibre-gl-js/src/source/geojson_source.ts

Line 140 in e50a264

this.maxzoom = 18;

this is not the same property as that is passed into Map
and has to be the same or higher than the clusterMaxZoom for clustering to update as you zoom in

this was not documented anywhere i could find
and given that people looked at this fiddle and did not spot the mstake either...maxzoom should probably be set when clusterMaxZoom is passed in

i would suggest to add this line to initialization in geojson source

if (options.cluster === true && options.clusterMaxZoom !== undefined) this.maxzoom = Math.max(this.maxzoom, options.clusterMaxZoom + 1);

i think this should be fixed either by improved documentation or implicitely doing the 'correct' thing based on user input... or at least a warning when the source maxzoom is the same or lower than the clusterMaxZoom

[HarelM]

I'm not following, the following line takes the maxzoom of 18 and checks if the options has clusterMaxZoom to override the default here:
https://github.com/maplibre/maplibre-gl-js/blob/e50a264b2174e8534d265e47b13d55fdc79ee5a9/src/source/geojson_source.ts#LL178C17-L178C24
What am I missing?

[HarelM]

The behavior here is strange.
The maxzoom is intended for the geojson-vt to create tiles up to a certain zoom level.
The clusterMaxZoom controls the limit on when to stop clustering.
If I understand it correctly, the maxzoom should be bigger than the cluster max zoom so that the bug that is reported here won't happen. I'm not sure though.
I played around with maxzoom and clusterMaxZoom properties of the source and got things working.
But, there's definitely a bug here, although it can be workaround by setting the maxzoom to be clusterMaxZoom + 1 I think.

here's my hacked addSource code:

map.addSource('earthquakes', {
    type: 'geojson',
    // Point to GeoJSON data. This example visualizes all M1.0+ earthquakes
    // from 12/22/15 to 1/21/16 as logged by USGS' Earthquake hazards program.
    data: data,
    cluster: true,
    maxzoom: clusterMaxZoom > 17 ? clusterMaxZoom + 1 : 18, // 18 is the default so keeping it as original, this is the hack...
    clusterMaxZoom: clusterMaxZoom, // Max zoom to cluster points on
    clusterRadius: 50 // Radius of each cluster when clustering points (defaults to 50)
  });

[NikkyAI]

another thing we are now running into...
whenever we set the maxzoom to 19 or higher.. eventually the map crashes (not sure if this is due to clustering or something else)

the stacktrace just points to where map is initialized...

i tried setting the maxzoom of the source to 19 or higher than the maps maxZoom ... it seems to behave the same

PS: we are updating the data using diffs, this might be of value ?

i cannot reproduce it in a jsfiddle.. yet...
once i am able to i will open a new issue i guess

[HarelM]

Try reproducing this on a simple scenario. Diffs are relatively new to geojson and I'm not sure how well tested they are...

[HarelM]

Use maxzoom - all small letters. See my code.

[NikkyAI]

i think i found and fixed the culprit of that weird crash.. there was some code in our app that added sources and layers without checking if they exist already...
thats definitly something to optimize on our end

not sure why that would be causing issues... but i guess it heppened because those sources get added while zoomed in further than the default maxzoom ...
either way it does not happen anymore now

[andrewharvey]

i think this should be fixed either by improved documentation or implicitely doing the 'correct' thing based on user input... or at least a warning when the source maxzoom is the same or lower than the clusterMaxZoom

If I understand it correctly, the maxzoom should be bigger than the cluster max zoom so that the bug that is reported here won't happen. I'm not sure though.

Correct.

clusterMaxZoom: Max zoom on which to cluster points if clustering is enabled. Defaults to one zoom less than maxzoom (so that last zoom features are not clustered). Clusters are re-evaluated at integer zoom levels so setting clusterMaxZoom to 14 means the clusters will be displayed until z15.

If you set clusterMaxZoom to 18, clusters will continue to display >=18 <19 (up to 18.99...). To ensure you're still rending the source unclustered at z19 you need to have the source maxzoom set to at least 19.

If you want the clustering to stop at z18 you need to set clusterMaxZoom to 17 then the source maxzoom to 18.

So I don't think this is a bug, it's working as expected, but I do agree that we should omit a warning if maxzoom <= clusterMaxZoom.

I think a warning is less dangerous than trying to override the maxzoom value based on the clusterMaxZoom because as user you'll provide one set of values for maxzoom / clusterMaxZoom but internally maplibre-js would be using different values, which would be confusing.

I've put together #4604 as a suggestion.

[andrewharvey]

I suggest changing the issue title to "clustering breaks when using maxzoom <= clusterMaxZoom" be better represent the actual issue reported here.