Skip to content

Commit

Permalink
[fix] Fixed issue when mapstate latitude or langitude are out of boun…
Browse files Browse the repository at this point in the history
…ds (#2882)

- fixed issue when mapstate latitude or langitude are out of bounds

Signed-off-by: Ihor Dykhta <dikhta.igor@gmail.com>
  • Loading branch information
igorDykhta authored Jan 3, 2025
1 parent 92c9e6a commit 603fde8
Show file tree
Hide file tree
Showing 7 changed files with 247 additions and 20 deletions.
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@
"typescript": "tsc --noEmit",
"web": "(yarn && yarn install:web && yarn start:web)",
"deploy": "yarn install:web && cd website && yarn build",
"clean": "rm -rf node_modules examples/**/node_modules website/node_modules",
"release:patch": "git add CHANGELOG.md && git commit -m 'updated CHANGELOG.md' && npm version patch && git push origin && git push origin --tags",
"clean": "rm -rf node_modules examples/**/node_modules website/node_modules .nyc_output coverage jest-coverage tape-coverage",
"release:patch": "git add CHANGELOG.md && git commit -m 'updated CHANGELOG.md' && npm version patch && git push origin && git push origin --tags",
"fix-dependencies": "./scripts/fix-dependencies.sh"
},
"files": [
Expand Down
21 changes: 12 additions & 9 deletions src/reducers/src/data-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,14 @@

import {Bounds} from '@kepler.gl/types';
import {Layer} from '@kepler.gl/layers';

const MAX_LATITUDE = 90;
const MIN_LATITUDE = -90;
const MAX_LONGITUDE = 180;
const MIN_LONGITUDE = -180;
import {
MAX_LATITUDE,
MIN_LATITUDE,
MAX_LONGITUDE,
MIN_LONGITUDE,
validateLongitude,
validateLatitude
} from '@kepler.gl/utils';

/**
* takes a list of layer bounds and returns a single bound
Expand All @@ -27,10 +30,10 @@ export function processLayerBounds(layerBounds: Bounds[]): Bounds {
// at new WebMercatorViewport16 (web-mercator-viewport.js:92:5)
// at getViewportFromMapState (map-utils.js:46:66)
return [
minLongitude <= MIN_LONGITUDE ? 0 : minLongitude,
minLatitude <= MIN_LATITUDE ? 0 : minLatitude,
maxLongitude >= MAX_LONGITUDE ? 0 : maxLongitude,
maxLatitude >= MAX_LATITUDE ? 0 : maxLatitude
validateLongitude(minLongitude),
validateLatitude(minLatitude),
validateLongitude(maxLongitude),
validateLatitude(maxLatitude)
];
},
[MAX_LONGITUDE, MAX_LATITUDE, MIN_LONGITUDE, MIN_LATITUDE]
Expand Down
13 changes: 11 additions & 2 deletions src/reducers/src/map-state-updaters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,12 @@ import {fitBounds} from '@math.gl/web-mercator';
import deepmerge from 'deepmerge';
import pick from 'lodash.pick';

import {getCenterAndZoomFromBounds, validateBounds, MAPBOX_TILE_SIZE} from '@kepler.gl/utils';
import {
getCenterAndZoomFromBounds,
validateBounds,
MAPBOX_TILE_SIZE,
validateViewPort
} from '@kepler.gl/utils';
import {MapStateActions, ReceiveMapConfigPayload, ActionTypes} from '@kepler.gl/actions';
import {MapState, Bounds, Viewport} from '@kepler.gl/types';

Expand Down Expand Up @@ -101,7 +106,8 @@ export const updateMapUpdater = (
state: MapState,
action: MapStateActions.UpdateMapUpdaterAction
): MapState => {
const {viewport, mapIndex = 0} = action.payload;
const {viewport: inputViewport, mapIndex = 0} = action.payload;
const viewport = validateViewPort(inputViewport);

if (state.isViewportSynced) {
// The `updateViewport` function is typed as (Viewport, Viewport) -> Viewport but here the
Expand Down Expand Up @@ -272,6 +278,9 @@ export const receiveMapConfigUpdater = (
});
}

// make sure we validate map state before we merge
mergedState = validateViewPort(mergedState);

return {
...mergedState,
// update width if `isSplit` has changed
Expand Down
133 changes: 133 additions & 0 deletions src/utils/map-utils.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
// SPDX-License-Identifier: MIT
// Copyright contributors to the kepler.gl project

import WebMercatorViewport from 'viewport-mercator-project';

import {TRANSITION_DURATION} from '@kepler.gl/constants';
import {
getMapLayersFromSplitMaps,
onViewPortChange,
getViewportFromMapState
} from '@kepler.gl/utils';

export const INITIAL_MAP_STATE = {
pitch: 0,
bearing: 0,
latitude: 40,
longitude: -100,
zoom: 9,
dragRotate: false,
width: 800,
height: 800,
minZoom: undefined,
maxZoom: undefined,
maxBounds: undefined,
isSplit: false,
isViewportSynced: true,
isZoomLocked: false,
splitMapViewports: []
};

describe('mapUtils', () => {
describe('onViewPortChange', () => {
let mockOnUpdateMap;
let mockOnViewStateChange;

beforeEach(() => {
mockOnUpdateMap = jest.fn();
mockOnViewStateChange = jest.fn();
});

test('should use restViewState when width and height are 0', () => {
const viewState = {...INITIAL_MAP_STATE, width: 0, height: 0};
onViewPortChange(viewState, mockOnUpdateMap, mockOnViewStateChange);

/* eslint-disable no-unused-vars */
const {width, height, ...restViewState} = INITIAL_MAP_STATE;
expect(mockOnUpdateMap).toHaveBeenCalledWith({...restViewState, transitionDuration: 0}, 0);
});

