DumpTools: Add bitmaprenderer fallback when ThinEngine is unavailable #17015
Conversation
|
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
|
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
|
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
|
Snapshot stored with reference name: Test environment: To test a playground add it to the URL, for example: https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/refs/pull/17015/merge/index.html#WGZLGJ#4600 Links to test babylon tools with this snapshot: https://playground.babylonjs.com/?snapshot=refs/pull/17015/merge To test the snapshot in the playground with a playground ID add it after the snapshot query string: https://playground.babylonjs.com/?snapshot=refs/pull/17015/merge#BCU1XR#0 |
|
WebGL2 visualization test reporter: |
|
Visualization tests for WebGPU |
|
Looking into the failed vis tests... |
|
WebGL2 visualization test reporter: |
|
Visualization tests for WebGPU |
|
Was a timing issue. The drawing to the shared canvas (whether via
(Under the hood, `canvas.toBlob` and `canvas.convertToBlob` capture the current canvas data synchronously, then perform the encoding asynchronously. We only care that the rendering + reading are synchronous. However, since the reading + encoding are coupled together in these APIs-- and because the APIs are callback-based-- we have to wrap the full draw + read + encode in the same Promise.)
|
|
WebGL2 visualization test reporter: |
|
Visualization tests for WebGPU |
For backwards compatibility. I missed this case in #17015. Fixes specific CreateScreenshotWithRenderTarget usage in Sandbox.
…BabylonJS#17015) This PR updates DumpTools to fall back to a BitmapRenderingContext from a canvas when ThinEngine isn’t available, instead of relying solely on ThinEngine. It also fixes issues that were first resolved in BabylonJS#13251 but regressed after BabylonJS#13803. To make DumpData Native-friendly, we need to avoid ThinEngine. While it might be possible to swap ThinEngine with [insert platform compatible engine], in practice it’s much simpler— at least for Native —to extend the existing 2D Canvas polyfill with bitmaprenderer support, rather than extending its graphics infrastructure to handle rendering and reading from a separate context or canvas. We can safely remove the engine dependency entirely if/when Webkit and Gecko respect the `premultiplyAlpha` flag when reading ImageBitmap. Or we can remove it now, if we're OK with breaking those (quite edge) cases. Thoughts? <details> <summary>More details</summary> For reference, below is a table showing observed results for whether unassociated alpha is preserved when reading pixel data, across different rendering contexts and browsers. Platform | WebGL + Canvas | WebGL + OffscreenCanvas | Bitmap + Canvas | Bitmap + OffscreenCanvas -- | -- | -- | -- | -- Blink (Chromium) | ✅| ❌| ✅| ✅ WebKit | ✅| ❌| ❌| ❌ Gecko | ✅| ✅| ❌| ❌ </details>
This PR updates DumpTools to fall back to a BitmapRenderingContext from a canvas when ThinEngine isn’t available, instead of relying solely on ThinEngine. It also fixes issues that were first resolved in #13251 but regressed after #13803.
To make DumpData Native-friendly, we need to avoid ThinEngine. While it might be possible to swap ThinEngine with [insert platform compatible engine], in practice it’s much simpler— at least for Native —to extend the existing 2D Canvas polyfill with bitmaprenderer support, rather than extending its graphics infrastructure to handle rendering and reading from a separate context or canvas.
We can safely remove the engine dependency entirely if/when Webkit and Gecko respect the
premultiplyAlphaflag when reading ImageBitmap. Or we can remove it now, if we're OK with breaking those (quite edge) cases. Thoughts?More details
For reference, below is a table showing observed results for whether unassociated alpha is preserved when reading pixel data, across different rendering contexts and browsers.