-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Fix PointLayer polygon filtering for GeoJSON column mode #3301
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| // SPDX-License-Identifier: MIT | ||
| // Copyright contributors to the kepler.gl project | ||
|
|
||
| import {parseGeoJsonRawFeature} from './geojson-utils'; | ||
|
|
||
| /** | ||
| * Parse a raw geo field into Point/MultiPoint coordinates. | ||
| * Supports GeoJSON objects, WKT strings (via loaders.gl), and binary geometries. | ||
| */ | ||
| export function getGeojsonPointPositionFromRaw( | ||
| raw: unknown | ||
| ): number[] | number[][] | null { | ||
| const feature = parseGeoJsonRawFeature(raw); | ||
| const geometry = feature?.geometry as any; | ||
| if (!geometry) { | ||
| return null; | ||
| } | ||
|
|
||
| if (geometry.type === 'Point' || geometry.type === 'MultiPoint') { | ||
| return geometry.coordinates; | ||
| } | ||
|
|
||
| if (geometry.type === 'GeometryCollection') { | ||
| const geometries = geometry.geometries || []; | ||
| const coords = geometries.reduce((accu, g) => { | ||
| if (g?.type === 'Point') { | ||
| accu.push(g.coordinates); | ||
| } else if (g?.type === 'MultiPoint') { | ||
| accu.push(...g.coordinates); | ||
| } | ||
| return accu; | ||
| }, []); | ||
|
|
||
| return coords.length ? coords : null; | ||
| } | ||
|
|
||
| return null; | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -44,6 +44,7 @@ import { | |||||||||||
| FindDefaultLayerProps | ||||||||||||
| } from '../layer-utils'; | ||||||||||||
| import {getGeojsonPointDataMaps, GeojsonPointDataMaps} from '../geojson-layer/geojson-utils'; | ||||||||||||
| import {getGeojsonPointPositionFromRaw} from '../geojson-layer/geojson-position-utils'; | ||||||||||||
| import { | ||||||||||||
| ColorRange, | ||||||||||||
| Merge, | ||||||||||||
|
|
@@ -243,7 +244,42 @@ export default class PointLayer extends Layer { | |||||||||||
| case COLUMN_MODE_GEOARROW: | ||||||||||||
| return geoarrowPosAccessor(this.config.columns)(dataContainer); | ||||||||||||
| case COLUMN_MODE_GEOJSON: | ||||||||||||
| return geojsonPosAccessor(this.config.columns); | ||||||||||||
| return d => { | ||||||||||||
| // When hovering rendered points, deck.gl passes the expanded object that already | ||||||||||||
| // contains a numeric position. | ||||||||||||
| if (d && Array.isArray(d.position)) { | ||||||||||||
| return d.position; | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| // When filtering (and other CPU utilities), we typically get {index}. | ||||||||||||
| const index = d?.index; | ||||||||||||
| if (typeof index === 'number') { | ||||||||||||
| const mapped = this.dataToFeature?.[index]; | ||||||||||||
| // dataToFeature can contain [] (empty GeometryCollection); treat as null. | ||||||||||||
| if (Array.isArray(mapped) && mapped.length) { | ||||||||||||
| return mapped; | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| const {geojson} = this.config.columns; | ||||||||||||
| if (geojson?.fieldIdx > -1) { | ||||||||||||
| const raw = dataContainer.valueAt(index, geojson.fieldIdx); | ||||||||||||
| const coords = getGeojsonPointPositionFromRaw(raw); | ||||||||||||
| if (coords) { | ||||||||||||
| // cache parsed coordinates to avoid re-parsing during filtering/hover | ||||||||||||
| this.dataToFeature[index] = coords; | ||||||||||||
| } | ||||||||||||
| return coords; | ||||||||||||
| } | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| // Fallback: sometimes utilities pass the whole row as an array. | ||||||||||||
| if (Array.isArray(d)) { | ||||||||||||
| const {geojson} = this.config.columns; | ||||||||||||
| return getGeojsonPointPositionFromRaw(d[geojson.fieldIdx]); | ||||||||||||
|
||||||||||||
| return getGeojsonPointPositionFromRaw(d[geojson.fieldIdx]); | |
| if (geojson?.fieldIdx > -1) { | |
| return getGeojsonPointPositionFromRaw(d[geojson.fieldIdx]); | |
| } | |
| return null; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| // SPDX-License-Identifier: MIT | ||
| // Copyright contributors to the kepler.gl project | ||
|
|
||
| /** @jest-environment node */ | ||
|
|
||
| // loaders.gl expects fetch globals (Response, etc). Node < 18 / Jest may not provide them. | ||
| // eslint-disable-next-line @typescript-eslint/no-var-requires | ||
| const nodeFetch = require('node-fetch'); | ||
| (global as any).fetch = (global as any).fetch || nodeFetch; | ||
| (global as any).Response = (global as any).Response || nodeFetch.Response; | ||
| (global as any).Headers = (global as any).Headers || nodeFetch.Headers; | ||
| (global as any).Request = (global as any).Request || nodeFetch.Request; | ||
|
|
||
| import {getPolygonFilterFunctor} from './filter-utils'; | ||
| import {getGeojsonPointPositionFromRaw} from '../../layers/src/geojson-layer/geojson-position-utils'; | ||
|
|
||
| describe('filterUtils - polygon filter', () => { | ||
| const squarePolygon = { | ||
| type: 'Feature', | ||
| properties: {}, | ||
| geometry: { | ||
| type: 'Polygon', | ||
| coordinates: [ | ||
| [ | ||
| [-1, -1], | ||
| [1, -1], | ||
| [1, 1], | ||
| [-1, 1], | ||
| [-1, -1] | ||
| ] | ||
| ] | ||
| } | ||
| }; | ||
|
|
||
| test('WKT POINT string can be used for polygon filter', () => { | ||
| const layer = { | ||
| type: 'point', | ||
| getPositionAccessor: () => (d: {raw: unknown}) => getGeojsonPointPositionFromRaw(d.raw) | ||
| }; | ||
|
|
||
| const filter = {value: squarePolygon}; | ||
| const fn = getPolygonFilterFunctor(layer, filter, null); | ||
|
|
||
| expect(getGeojsonPointPositionFromRaw('POINT (0.5 0.5)')).toEqual([0.5, 0.5]); | ||
|
|
||
| expect(fn({raw: 'POINT (0.5 0.5)'})).toBe(true); | ||
| expect(fn({raw: 'POINT (10 10)'})).toBe(false); | ||
| }); | ||
|
|
||
| test('MultiPoint keeps row if any point is inside polygon', () => { | ||
| const layer = { | ||
| type: 'point', | ||
| getPositionAccessor: () => (d: {pos: any}) => d.pos | ||
| }; | ||
|
|
||
| const filter = {value: squarePolygon}; | ||
| const fn = getPolygonFilterFunctor(layer, filter, null); | ||
|
|
||
| expect( | ||
| fn({ | ||
| pos: [ | ||
| [10, 10], | ||
| [0.2, 0.2] | ||
| ] | ||
| }) | ||
| ).toBe(true); | ||
|
|
||
| expect( | ||
| fn({ | ||
| pos: [ | ||
| [10, 10], | ||
| [20, 20] | ||
| ] | ||
| }) | ||
| ).toBe(false); | ||
| }); | ||
| }); | ||
|
Comment on lines
+17
to
+77
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reduce function is missing an explicit type annotation for the initial accumulator value. The empty array literal on line 32 needs to be typed as
number[][]to ensure type safety. Without this, TypeScript cannot properly infer the accumulator type, which could lead to type errors or runtime issues if the code is modified in the future.