visgl / deck.gl

WebGL2 powered visualization framework
https://deck.gl
MIT License
12.25k stars 2.08k forks source link

[Bug] MaskEffect crashes for canvas with height 0 #8059

Closed felixpalmer closed 1 year ago

felixpalmer commented 1 year ago

Description

When the canvas is 0px, deck.gl crashes with the following error:

Uncaught TypeError: Cannot read properties of undefined (reading 'resolution')
    at MaskEffect.preRender (mask-effect.ts:81:16)
    at DeckRenderer._preRender (deck-renderer.ts:110:47)
    at DeckRenderer.renderLayers (deck-renderer.ts:74:12)
    at Deck._drawLayers (deck.ts:990:24)
    at Deck.redraw (deck.ts:539:12)
    at Deck._onRenderFrame (deck.ts:1038:10)
    at AnimationLoop.onRender (animation-loop.js:269:23)
    at AnimationLoop._renderFrame (animation-loop.js:370:10)
    at AnimationLoop.redraw (animation-loop.js:191:10)
    at renderFrame (animation-loop.js:297:12)

Flavors

Expected Behavior

No response

Steps to Reproduce

Modify https://deck.gl/docs/api-reference/extensions/mask-extension:

const deckgl = new Deck({
  height: 0,
  ...
});

The cause of this bug is that a 0 height canvas leads this.viewManager!.getViewports() to return [], which triggers the error in MaskEffect:

https://github.com/visgl/deck.gl/blob/master/modules/extensions/src/mask/mask-effect.ts#L83

Environment

Logs

No response

Pessimistress commented 1 year ago

deckRenderer.renderLayers should abort the flow if viewports is empty.

Pessimistress commented 1 year ago

8.9.26

saschnet commented 8 months ago

I found a similar problem that is related to the same origin under very specific conditions.

You can replicate it under every use of the mask effect, e.g. the official example at https://codepen.io/vis-gl/pen/ExbKoYg

If you have two screens on your desktop of which one is bigger than the other, e.g. grafik

When using Firefox and you position your mouse fully at the top-outside of the left screen (number 2) without reaching the right screen as you have to move your mouse down the arrive at the screen number 1, the prerender fails exactly as in this issue as no viewport is defined. grafik

The issue can be circumvent by adding a check for the viewport before checking the resolution

  if (viewport===undefined) {
    return {didRender};
  }

https://github.com/visgl/deck.gl/blob/28fb60fc262fd241cb20dab908cf47e15cc4c912/modules/extensions/src/mask/mask-effect.ts#L81

This might be triggered by an unanticipated behavior of the browser. However, I think it would make sense to control for such behaviors directly by verification that a viewport exists before crashing in such scenarios.

Environment

Framework version: deck.gl@8.9.35 Browser: Firefox OS: Windows 10

felixpalmer commented 8 months ago

Could you make a PR?

saschnet commented 8 months ago

Okay, I will do so!