Copy canvas data into intermediary ImageData buffer #5155
Conversation
src/source/canvas_source.js
Outdated
| const gl = this.context; | ||
| const data = new Uint8Array(this.width * this.height * 4); | ||
| gl.readPixels(0, 0, this.width, this.height, gl.RGBA, gl.UNSIGNED_BYTE, data); | ||
| return new window.ImageData(new Uint8ClampedArray(data), this.width, this.height); |
There was a problem hiding this comment.
According to MDN, the ImageData constructor is not available on IE (and Safari support is unknown).
One possible way around this would be to return {width: number, height: number, data: Uint8ClampedArray} (which ImageData will also satisfy), and have _prepareImage accept that.
There was a problem hiding this comment.
I started going down this path but ran into complications with different texImage2D/texSubImage2D signatures and with flow 😕 instead I went with using CanvasRenderingContext2D.createImageData, which is much cleaner but requires creating an extra canvas context for webgl-based canvas sources…
| @@ -22,7 +22,8 @@ import type Evented from '../util/evented'; | |||
| * [-76.52, 39.18], | |||
| * [-76.52, 39.17], | |||
| * [-76.54, 39.17] | |||
| * ] | |||
| * ], | |||
| * contextType: '2d' | |||
There was a problem hiding this comment.
If we have to make a breaking change here, maybe it's better to replace the canvas property with a context property of type CanvasRenderingContext2D | WebGLRenderingContext. On the other hand, that would be a property that's not JSON-serializable...
There was a problem hiding this comment.
Intuitively I think I prefer the JSON-serializable string, and feel okay with the string enum options since it must match the string that was used to access the drawing context in the first place 🤔 though I'm happy to defer to stronger opinions…
…reate a new ImageData object without its constructor
I believe this fixes #4262: I've confirmed with a few colleagues who have affected GPUs that multiple test cases were fixed with this build.
I suspect affected GPUs have a bug in using an
HTMLCanvasElementas thepixelsargument intexImage2D/texSubImage2D. I haven't been able to corroborate this theory anywhere on the internet, unfortunately.This PR circumvents that by copying canvas data into an intermediary
⚠️ BREAKING ⚠️ : It does necessitate the addition of a
ImageDataobject.CanvasSource#contextTypeparameter.Launch Checklist