Fix/webgpu crash pixel density#8476
Fix/webgpu crash pixel density#8476saurabh24thakur wants to merge 2 commits intoprocessing:dev-2.0from
Conversation
- Added guard clauses in RendererWebGPU to handle uninitialized devices. - Improved Renderer3D._resetContext to wait for context initialization and inherit pixel density. - Added regression test in test/unit/webgpu/issue_repro.js.
|
|
||
| // If we reach here, no immediate crash occurred. | ||
| // Let's wait for the context to finish resetting to be sure everything is stable. | ||
| await new Promise(resolve => setTimeout(resolve, 100)); |
There was a problem hiding this comment.
Timeouts in tests are fairly brittle, we can use expect(() => { ... }).not.toThrow() or something like that as an alternative.
| myp5 = new p5(function(p) { | ||
| p.setup = async function() { | ||
| try { | ||
| await p.createCanvas(100, 100, 'webgpu'); |
There was a problem hiding this comment.
Can we use p.WEBGPU rather than the value of the constant?
| await p.createCanvas(100, 100, 'webgpu'); | ||
|
|
||
| // This triggers an asynchronous _resetContext | ||
| p.setAttributes({ powerPreference: 'low-power' }); |
There was a problem hiding this comment.
I'm not 100% sure that we'll do something with this, can we try changing antialias as a more normal example?
| @@ -0,0 +1,51 @@ | |||
| import p5 from '../../../src/app.js'; | |||
There was a problem hiding this comment.
A new file called issue_repro.js is not really readable for future contributors trying to understand the structure of our code and tests. Could this just be a test in the existing WebGPU renderer tests?
| } | ||
|
|
||
| _updateSize() { | ||
| if (!this.device) return; |
There was a problem hiding this comment.
To clarify, is this added just to methods that get called automatically when you reinitialize the context?
|
Thanks for working on this! Left a few comments, let me know what you think! |
2 participants
Fixes #8456
Description
Fixes a crash when using the WebGPU renderer where calling pixelDensity() immediately after setAttributes() (or any other function that triggers an asynchronous context reset) would result in
TypeError: Cannot read properties of undefined (reading 'createTexture').The Issue
In p5.js 2.0, setAttributes() triggers an asynchronous reset of the rendering context via _resetContext(). Because core functions like pixelDensity() are synchronous, they can trigger a resize() (and subsequently _updateSize()) on a new renderer instance before its WebGPU device has been successfully initialized.
Changes
this.deviceis not yet defined. Since _initContext() calls these methods upon successful initialization, the state is eventually synchronized correctly once the device is ready.awaitthe new renderer'scontextReadypromise before replacing the existing renderer on the p5 instance. This ensures that the sketch's_rendererproperty always points to a valid, initialized renderer.Checklist