test('should use viewState when width and height are greater than 0', () => {
const viewState = INITIAL_MAP_STATE;
onViewPortChange(viewState, mockOnUpdateMap, mockOnViewStateChange);

expect(mockOnUpdateMap).toHaveBeenCalledWith({...viewState, transitionDuration: 0}, 0);
});

test('should set transitionDuration based on primary flag', () => {
const viewState = INITIAL_MAP_STATE;
onViewPortChange(viewState, mockOnUpdateMap, mockOnViewStateChange, true);
expect(mockOnUpdateMap).toHaveBeenCalledWith(
{...viewState, transitionDuration: TRANSITION_DURATION},
0
);

onViewPortChange(viewState, mockOnUpdateMap, mockOnViewStateChange, false);
expect(mockOnUpdateMap).toHaveBeenCalledWith({...viewState, transitionDuration: 0}, 0);
});

test('should call onViewStateChange if it is a function', () => {
const viewState = {width: 100, height: 100};
onViewPortChange(viewState, mockOnUpdateMap, mockOnViewStateChange);
expect(mockOnViewStateChange).toHaveBeenCalledWith({...viewState, transitionDuration: 0});
});

test('should call onUpdateMap with correct arguments', () => {
const viewState = {width: 100, height: 100};
const mapIndex = 1;
onViewPortChange(viewState, mockOnUpdateMap, mockOnViewStateChange, false, mapIndex);
expect(mockOnUpdateMap).toHaveBeenCalledWith({...viewState, transitionDuration: 0}, mapIndex);
});
});
describe('getMapLayersFromSplitMaps', () => {
test('returns layers for valid index', () => {
const splitMaps = [{layers: ['Layer1', 'Layer2']}, {layers: ['Layer3', 'Layer4']}];
const mapIndex = 1;

expect(getMapLayersFromSplitMaps(splitMaps, mapIndex)).toEqual(['Layer3', 'Layer4']);
});

test('returns undefined for invalid index', () => {
const splitMaps = [{layers: ['Layer1', 'Layer2']}];
const mapIndex = 2;

expect(getMapLayersFromSplitMaps(splitMaps, mapIndex)).toBeUndefined();
});

test('returns undefined for empty splitMaps array', () => {
expect(getMapLayersFromSplitMaps([], 0)).toBeUndefined();
});

test('returns undefined if layers property does not exist', () => {
const splitMaps = [{}, {layers: ['Layer3', 'Layer4']}];
const mapIndex = 0;

expect(getMapLayersFromSplitMaps(splitMaps, mapIndex)).toBeUndefined();
});
});

describe('getViewportFromMapState', () => {
test('returns WebMercatorViewport when globe property is undefined', () => {
const mapState = {};
const result = getViewportFromMapState(mapState);

expect(result).toBeInstanceOf(WebMercatorViewport);
});

test('should not throw an exception if latitude is -90', () => {
const mapState = {
...INITIAL_MAP_STATE,
latitude: -90
};
expect(() => getViewportFromMapState(mapState)).not.toThrow();
});

test('should not throw an exception if latitude is 90', () => {
const mapState = {
...INITIAL_MAP_STATE,
latitude: 90
};
expect(() => getViewportFromMapState(mapState)).not.toThrow();
});
});
});
57 changes: 54 additions & 3 deletions src/utils/src/data-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,57 @@ import {isPlainObject} from './utils';

export type FieldFormatter = (value: any) => string;

// We need threat latitude differently otherwise marcator project view throws
// a projection matrix error
// Uncaught Error: Pixel project matrix not invertible
// at WebMercatorViewport16.Viewport6 (viewport.js:81:13)
export const MAX_LATITUDE = 89.9;
export const MIN_LATITUDE = -89.9;
export const MAX_LONGITUDE = 180;
export const MIN_LONGITUDE = -180;

