Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions modules/mapbox/src/deck-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,10 @@ export function drawLayer(
clearStack = true;
}

if (!currentViewport) {
return;
}

deck._drawLayers('mapbox-repaint', {
viewports: [currentViewport],
layerFilter: params =>
Expand Down Expand Up @@ -167,6 +171,10 @@ export function drawLayerGroup(
clearStack = true;
}

if (!currentViewport) {
return;
}

deck._drawLayers('mapbox-repaint', {
viewports: [currentViewport],
layerFilter: params => {
Expand Down
6 changes: 5 additions & 1 deletion test/modules/mapbox/mapbox-gl-mock/map.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,11 @@ export default class Map extends Evented {
}

_render() {
this.style.render();
try {
this.style.render();
} catch (e) {
this._renderError = e;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale _renderError never cleared between render calls

Low Severity

_renderError is only ever assigned on failure and never reset to null/undefined at the start of _render(). If _render() is called multiple times (e.g., via multiple triggerRepaint() calls queued by Deck's _customRender), an error from an earlier render persists even if a later render succeeds. The t.notOk(_renderError) assertion in the render handler would then report a stale error, producing a misleading or false test failure. Adding this._renderError = null before the try-catch would ensure the value always reflects the most recent render attempt.

Fix in Cursor Fix in Web

this.fire(new Event('render'));
}

Expand Down
91 changes: 91 additions & 0 deletions test/modules/mapbox/mapbox-layer.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ test('MapboxLayer#external Deck multiple views supplied', t => {
map.addLayer(layerDefaultView);

map.on('render', () => {
t.notOk((map as any)._renderError, 'render should not throw');
t.deepEqual(
drawLog,
[
Expand Down Expand Up @@ -184,6 +185,7 @@ test('MapboxLayer#external Deck custom views', t => {

map.addLayer(new MapboxLayer({id: 'scatterplot'}));
map.on('render', () => {
t.notOk((map as any)._renderError, 'render should not throw');
t.deepEqual(
drawLog,
[
Expand All @@ -200,6 +202,95 @@ test('MapboxLayer#external Deck custom views', t => {
});
});

test('MapboxLayer#drawLayer with zero-size canvas', t => {
const map = new MockMapboxMap({
center: {lng: -122.45, lat: 37.78},
zoom: 12
});

map.on('load', () => {
const deck = new Deck({
device,
viewState: {longitude: 0, latitude: 0, zoom: 1},
layers: [
new ScatterplotLayer({
id: 'scatterplot',
data: [],
getPosition: d => d.position,
getRadius: 10,
getFillColor: [255, 0, 0]
})
],
// Set zero dimensions before the first render so makeViewport returns null.
// getDeckInstance wraps onLoad, so by the time this fires deck.isInitialized
// is true and drawLayer won't return early.
onLoad: () => {
(deck as any).width = 0;
(deck as any).height = 0;

map.on('render', () => {
t.notOk(
(map as any)._renderError,
'render should not throw when canvas has zero dimensions'
);
deck.finalize();
t.end();
});

(map as any)._render();
}
});

getDeckInstance({map, deck});
map.addLayer(new MapboxLayer({id: 'scatterplot'}));
});
});

test('MapboxLayer#afterRender with zero-size canvas', t => {
// afterRender is triggered when deck has multiple views (hasNonMapboxViews).
// It calls getViewport to replace the mapbox viewport and must not pass null
// into deck._drawLayers when the canvas has zero dimensions.
const map = new MockMapboxMap({
center: {lng: -122.45, lat: 37.78},
zoom: 12
});

map.on('load', () => {
const deck = new Deck({
device,
views: [new MapView({id: 'mapbox'}), new MapView({id: 'overview'})],
viewState: {longitude: 0, latitude: 0, zoom: 1},
layers: [
new ScatterplotLayer({
id: 'scatterplot',
data: [],
getPosition: d => d.position,
getRadius: 10,
getFillColor: [255, 0, 0]
})
],
onLoad: () => {
(deck as any).width = 0;
(deck as any).height = 0;

map.on('render', () => {
t.notOk(
(map as any)._renderError,
'afterRender should not throw when canvas has zero dimensions'
);
deck.finalize();
t.end();
});

(map as any)._render();
}
});

getDeckInstance({map, deck});
map.addLayer(new MapboxLayer({id: 'scatterplot'}));
});
});

/** Used to compare view states */
export function objectEqual(actual, expected) {
if (equals(actual, expected)) {
Expand Down
Loading