FrameGraph: remove static imports #17537
Conversation
… fix-volumlighting
… fix-volumlighting
… in a single file
… fix-volumlighting
… fix-volumlighting
|
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/17537/merge/index.html#WGZLGJ#4600 Links to test babylon tools with this snapshot: https://playground.babylonjs.com/?snapshot=refs/pull/17537/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/17537/merge#BCU1XR#0 |
|
|
||
| private _invProjection: Matrix; | ||
|
|
||
| protected override _gatherImports(useWebGPU: boolean, list: Promise<any>[]) { |
There was a problem hiding this comment.
Not sure if it is too late to change this protected API, but Promise<unknown> would be safer from a typing standpoint (assuming there is no reason unknown would be a problem in this context).
There was a problem hiding this comment.
I don't know if it would be a problem to make this change, but it would involve modifying many files (EffectWrapper + all post-processes), so that's something for another PR for sure.
| } | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/promise-function-async, no-restricted-syntax | ||
| public override initAsync(): Promise<any> { |
There was a problem hiding this comment.
Same comment if this can be Promise<unknown>
|
|
||
| if (this._engine.isWebGPU) { | ||
| // eslint-disable-next-line github/no-then | ||
| void import("../ShadersWGSL/lightingVolume.compute").then(() => { |
There was a problem hiding this comment.
Doesn't this mean if there is an error it will not be possible to see and handle that error from an API standpoint? It will just show up in the console as an unhandled error, right? Would it make more sense to have an async factory function, or is it necessary to be able to synchronously construct a LightingVolume and have async work in the background?
| private readonly _initAsyncPromises: Promise<void>[] = []; | ||
| private _currentProcessedTask: FrameGraphTask | null = null; | ||
| private _whenReadyAsyncCancel: Nullable<() => void> = null; | ||
| private _importPromise: Promise<any>; |
There was a problem hiding this comment.
One more time, Promise<uknown> would be safer here :)
|
|
||
| if (this._engine.isWebGPU) { | ||
| // eslint-disable-next-line github/no-then | ||
| void import("../ShadersWGSL/lightingVolume.compute").then(() => { |
There was a problem hiding this comment.
Isn't this WebGPU only? If so, then is there any reason to do a dynamic import?
There was a problem hiding this comment.
Yes, it's only WebGPU, but I don't want to add this import to the WebGPUEngine file, so that this file isn't loaded if you're not using the LightingVolume class. Is there another way to do this than the one I used?
|
Devhost visualization test reporter: |
|
Visualization tests for WebGPU |
|
WebGL2 visualization test reporter: |
4 participants
No description provided.