/**
* Validates a latitude value.
* Ensures that the latitude is within the defined minimum and maximum latitude bounds.
* If the value is out of bounds, it returns the nearest bound value.
* @param latitude - The latitude value to validate.
* @returns The validated latitude value.
*/
export function validateLatitude(latitude: number | undefined): number {
return validateCoordinate(latitude ?? 0, MIN_LATITUDE, MAX_LATITUDE);
}

/**
* Validates a longitude value.
* Ensures that the longitude is within the defined minimum and maximum longitude bounds.
* If the value is out of bounds, it returns the nearest bound value.
* @param longitude - The longitude value to validate.
* @returns The validated longitude value.
*/
export function validateLongitude(longitude: number | undefined): number {
return validateCoordinate(longitude ?? 0, MIN_LONGITUDE, MAX_LONGITUDE);
}

/**
* Validates a coordinate value.
* Ensures that the value is within the specified minimum and maximum bounds.
* If the value is out of bounds, it returns the nearest bound value.
* @param value - The coordinate value to validate.
* @param minValue - The minimum bound for the value.
* @param maxValue - The maximum bound for the value.
* @returns The validated coordinate value.
*/
export function validateCoordinate(value: number, minValue: number, maxValue: number): number {
if (value <= minValue) {
return minValue;
}
if (value >= maxValue) {
return maxValue;
}

return value;
}

/**
* simple getting unique values of an array
*
Expand Down Expand Up @@ -179,7 +230,7 @@ export function getRoundingDecimalFromStep(step: number): number {
*/
export function normalizeSliderValue(
val: number,
minValue: number,
minValue: number | undefined,
step: number,
marks?: number[] | null
): number {
Expand All @@ -198,7 +249,7 @@ export function normalizeSliderValue(
* @param val
* @returns - rounded number
*/
export function roundValToStep(minValue: number, step: number, val: number): number {
export function roundValToStep(minValue: number | undefined, step: number, val: number): number {
if (!isNumber(step) || !isNumber(minValue)) {
return val;
}
Expand Down Expand Up @@ -273,7 +324,7 @@ export const parseFieldValue = (value: any, type: string): string => {
* @param field
*/
export function getFormatter(
format: string | Record<string, string> | null,
format?: string | Record<string, string> | null,
field?: Field
): FieldFormatter {
if (!format) {
Expand Down
35 changes: 33 additions & 2 deletions src/utils/src/map-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,27 @@ import WebMercatorViewport from 'viewport-mercator-project';
import {TRANSITION_DURATION} from '@kepler.gl/constants';
import {SplitMapLayers, SplitMap, Viewport, MapState} from '@kepler.gl/types';

import {validateLatitude, validateLongitude} from './data-utils';

/**
* Validates a ViewPort object.
* It retains all properties of the original ViewPort object,
* but ensures that the latitude is within the defined bounds.
* @param viewport - The ViewPort object to validate.
* @returns A new ViewPort object with validated latitude.
*/
export const validateViewPort = <T extends Pick<Viewport, 'latitude' | 'longitude'>>(
viewport: T
): T => {
return {
...viewport,
// make sure to process latitude to avoid 90, -90 values
// Uncaught Error: Pixel project matrix not invertible
...(viewport.latitude ? {latitude: validateLatitude(viewport.latitude)} : {}),
...(viewport.longitude ? {longitude: validateLongitude(viewport.longitude)} : {})
};
};

export const onViewPortChange = (
viewState: Viewport,
onUpdateMap: (next: any, mapIndex: number) => any,
Expand Down Expand Up @@ -37,9 +58,19 @@ export const getMapLayersFromSplitMaps = (

/**
* Generates a viewport from a map state.
* @param {*} mapState
* @param mapState
* @returns A viewport.
*/
export const getViewportFromMapState = (mapState: MapState): Viewport => {
return new WebMercatorViewport(mapState);
// Make sure we capture error
// e.g. Error message: "Pixel project matrix not invertible"
let viewPort;
try {
viewPort = new WebMercatorViewport(mapState);
} catch {
// catch error and fallback to default map state
viewPort = new WebMercatorViewport(validateViewPort(mapState));
}

return viewPort;
};
4 changes: 2 additions & 2 deletions test/node/utils/data-utils-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -250,15 +250,15 @@ test('dataUtils -> validateBounds', t => {
[10, -90, 20, -20],
[10, -10, 90, -90]
],
output: [10, -10, 90, -20],
output: [10, -89.9, 90, -20],
message: 'should handle latitude -90,90 correctly'
},
{
input: [
[-180, -90, 20, -20],
[15, -190, 25, -25]
],
output: [0, 0, 25, -20],
output: [-180, -89.9, 25, -20],
message: 'should handle extreme longitude values'
}
];
Expand Down

0 comments on commit 603fde8

Please sign in to comment.