Skip to content
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

WebGPURenderer: implement compileAsync() #27098

Merged
merged 7 commits into from
Jan 23, 2024
Merged

Conversation

aardgoose
Copy link
Contributor

Add compileAsync( scene, camera ) for WebGPU and WebGL backends.

With WebGL the KHR_parallel_shader_compile extension is used if available.

Added to webgpu_occlusion example to test/demonstrate use.

Note: targetScene support to be added

@aardgoose aardgoose force-pushed the compile-async branch 2 times, most recently from 9893d36 to 2ec5d83 Compare January 1, 2024 21:09
@RenaudRohlinger RenaudRohlinger added this to the r161 milestone Jan 7, 2024
@RenaudRohlinger
Copy link
Collaborator

Looks good to me.
Just added a commit to forward the compile -> compileAsync via getter.

In the WebGLRenderer we throttle the check of completion with setTimeout(()=>{}, 10). In my opinion using a RAF is just fine so I would let it as it is in the WebGLBackend.

I will let you decide if we merge this one or not as it affects the WebGPU side @sunag.

@sunag
Copy link
Collaborator

sunag commented Jan 7, 2024

I think that since compileAsync() will call updateBefore() in many situations we will have multiples render() calls being executed during a compileAsync() process. I think compileAsync() should force the compilation of the entire consecutive process of the render tree and avoid overloading the frame in which it was called that could happen in some cases.

@aardgoose
Copy link
Contributor Author

@RenaudRohlinger I Overwrote you commit by mistake, To you want to re add.

@RenaudRohlinger
Copy link
Collaborator

@aardgoose if I remember correctly the only difference was that I replaced:

async compile( /*scene, camera, targetScene = null */ ) {

		console.warn( 'THREE.Renderer: .compile() is not implemented yet.' );

}

to:

get compile() { // @deprecated, r170

      console.warn( 'THREE.WebGPURenderer: The property .compile has been deprecated. Use .compileAsync instead.' );
      return this.compileAsync;

}

In order to follow the best practice of the repo.

Are we good regarding the comment of @sunag?

@sunag
Copy link
Collaborator

sunag commented Jan 23, 2024

I restored the commits, I ended up saving because of a small correction I made. Let's merge :)

@sunag sunag merged commit 3449f17 into mrdoob:dev Jan 23, 2024
11 checks passed
@@ -47,6 +47,7 @@ class WebGLBackend extends Backend {
this.discard = false;

this.extensions.get( 'EXT_color_buffer_float' );
this.parallel = this.extensions.get( 'KHR_parallel_shader_compile' );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really like that we are introducing a new property just to get a constant... Maybe at least mark it private?


get compute() {

return this.computeAsync;
console.warn( 'THREE.Renderer: compile() is deprecated and will be removed in r170, use compileAsync instead.' ); // @deprecated, r170
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that all other non-async methods just point to their async counterparts, why should we deprecate this one? /ping @sunag @aardgoose @RenaudRohlinger

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants