-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Fix/webgpu crash pixel density #8476
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev-2.0
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| import p5 from '../../../src/app.js'; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A new file called |
||
| import rendererWebGPU from "../../../src/webgpu/p5.RendererWebGPU"; | ||
|
|
||
| p5.registerAddon(rendererWebGPU); | ||
|
|
||
| suite('WebGPU Issue Repro (pixelDensity after setAttributes)', function() { | ||
| let myp5; | ||
|
|
||
| afterEach(function() { | ||
| if (myp5) { | ||
| myp5.remove(); | ||
| } | ||
| }); | ||
|
|
||
| test('pixelDensity(1) after setAttributes() should not crash', async function() { | ||
| // This test simulates the issue where a synchronous call (pixelDensity) | ||
| // happens before an asynchronous initialization (setAttributes -> _resetContext) | ||
| // is complete. | ||
|
|
||
| await new Promise((resolve, reject) => { | ||
| try { | ||
| myp5 = new p5(function(p) { | ||
| p.setup = async function() { | ||
| try { | ||
| await p.createCanvas(100, 100, 'webgpu'); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we use |
||
|
|
||
| // This triggers an asynchronous _resetContext | ||
| p.setAttributes({ powerPreference: 'low-power' }); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not 100% sure that we'll do something with this, can we try changing |
||
|
|
||
| // This triggers a synchronous resize() -> _updateSize() | ||
| // before the new renderer's device is ready. | ||
| p.pixelDensity(1); | ||
|
|
||
| resolve(); | ||
| } catch (err) { | ||
| reject(err); | ||
| } | ||
| }; | ||
| }); | ||
| } catch (err) { | ||
| reject(err); | ||
| } | ||
| }); | ||
|
|
||
| // 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)); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Timeouts in tests are fairly brittle, we can use |
||
| expect(myp5._renderer).to.exist; | ||
| expect(myp5._renderer.device).to.exist; | ||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To clarify, is this added just to methods that get called automatically when you reinitialize the context?