-
Notifications
You must be signed in to change notification settings - Fork 3.6k
FrameGraph: remove static imports #17537
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: master
Are you sure you want to change the base?
Changes from all commits
61d082e
f88d2e5
c91cb09
ac096ba
d189cfd
4309fa9
0629cd9
66d8ac1
79fbe92
ffb1f04
ec44284
dda7b99
ca9c683
ad8d205
243d557
e5f9cf0
65a1050
6788d2f
a415439
7181b38
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 |
|---|---|---|
|
|
@@ -9,11 +9,6 @@ import { FrameGraphObjectRendererTask } from "../Rendering/objectRendererTask"; | |
| import { ShaderMaterial } from "core/Materials/shaderMaterial"; | ||
| import { ShaderLanguage } from "core/Materials/shaderLanguage"; | ||
|
|
||
| import "core/Shaders/volumetricLightingRenderVolume.vertex"; | ||
| import "core/Shaders/volumetricLightingRenderVolume.fragment"; | ||
| import "core/ShadersWGSL/volumetricLightingRenderVolume.vertex"; | ||
| import "core/ShadersWGSL/volumetricLightingRenderVolume.fragment"; | ||
|
|
||
| const InvViewProjectionMatrix = new Matrix(); | ||
|
|
||
| /** | ||
|
|
@@ -187,6 +182,15 @@ export class FrameGraphVolumetricLightingTask extends FrameGraphTask { | |
| this.lightPower = this._lightPower; | ||
| } | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/promise-function-async, no-restricted-syntax | ||
| public override initAsync(): Promise<any> { | ||
|
Member
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. Same comment if this can be |
||
| if (this._frameGraph.engine.isWebGPU) { | ||
| return Promise.all([import("../../../ShadersWGSL/volumetricLightingRenderVolume.vertex"), import("../../../ShadersWGSL/volumetricLightingRenderVolume.fragment")]); | ||
| } | ||
|
|
||
| return Promise.all([import("../../../Shaders/volumetricLightingRenderVolume.vertex"), import("../../../Shaders/volumetricLightingRenderVolume.fragment")]); | ||
| } | ||
|
|
||
| public override isReady() { | ||
| return ( | ||
| this._renderLightingVolumeMaterial.isReady() && | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,9 +10,6 @@ import { _RetryWithInterval } from "core/Misc/timingTools"; | |
| import { Logger } from "core/Misc/logger"; | ||
| import { UniqueIdGenerator } from "core/Misc/uniqueIdGenerator"; | ||
|
|
||
| import "core/Engines/Extensions/engine.multiRender"; | ||
| import "core/Engines/WebGPU/Extensions/engine.multiRender"; | ||
|
|
||
| enum FrameGraphPassType { | ||
| Normal = 0, | ||
| Render = 1, | ||
|
|
@@ -37,6 +34,7 @@ export class FrameGraph implements IDisposable { | |
| private readonly _initAsyncPromises: Promise<void>[] = []; | ||
| private _currentProcessedTask: FrameGraphTask | null = null; | ||
| private _whenReadyAsyncCancel: Nullable<() => void> = null; | ||
| private _importPromise: Promise<any>; | ||
|
Member
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. One more time, |
||
|
|
||
| /** | ||
| * Name of the frame graph | ||
|
|
@@ -105,6 +103,7 @@ export class FrameGraph implements IDisposable { | |
| ) { | ||
| this._scene = scene; | ||
| this._engine = scene.getEngine(); | ||
| this._importPromise = this._engine.isWebGPU ? import("../Engines/WebGPU/Extensions/engine.multiRender") : import("../Engines/Extensions/engine.multiRender"); | ||
| this.textureManager = new FrameGraphTextureManager(this._engine, debugTextures, scene); | ||
| this._passContext = new FrameGraphContext(this._engine, this.textureManager, scene); | ||
| this._renderContext = new FrameGraphRenderContext(this._engine, this.textureManager, scene); | ||
|
|
@@ -234,6 +233,8 @@ export class FrameGraph implements IDisposable { | |
| this._built = false; | ||
|
|
||
| try { | ||
| await this._importPromise; | ||
|
|
||
| await this._whenAsynchronousInitializationDoneAsync(); | ||
|
|
||
| for (const task of this._tasks) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,8 +11,6 @@ import { StorageBuffer } from "core/Buffers/storageBuffer"; | |
| import { BaseTexture } from "core/Materials/Textures/baseTexture"; | ||
| import { VertexBuffer } from "core/Buffers/buffer"; | ||
|
|
||
| import "core/ShadersWGSL/lightingVolume.compute"; | ||
|
|
||
| const TmpVec3 = new Vector3(); | ||
|
|
||
| /** | ||
|
|
@@ -40,6 +38,7 @@ export class LightingVolume { | |
| private _positions: Float32Array; | ||
| private _indices: number[]; | ||
| private _needFullUpdateUBO = true; | ||
| private _webGPUReady = false; | ||
|
|
||
| private _shadowGenerator?: ShadowGenerator; | ||
| /** | ||
|
|
@@ -65,11 +64,7 @@ export class LightingVolume { | |
| this._createFallbackTextures(); | ||
| } | ||
|
|
||
| const depthTexture = this._shadowGenerator.getShadowMap()?.depthStencilTexture; | ||
| if (this._engine.isWebGPU && depthTexture) { | ||
| this._cs!.setInternalTexture("shadowMap", depthTexture); | ||
| this._cs2!.setInternalTexture("shadowMap", depthTexture); | ||
| } | ||
| this._setComputeShaderInputs(); | ||
| } | ||
|
|
||
| private _tesselation = 0; | ||
|
|
@@ -168,6 +163,15 @@ export class LightingVolume { | |
| scene.meshes.splice(scene.meshes.indexOf(this._mesh), 1); | ||
|
|
||
| if (this._engine.isWebGPU) { | ||
| // eslint-disable-next-line github/no-then | ||
| void import("../ShadersWGSL/lightingVolume.compute").then(() => { | ||
|
Member
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. 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?
Member
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. Isn't this WebGPU only? If so, then is there any reason to do a dynamic import?
Contributor
Author
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. 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 |
||
| this._webGPUReady = true; | ||
| this._createComputeShader(); | ||
| if (this._indices.length === 0) { | ||
| this._createGeometry(); | ||
| } | ||
| }); | ||
|
|
||
| this._uBuffer = new UniformBuffer(this._engine); | ||
|
|
||
| this._uBuffer.addUniform("invViewProjMatrix", 16); | ||
|
|
@@ -178,8 +182,6 @@ export class LightingVolume { | |
| this._uBuffer.addUniform("orthoMin", 3); | ||
| this._uBuffer.addUniform("orthoMax", 3); | ||
| this._uBuffer.update(); | ||
|
|
||
| this._createComputeShader(); | ||
| } else { | ||
| this._copyTexture = new CopyTextureToTexture(this._engine, false, true); | ||
| this._createFallbackTextures(); | ||
|
|
@@ -201,6 +203,9 @@ export class LightingVolume { | |
| if (this._cs2) { | ||
| isReady = this._cs2.isReady() && isReady; | ||
| } | ||
| if (this._engine.isWebGPU && !this._webGPUReady) { | ||
| isReady = false; | ||
| } | ||
| return isReady; | ||
| } | ||
|
|
||
|
|
@@ -209,7 +214,7 @@ export class LightingVolume { | |
| * @param forceUpdate If true, forces the update even if the frequency condition is not met. | ||
| */ | ||
| public update(forceUpdate = false) { | ||
| if (this._tesselation === 0 || !this._shadowGenerator) { | ||
| if (this._tesselation === 0 || !this._shadowGenerator || (this._engine.isWebGPU && !this._webGPUReady)) { | ||
| return; | ||
| } | ||
|
|
||
|
|
@@ -300,21 +305,29 @@ export class LightingVolume { | |
| entryPoint: "updatePlaneVertices", | ||
| }); | ||
|
|
||
| this._setComputeShaderInputs(); | ||
| } | ||
|
|
||
| private _setComputeShaderInputs() { | ||
| if (!this._engine.isWebGPU) { | ||
| return; | ||
| } | ||
|
|
||
| if (this._shadowGenerator) { | ||
| const depthTexture = this._shadowGenerator.getShadowMap()?.depthStencilTexture; | ||
| if (depthTexture) { | ||
| this._cs.setInternalTexture("shadowMap", depthTexture); | ||
| this._cs2.setInternalTexture("shadowMap", depthTexture); | ||
| this._cs?.setInternalTexture("shadowMap", depthTexture); | ||
| this._cs2?.setInternalTexture("shadowMap", depthTexture); | ||
| } | ||
| } | ||
|
|
||
| if (this._uBuffer) { | ||
| this._cs.setUniformBuffer("params", this._uBuffer); | ||
| this._cs2.setUniformBuffer("params", this._uBuffer); | ||
| this._cs?.setUniformBuffer("params", this._uBuffer); | ||
| this._cs2?.setUniformBuffer("params", this._uBuffer); | ||
| } | ||
| if (this._storageBuffer) { | ||
| this._cs.setStorageBuffer("positions", this._storageBuffer); | ||
| this._cs2.setStorageBuffer("positions", this._storageBuffer); | ||
| this._cs?.setStorageBuffer("positions", this._storageBuffer); | ||
| this._cs2?.setStorageBuffer("positions", this._storageBuffer); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -466,7 +479,7 @@ export class LightingVolume { | |
| private _createGeometry() { | ||
| const light = this._light; | ||
|
|
||
| if (!light) { | ||
| if (!light || (this._engine.isWebGPU && !this._webGPUReady)) { | ||
| return; | ||
| } | ||
|
|
||
|
|
@@ -490,8 +503,7 @@ export class LightingVolume { | |
|
|
||
| this._mesh.setVerticesBuffer(new VertexBuffer(webGPUEngine, this._storageBuffer.getBuffer(), "position", { takeBufferOwnership: false }), true, vertexNumber); | ||
|
|
||
| this._cs!.setStorageBuffer("positions", this._storageBuffer); | ||
| this._cs2!.setStorageBuffer("positions", this._storageBuffer); | ||
| this._setComputeShaderInputs(); | ||
|
|
||
| this._cs!.triggerContextRebuild = true; | ||
| this._cs2!.triggerContextRebuild = true; | ||
|
|
@@ -505,7 +517,7 @@ export class LightingVolume { | |
| private _updateGeometry() { | ||
| const light = this._light; | ||
|
|
||
| if (!light || !this._shadowGenerator) { | ||
| if (!light || !this._shadowGenerator || (this._engine.isWebGPU && !this._webGPUReady)) { | ||
| return; | ||
| } | ||
|
|
||
|
|
||
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.
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 reasonunknownwould be a problem in this context).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.
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.