Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
10 changes: 10 additions & 0 deletions modules/mapbox/src/deck-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,11 @@ export function drawLayer(
clearStack = true;
}

if (!currentViewport) {
// Viewport could not be created (e.g. canvas has zero dimensions).
return;
}

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

if (!currentViewport) {
// Viewport could not be created (e.g. canvas has zero dimensions).
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
42 changes: 42 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,46 @@ 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]
})
]
});

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

// Simulate a zero-size canvas (e.g. hidden or detached container).
// makeViewport() returns null when width or height is 0, so drawLayer
// must not pass that null into deck._drawLayers.
(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.fire('render');
});
});

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