Fix cluster radius scaling in setClusterOptions (#5055)

* Fix cluster radius scaling in setClusterOptions * Add _scaleClusterRadius method and adjust tests * CHANGELOG * Refactor into GeoJSONSource._pixelsToTileUnits
maplibre · Nov 17, 2024 · b9e001d · b9e001d
1 parent bc43205
commit b9e001d
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 20 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,7 @@
 ### 🐞 Bug fixes
 - Fixes scale control for globe on zoom out ([#4897](https://github.com/maplibre/maplibre-gl-js/pull/4897))
 - Fixes cooperative gestures displaying the mobile help text when screen width is smaller than 480px on non-touch devices ([#5053](https://github.com/maplibre/maplibre-gl-js/pull/5053))
+- Fixes incorrect cluster radius scaling in `GeoJSONSource.setClusterOptions()` ([#5055](https://github.com/maplibre/maplibre-gl-js/pull/5055))
 - _...Add new stuff here..._
 
 ## 5.0.0-pre.6

diff --git a/src/source/geojson_source.test.ts b/src/source/geojson_source.test.ts
@@ -2,6 +2,7 @@ import {Tile} from './tile';
 import {OverscaledTileID} from './tile_id';
 import {GeoJSONSource, GeoJSONSourceOptions} from './geojson_source';
 import {IReadonlyTransform} from '../geo/transform_interface';
+import {EXTENT} from '../data/extent';
 import {LngLat} from '../geo/lng_lat';
 import {extend} from '../util/util';
 import {Dispatcher} from '../util/dispatcher';
@@ -231,7 +232,7 @@ describe('GeoJSONSource#update', () => {
             sendAsync(message: ActorMessage<any>) {
                 expect(message.type).toBe(MessageType.loadData);
                 expect(message.data.geojsonVtOptions).toEqual({
-                    extent: 8192,
+                    extent: EXTENT,
                     maxZoom: 10,
                     tolerance: 4,
                     buffer: 256,
@@ -259,8 +260,8 @@ describe('GeoJSONSource#update', () => {
                 expect(message.data.superclusterOptions).toEqual({
                     maxZoom: 12,
                     minPoints: 3,
-                    extent: 8192,
-                    radius: 1600,
+                    extent: EXTENT,
+                    radius: 100 * EXTENT / source.tileSize,
                     log: false,
                     generateId: true
                 });
@@ -269,14 +270,15 @@ describe('GeoJSONSource#update', () => {
             }
         });
 
-        new GeoJSONSource('id', {
+        const source = new GeoJSONSource('id', {
             data: {},
             cluster: true,
             clusterMaxZoom: 12,
             clusterRadius: 100,
             clusterMinPoints: 3,
             generateId: true
-        } as GeoJSONSourceOptions, mockDispatcher, undefined).load();
+        } as GeoJSONSourceOptions, mockDispatcher, undefined);
+        source.load();
     }));
 
     test('modifying cluster properties after adding a source', () => new Promise<void>(done => {
@@ -285,20 +287,22 @@ describe('GeoJSONSource#update', () => {
             sendAsync(message) {
                 expect(message.type).toBe(MessageType.loadData);
                 expect(message.data.cluster).toBe(true);
-                expect(message.data.superclusterOptions.radius).toBe(80);
+                expect(message.data.superclusterOptions.radius).toBe(80 * EXTENT / source.tileSize);
                 expect(message.data.superclusterOptions.maxZoom).toBe(16);
                 done();
                 return Promise.resolve({});
             }
         });
-        new GeoJSONSource('id', {
-            data: {},
+        const source = new GeoJSONSource('id', {
+            type: 'geojson',
+            data: {} as GeoJSON.GeoJSON,
             cluster: false,
             clusterMaxZoom: 8,
             clusterRadius: 100,
             clusterMinPoints: 3,
             generateId: true
-        } as GeoJSONSourceOptions, mockDispatcher, undefined).setClusterOptions({cluster: true, clusterRadius: 80, clusterMaxZoom: 16});
+        }, mockDispatcher, undefined);
+        source.setClusterOptions({cluster: true, clusterRadius: 80, clusterMaxZoom: 16});
     }));
 
     test('forwards Supercluster options with worker request, ignore max zoom of source', () => new Promise<void>(done => {
@@ -309,8 +313,8 @@ describe('GeoJSONSource#update', () => {
                 expect(message.data.superclusterOptions).toEqual({
                     maxZoom: 12,
                     minPoints: 3,
-                    extent: 8192,
-                    radius: 1600,
+                    extent: EXTENT,
+                    radius: 100 * EXTENT / source.tileSize,
                     log: false,
                     generateId: true
                 });
@@ -319,15 +323,16 @@ describe('GeoJSONSource#update', () => {
             }
         });
 
-        new GeoJSONSource('id', {
+        const source = new GeoJSONSource('id', {
             data: {},
             maxzoom: 10,
             cluster: true,
             clusterMaxZoom: 12,
             clusterRadius: 100,
             clusterMinPoints: 3,
             generateId: true
-        } as GeoJSONSourceOptions, mockDispatcher, undefined).load();
+        } as GeoJSONSourceOptions, mockDispatcher, undefined);
+        source.load();
     }));
 
     test('transforms url before making request', () => {
@@ -424,7 +429,7 @@ describe('GeoJSONSource#update', () => {
         source.on('data', (e) => {
             if (e.sourceDataType === 'metadata') {
                 source.setData({} as GeoJSON.GeoJSON);
-                source.loadTile(new Tile(new OverscaledTileID(0, 0, 0, 0, 0), 512));
+                source.loadTile(new Tile(new OverscaledTileID(0, 0, 0, 0, 0), source.tileSize));
             }
         });
 

diff --git a/src/source/geojson_source.ts b/src/source/geojson_source.ts
@@ -157,8 +157,6 @@ export class GeoJSONSource extends Evented implements Source {
         if (options.attribution) this.attribution = options.attribution;
         this.promoteId = options.promoteId;
 
-        const scale = EXTENT / this.tileSize;
-
         if (options.clusterMaxZoom !== undefined && this.maxzoom <= options.clusterMaxZoom) {
             warnOnce(`The maxzoom value "${this.maxzoom}" is expected to be greater than the clusterMaxZoom value "${options.clusterMaxZoom}".`);
         }
@@ -171,8 +169,8 @@ export class GeoJSONSource extends Evented implements Source {
             source: this.id,
             cluster: options.cluster || false,
             geojsonVtOptions: {
-                buffer: (options.buffer !== undefined ? options.buffer : 128) * scale,
-                tolerance: (options.tolerance !== undefined ? options.tolerance : 0.375) * scale,
+                buffer: this._pixelsToTileUnits(options.buffer !== undefined ? options.buffer : 128),
+                tolerance: this._pixelsToTileUnits(options.tolerance !== undefined ? options.tolerance : 0.375),
                 extent: EXTENT,
                 maxZoom: this.maxzoom,
                 lineMetrics: options.lineMetrics || false,
@@ -182,7 +180,7 @@ export class GeoJSONSource extends Evented implements Source {
                 maxZoom: options.clusterMaxZoom !== undefined ? options.clusterMaxZoom : this.maxzoom - 1,
                 minPoints: Math.max(2, options.clusterMinPoints || 2),
                 extent: EXTENT,
-                radius: (options.clusterRadius || 50) * scale,
+                radius: this._pixelsToTileUnits(options.clusterRadius || 50),
                 log: false,
                 generateId: options.generateId || false
             },
@@ -196,6 +194,10 @@ export class GeoJSONSource extends Evented implements Source {
         }
     }
 
+    private _pixelsToTileUnits(pixelValue: number): number {
+        return pixelValue * (EXTENT / this.tileSize);
+    }
+
     async load() {
         await this._updateWorkerData();
     }
@@ -259,7 +261,7 @@ export class GeoJSONSource extends Evented implements Source {
     setClusterOptions(options: SetClusterOptions): this {
         this.workerOptions.cluster = options.cluster;
         if (options) {
-            if (options.clusterRadius !== undefined) this.workerOptions.superclusterOptions.radius = options.clusterRadius;
+            if (options.clusterRadius !== undefined) this.workerOptions.superclusterOptions.radius = this._pixelsToTileUnits(options.clusterRadius);
             if (options.clusterMaxZoom !== undefined) this.workerOptions.superclusterOptions.maxZoom = options.clusterMaxZoom;
         }
         this._updateWorkerData();