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

TSL: Fix texture_depth_2d in wgslFn #27323

Merged
merged 1 commit into from
Dec 6, 2023
Merged

Conversation

sunag
Copy link
Collaborator

@sunag sunag commented Dec 6, 2023

Related issue: #27274 (comment)

Description

Fix texture_depth_2d node reference if used as parameter in wgslFn.

@sunag sunag marked this pull request as ready for review December 6, 2023 02:37
@sunag sunag added this to the r160 milestone Dec 6, 2023
@sunag sunag merged commit f4b09ec into mrdoob:dev Dec 6, 2023
11 checks passed
@sunag sunag deleted the dev-fix-texture_depth_2d branch December 6, 2023 03:00
@Spiri0
Copy link
Contributor

Spiri0 commented Dec 28, 2023

Unfortunately, when I try to use the DepthTexture in wgslFn, I get the following error message:

WebGPUBindingUtils.js:134 Uncaught (in promise) TypeError: Failed to execute 'writeBuffer' on 'GPUQueue': parameter 1 is not of type 'GPUBuffer'.
    at WebGPUBindingUtils.updateBinding (WebGPUBindingUtils.js:134:16)
    at WebGPUBackend.updateBinding (WebGPUBackend.js:1013:21)
    at Bindings._update (Bindings.js:121:14)
    at Bindings.updateForRender (Bindings.js:73:8)
    at WebGPURenderer._renderObjectDirect (Renderer.js:1020:18)
    at WebGPURenderer.renderObject (Renderer.js:985:9)
    at WebGPURenderer._renderObjects (Renderer.js:935:10)
    at WebGPURenderer.render (Renderer.js:336:40)
    at ThreeJSController.Render (threejs-component.js:94:18)
    at main.js:212:31
const depthTexture = new THREE.DepthTexture();
depthTexture.type = THREE.FloatType;

const testWGSL = wgslFn(`
    fn testWGSL(
      depth: texture_depth_2d
    ) -> vec4<f32> {

      return vec4<f32>(1, 0, 0, 1);  //just to see something if the shader work
    }
`);
  
const shaderParams = {
    depth: texture(depthTexture)
}

const material = new MeshBasicNodeMaterial();
material.colorNode = testWGSL(shaderParams);
  
const geometry = new THREE.BoxGeometry( 1, 1, 1 );
const cube = new THREE.Mesh( geometry, material ); 
scene.add( cube );

As soon as I change "new THREE.DepthTexture();" to a normal texture "new THREE.Texture();" and the "texture_depth_2d" to "texture_2d" the test code goes.
Is the error code helpful?

@sunag
Copy link
Collaborator Author

sunag commented Dec 28, 2023

Hmm.. this is other problem releated, seems that DepthTexture needs to be on a RenderTarget already used once.

@Spiri0
Copy link
Contributor

Spiri0 commented Dec 28, 2023

I also tried this with the renderTarget setup from the WebGPU DepthTexture example. The example was my starting point.
DepthTextures seems to be a bit special if you want to get them into a wgslFn.
Well, I see it positively. The WebGPU integration has made enormous progress in half a year and when I look back at r154, r160 is a huge difference.

@Spiri0
Copy link
Contributor

Spiri0 commented Dec 29, 2023

I couldn't find your comment in the forum in which you mentioned that you would like to port the sky example to WebGPU otherwise I would have mentioned it there. I did that today.

sky-webgpu.PNG

Only the two includes in the colorNode wgslFn are missing because I didn't know what the wgsl equivalent was

#include <tonemapping_fragment>
#include <colorspace_fragment>

Do you want that?
I included the cubemap dynamic because I wanted to know how it works with the sky, but you can throw that out again.
Or is Mugen87 the right contact for something like that?
I have no idea what the procedure is to contribute something to three.js.

Does what you mentioned mean that additional extensions are needed for the DepthTexture (texture_depth_2d) for wgslFn?
Then it would be a good opportunity to do the same also for the remaining 3 DepthTexture types (texture_depth_2d_array, texture_depth_cube, texture_depth_cube_array)
Then it would be fully integrated.

@sunag
Copy link
Collaborator Author

sunag commented Dec 30, 2023

Oh great! What I imagined was create a Node called SkyNode to be used in scene.backgroundNode:

scene.backgroundNode = new SkyNode( ...params );

It would be great to have this in TSL.

About the includes: <tonemapping_fragment>, <colorspace_fragment> you shouldn't worry since this is being used in scene.backgroundNode.

@Spiri0
Copy link
Contributor

Spiri0 commented Dec 31, 2023

The SkyNode thing sounds good.

I loaded a cubeTexture for testing. When I pass this to a wgslFn, the texture type is correct with

texture_cube<f32>

but the console shows lots of warnings about the wrong binding dimension.
I haven't included the console warning here because I'm satisfied with one miracle per new release and the depth textures would be more important to me, and I know I'm not the only one with issues. However, I wanted to mention it because WebGPU is also a personal concern for me. For a photorealistic result like in unity, I need an enormous number of functions from wgsl. But what was previously only possible in unity can now also be done with three.js with WebGPU. If you wish I can make a small CodePen for this.

But now it's New Year's Eve and its time to take a break and toast to all the successful expansions. I also know unity water codes and my current status with three.js is already very advanced. With correct refraction (depthTextures) and correct reflection (cube textures) and post-processing (underwater), three.js will produce just as beautiful results as in the well-known and popular sea games. But that has time in the new year. I wish you a happy new year.

AdaRoseCannon pushed a commit to AdaRoseCannon/three.js that referenced this pull request Jan 15, 2024
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.

2 participants