Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 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
46 changes: 46 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,50 @@ 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'}));
});
});

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