Skip to content

Commit 914c3d2

Browse files
chrisgervangclaude
andauthored
chore(mapbox): Remove internal deck mode from MapboxLayer (#9955)
* chore(mapbox): Remove internal deck mode from MapboxLayer MapboxLayer is no longer a public API (only MapboxOverlay is exported), so the internal deck mode code paths are no longer needed. Changes: - Remove `isExternal` flag from UserData (always true now) - Remove internal deck creation branch in getDeckInstance - Remove `updateLayers` function (only used for internal mode) - Make `deck` prop required in getDeckInstance - Simplify MapboxLayer and MapboxLayerGroup to read map.__deck directly - Only MapboxOverlay calls getDeckInstance for initialization - Update tests to simulate MapboxOverlay flow Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * refactor(mapbox): Use getter for deck in MapboxLayer and MapboxLayerGroup Replace `this.deck` field with a getter that reads from `map.__deck` directly. This avoids potential stale references and simplifies the lifecycle - `onAdd` no longer needs to capture the deck instance. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(mapbox): Keep non-null assertions in render method Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * refactor(mapbox): Make __deck non-optional in MapWithDeck type Since MapboxLayer and MapboxLayerGroup are internal classes only used by MapboxOverlay, and MapboxOverlay always sets up map.__deck before adding these layers, __deck is guaranteed to exist when the map is set. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * refactor(mapbox): Remove deck getter, access map.__deck directly Since __deck is guaranteed on MapWithDeck, we can access it directly through the map rather than through a getter. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * test(mapbox): Remove trivial deck instance assertions These assertions just verified getDeckInstance set map.__deck, which is obvious and doesn't test any MapboxLayer behavior. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(mapbox): Restore original test structure for external Deck The first test should create Deck without gl option to test the case where deck has its own gl context. This matches the original test behavior where deck.props.gl !== map's gl. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(mapbox): Initialize map.__deck in tests before using MapboxLayer The resolveLayers test now calls getDeckInstance to set up map.__deck before resolving layers, since MapboxLayer.onAdd expects it to be set. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * refactor(mapbox): Consolidate gl context handling in getDeckInstance - Remove `gl` parameter from getDeckInstance - now obtained internally from map.painter.context.gl - Remove always-true `if (deck.props.gl === gl)` condition since deck is always created without gl and getDeckInstance sets it - Move WebGL1 compatibility warning from MapboxOverlay to getDeckInstance - Simplify MapboxOverlay._onAddInterleaved - just creates Deck with user props - Update tests to not pass gl when creating Deck instances Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(mapbox): Pass gl at Deck construction time, not via setProps The gl prop must be set when constructing Deck, not via setProps after construction. Move gl retrieval and WebGL1 compatibility warning back to mapbox-overlay.ts where the Deck is constructed, and remove redundant gl handling from getDeckInstance. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * refactor(mapbox): Remove redundant parameters merge in getDeckInstance Parameters are already merged when creating Deck in mapbox-overlay.ts, so the merge in getDeckInstance is redundant. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * refactor(mapbox): Remove unused deck prop from MapboxLayerProps The deck prop was vestigial from the old code where it was passed to getDeckInstance. Now MapboxLayer accesses the deck via map.__deck, so the prop is no longer needed. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * chore(mapbox): Remove unsupported mapbox-layers-react test app Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(mapbox): Remove deck prop from MapboxLayer in resolve-layers Missed this usage when removing the deck prop from MapboxLayerProps. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * chore(mapbox): Remove unsupported mapbox-layers test apps Remove mapbox-layers test app directory and update index.html links. The mapbox-layers-react app was already deleted in a previous commit. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * docs(mapbox): Update MapboxLayer doc to reflect internal-only status MapboxLayer is no longer part of the public API. Updated the doc to add a deprecation notice and migration guide pointing to MapboxOverlay. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * docs(mapbox): Remove MapboxLayer doc and update references MapboxLayer is internal-only and should not be documented as a public API. Removed the doc file and updated references in whats-new.md and test README. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(mapbox): Update tests to create Deck with default parameters Tests now create Deck with default parameters merged, matching how MapboxOverlay._onAddInterleaved creates the Deck. This is needed because getDeckInstance no longer merges default parameters. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * refactor(mapbox): Remove mapboxLayers tracking from UserData Instead of tracking MapboxLayer instances in a Set, use map.getLayer() to check if a deck layer has a corresponding MapboxLayer on the map. This simplifies the code since resolve-layers already creates MapboxLayer instances with the same ID as the deck layer. Ref: #9944 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * refactor(mapbox): Add null guard and consistent signature to MapboxLayer - Add gl parameter to onAdd for consistency with CustomLayerInterface - Add null guard in render() for safety, matching MapboxLayerGroup Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 50df2f2 commit 914c3d2

File tree

15 files changed

+82
-473
lines changed

15 files changed

+82
-473
lines changed

docs/api-reference/mapbox/mapbox-layer.md

Lines changed: 0 additions & 73 deletions
This file was deleted.

docs/whats-new.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -850,7 +850,7 @@ You can now use ArcGIS basemaps with deck.gl. This new module lets apps render d
850850

851851
The `MapView` now supports repeating worlds at low zoom levels. For backward compatibility, this feature is opt-in. Apps may turn it on by setting `views: new MapView({repeat: true})` on `Deck` or `DeckGL`.
852852

853-
Repeating is always on when using [MapboxLayer](./api-reference/mapbox/mapbox-layer.md) and [GoogleMapsOverlay](./api-reference/google-maps/google-maps-overlay.md).
853+
Repeating is always on when using `MapboxLayer` and [GoogleMapsOverlay](./api-reference/google-maps/google-maps-overlay.md).
854854

855855
As a result, `GoogleMapsOverlay` now supports all Google Maps zoom levels.
856856

modules/mapbox/src/deck-utils.ts

Lines changed: 41 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,7 @@ import {lngLatToWorld, unitsPerMeter} from '@math.gl/web-mercator';
1414
const MAPBOX_VIEW_ID = 'mapbox';
1515

1616
type UserData = {
17-
isExternal: boolean;
1817
currentViewport?: Viewport | null;
19-
mapboxLayers: Set<MapboxLayer<any>>;
20-
// mapboxVersion: {minor: number; major: number};
2118
};
2219

2320
// Mercator constants
@@ -27,24 +24,22 @@ const DEGREES_TO_RADIANS = Math.PI / 180;
2724
// Create an interleaved deck instance.
2825
export function getDeckInstance({
2926
map,
30-
gl,
3127
deck
3228
}: {
3329
map: Map & {__deck?: Deck<any> | null};
34-
gl: WebGL2RenderingContext;
35-
deck?: Deck<any>;
30+
deck: Deck<any>;
3631
}): Deck<any> {
3732
// Only create one deck instance per context
3833
if (map.__deck) {
3934
return map.__deck;
4035
}
4136

4237
// Only initialize certain props once per context
43-
const customRender = deck?.props._customRender;
44-
const onLoad = deck?.props.onLoad;
38+
const customRender = deck.props._customRender;
39+
const onLoad = deck.props.onLoad;
4540

4641
const deckProps = {
47-
...deck?.props,
42+
...deck.props,
4843
_customRender: () => {
4944
map.triggerRepaint();
5045
// customRender may be subscribed by DeckGL React component to update child props
@@ -54,52 +49,33 @@ export function getDeckInstance({
5449
customRender?.('');
5550
}
5651
};
57-
deckProps.parameters = {...getDefaultParameters(map, true), ...deckProps.parameters};
5852
deckProps.views ||= getDefaultView(map);
5953

60-
let deckInstance: Deck;
61-
62-
if (!deck || deck.props.gl === gl) {
63-
// If deck isn't defined (Internal MapboxLayer use case),
64-
// or if deck is defined and is using the WebGLContext created by mapbox (MapboxOverlay and External MapboxLayer use case),
65-
// block deck from setting the canvas size, and use the map's viewState to drive deck.
66-
// Otherwise, we use deck's viewState to drive the map.
67-
Object.assign(deckProps, {
68-
gl,
69-
width: null,
70-
height: null,
71-
touchAction: 'unset',
72-
viewState: getViewState(map)
73-
});
74-
if (deck?.isInitialized) {
54+
// deck is using the WebGLContext created by mapbox,
55+
// block deck from setting the canvas size, and use the map's viewState to drive deck.
56+
Object.assign(deckProps, {
57+
width: null,
58+
height: null,
59+
touchAction: 'unset',
60+
viewState: getViewState(map)
61+
});
62+
if (deck.isInitialized) {
63+
watchMapMove(deck, map);
64+
} else {
65+
deckProps.onLoad = () => {
66+
onLoad?.();
7567
watchMapMove(deck, map);
76-
} else {
77-
deckProps.onLoad = () => {
78-
onLoad?.();
79-
watchMapMove(deckInstance, map);
80-
};
81-
}
68+
};
8269
}
8370

84-
if (deck) {
85-
deckInstance = deck;
86-
deck.setProps(deckProps);
87-
(deck.userData as UserData).isExternal = true;
88-
} else {
89-
deckInstance = new Deck(deckProps);
90-
map.on('remove', () => {
91-
removeDeckInstance(map);
92-
});
93-
}
71+
deck.setProps(deckProps);
9472

95-
(deckInstance.userData as UserData).mapboxLayers = new Set();
96-
// (deckInstance.userData as UserData).mapboxVersion = getMapboxVersion(map);
97-
map.__deck = deckInstance;
73+
map.__deck = deck;
9874
map.on('render', () => {
99-
if (deckInstance.isInitialized) afterRender(deckInstance, map);
75+
if (deck.isInitialized) afterRender(deck, map);
10076
});
10177

102-
return deckInstance;
78+
return deck;
10379
}
10480

10581
function watchMapMove(deck: Deck, map: Map & {__deck?: Deck | null}) {
@@ -141,20 +117,6 @@ export function getDefaultParameters(map: Map, interleaved: boolean): Parameters
141117
return result;
142118
}
143119

144-
export function addLayer(deck: Deck, layer: MapboxLayer<any>): void {
145-
(deck.userData as UserData).mapboxLayers.add(layer);
146-
updateLayers(deck);
147-
}
148-
149-
export function removeLayer(deck: Deck, layer: MapboxLayer<any>): void {
150-
(deck.userData as UserData).mapboxLayers.delete(layer);
151-
updateLayers(deck);
152-
}
153-
154-
export function updateLayer(deck: Deck, layer: MapboxLayer<any>): void {
155-
updateLayers(deck);
156-
}
157-
158120
export function drawLayer(
159121
deck: Deck,
160122
map: Map,
@@ -371,28 +333,26 @@ function getViewport(deck: Deck, map: Map, renderParameters?: unknown): Viewport
371333
}
372334

373335
function afterRender(deck: Deck, map: Map): void {
374-
const {isExternal} = deck.userData as UserData;
375-
376-
if (isExternal) {
377-
// Draw non-Mapbox layers
378-
let viewports = deck.getViewports();
379-
const mapboxViewportIdx = viewports.findIndex(vp => vp.id === MAPBOX_VIEW_ID);
380-
const hasNonMapboxViews = viewports.length > 1 || mapboxViewportIdx < 0;
381-
382-
if (hasNonMapboxViews) {
383-
if (mapboxViewportIdx >= 0) {
384-
viewports = viewports.slice();
385-
viewports[mapboxViewportIdx] = getViewport(deck, map);
386-
}
387-
388-
deck._drawLayers('mapbox-repaint', {
389-
viewports,
390-
layerFilter: params =>
391-
(!deck.props.layerFilter || deck.props.layerFilter(params)) &&
392-
params.viewport.id !== MAPBOX_VIEW_ID,
393-
clearCanvas: false
394-
});
336+
// Draw non-Mapbox layers (layers that don't have a corresponding MapboxLayer on the map)
337+
const deckLayers = flatten(deck.props.layers, Boolean) as Layer[];
338+
const hasNonMapboxLayers = deckLayers.some(layer => layer && !map.getLayer(layer.id));
339+
let viewports = deck.getViewports();
340+
const mapboxViewportIdx = viewports.findIndex(vp => vp.id === MAPBOX_VIEW_ID);
341+
const hasNonMapboxViews = viewports.length > 1 || mapboxViewportIdx < 0;
342+
343+
if (hasNonMapboxLayers || hasNonMapboxViews) {
344+
if (mapboxViewportIdx >= 0) {
345+
viewports = viewports.slice();
346+
viewports[mapboxViewportIdx] = getViewport(deck, map);
395347
}
348+
349+
deck._drawLayers('mapbox-repaint', {
350+
viewports,
351+
layerFilter: params =>
352+
(!deck.props.layerFilter || deck.props.layerFilter(params)) &&
353+
(params.viewport.id !== MAPBOX_VIEW_ID || !map.getLayer(params.layer.id)),
354+
clearCanvas: false
355+
});
396356
}
397357

398358
// End of render cycle, clear generated viewport
@@ -408,17 +368,3 @@ function onMapMove(deck: Deck, map: Map): void {
408368
// a second repaint
409369
deck.needsRedraw({clearRedrawFlags: true});
410370
}
411-
412-
function updateLayers(deck: Deck): void {
413-
if ((deck.userData as UserData).isExternal) {
414-
return;
415-
}
416-
417-
const layers: Layer[] = [];
418-
(deck.userData as UserData).mapboxLayers.forEach(deckLayer => {
419-
const LayerType = deckLayer.props.type;
420-
const layer = new LayerType(deckLayer.props);
421-
layers.push(layer);
422-
});
423-
deck.setProps({layers});
424-
}

modules/mapbox/src/mapbox-layer-group.ts

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,12 @@
22
// SPDX-License-Identifier: MIT
33
// Copyright (c) vis.gl contributors
44

5-
import {getDeckInstance, drawLayerGroup} from './deck-utils';
5+
import {drawLayerGroup} from './deck-utils';
66
import type {Map, CustomLayerInterface} from './types';
77
import {assert, type Deck} from '@deck.gl/core';
88

9+
type MapWithDeck = Map & {__deck: Deck};
10+
911
export type MapboxLayerGroupProps = {
1012
id: string;
1113
renderingMode?: '2d' | '3d';
@@ -21,8 +23,7 @@ export default class MapboxLayerGroup implements CustomLayerInterface {
2123
/* Mapbox v3 Standard style */
2224
slot?: 'bottom' | 'middle' | 'top';
2325
beforeId?: string;
24-
map: Map | null;
25-
deck: Deck | null;
26+
map: MapWithDeck | null;
2627

2728
/* eslint-disable no-this-before-super */
2829
constructor(props: MapboxLayerGroupProps) {
@@ -34,19 +35,17 @@ export default class MapboxLayerGroup implements CustomLayerInterface {
3435
this.slot = props.slot;
3536
this.beforeId = props.beforeId;
3637
this.map = null;
37-
this.deck = null;
3838
}
3939

4040
/* Mapbox custom layer methods */
4141

42-
onAdd(map: Map, gl: WebGL2RenderingContext): void {
42+
onAdd(map: MapWithDeck, gl: WebGL2RenderingContext): void {
4343
this.map = map;
44-
this.deck = getDeckInstance({map, gl});
4544
}
4645

4746
render(gl, renderParameters) {
48-
if (!this.deck || !this.map) return;
47+
if (!this.map) return;
4948

50-
drawLayerGroup(this.deck, this.map, this, renderParameters);
49+
drawLayerGroup(this.map.__deck, this.map, this, renderParameters);
5150
}
5251
}

modules/mapbox/src/mapbox-layer.ts

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,15 @@
22
// SPDX-License-Identifier: MIT
33
// Copyright (c) vis.gl contributors
44

5-
import {getDeckInstance, addLayer, removeLayer, updateLayer, drawLayer} from './deck-utils';
5+
import {drawLayer} from './deck-utils';
66
import type {Map, CustomLayerInterface} from './types';
77
import type {Deck, Layer} from '@deck.gl/core';
88

9+
type MapWithDeck = Map & {__deck: Deck};
10+
911
export type MapboxLayerProps<LayerT extends Layer> = Partial<LayerT['props']> & {
1012
id: string;
1113
renderingMode?: '2d' | '3d';
12-
deck?: Deck;
1314
/* Mapbox v3 Standard style */
1415
slot?: 'bottom' | 'middle' | 'top';
1516
};
@@ -20,8 +21,7 @@ export default class MapboxLayer<LayerT extends Layer> implements CustomLayerInt
2021
renderingMode: '2d' | '3d';
2122
/* Mapbox v3 Standard style */
2223
slot?: 'bottom' | 'middle' | 'top';
23-
map: Map | null;
24-
deck: Deck | null;
24+
map: MapWithDeck | null;
2525
props: MapboxLayerProps<LayerT>;
2626

2727
/* eslint-disable no-this-before-super */
@@ -35,34 +35,27 @@ export default class MapboxLayer<LayerT extends Layer> implements CustomLayerInt
3535
this.renderingMode = props.renderingMode || '3d';
3636
this.slot = props.slot;
3737
this.map = null;
38-
this.deck = null;
3938
this.props = props;
4039
}
4140

4241
/* Mapbox custom layer methods */
4342

44-
onAdd(map: Map, gl: WebGL2RenderingContext): void {
43+
onAdd(map: MapWithDeck, gl: WebGL2RenderingContext): void {
4544
this.map = map;
46-
this.deck = getDeckInstance({map, gl, deck: this.props.deck});
47-
addLayer(this.deck, this);
4845
}
4946

5047
onRemove(): void {
51-
if (this.deck) {
52-
removeLayer(this.deck, this);
53-
}
48+
this.map = null;
5449
}
5550

5651
setProps(props: MapboxLayerProps<LayerT>) {
5752
// id cannot be changed
5853
Object.assign(this.props, props, {id: this.id});
59-
// safe guard in case setProps is called before onAdd
60-
if (this.deck) {
61-
updateLayer(this.deck, this);
62-
}
6354
}
6455

6556
render(gl, renderParameters) {
66-
drawLayer(this.deck!, this.map!, this, renderParameters);
57+
if (!this.map) return;
58+
59+
drawLayer(this.map.__deck, this.map, this, renderParameters);
6760
}
6861
}

0 commit comments

Comments
 (0)