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

Add access to WebXR depth texture + add example #27695

Merged
merged 2 commits into from
Jun 1, 2024

Conversation

cabanier
Copy link
Contributor

@cabanier cabanier commented Feb 6, 2024

Some authors want to use the depth texture to add an effect using a custom shader.
This diff enhances the WebXR code to allow access to the texture and add a sample.

Thanks to @hybridherbst !

This contribution is funded by Meta

Copy link

github-actions bot commented Feb 6, 2024

📦 Bundle size

Full ESM build, minified and gzipped.

Filesize dev Filesize PR Diff
677.9 kB (168.1 kB) 678 kB (168.1 kB) +98 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Filesize dev Filesize PR Diff
456 kB (110.1 kB) 456.1 kB (110.1 kB) +98 B

@cabanier
Copy link
Contributor Author

cabanier commented Feb 6, 2024

@Mugen87 is there a way to change an existing shader? I'd like to reuse MeshLambertMaterial by adding my helper functions and inserting the calculation of the edge pixels.

@cabanier cabanier marked this pull request as ready for review February 6, 2024 23:54
@gkjohnson
Copy link
Collaborator

@Mugen87 is there a way to change an existing shader? I'd like to reuse MeshLambertMaterial by adding my helper functions and inserting the calculation of the edge pixels.

You can use onBeforeCompile and modify the existing shader with find and replace:

material.onBeforeCompile = shader => {
  
  shader.uniforms.depthTex = { value: /* texture here */ };
  shader.fragmentShader = shader.fragmentShader
    .replace( /#include<color_fragment>/g, match => /* glsl */`
      
      // your code here
      ${ match }

    ` );

};

This should also address #27664

@Mugen87 Mugen87 added this to the r162 milestone Feb 7, 2024
@hybridherbst
Copy link
Contributor

If you want it globally included you can also replace things in ShaderChunks.someSpecificChunk, then it doesn't need to be modified in every single material.
What I'm not sure in that second approach is what the right way is to set some uniforms either for all objects or per object; @gkjohnson do you have a recommendation there?

@gkjohnson
Copy link
Collaborator

If you want it globally included you can also replace things in ShaderChunks.someSpecificChunk, then it doesn't need to be modified in every single material.

Generally I'd discourage modifying the built in shaders in the examples. It works but it's brittle. And of course having multiple libraries or components modifying built in materials can easily lead to overwritten changes and breakage.

What I'm not sure in that second approach is what the right way is to set some uniforms either for all objects or per object;

Global uniforms aren't supported so you'll have to set it per material. UBOs are now in core but the current implementation doesn't make this case much more ergonomic. See #16922.

Encapsulating the above in a class can make this easier to manage at an application level but I don't think it's necessary for the example:

class CustomMaterial extends MeshStandardMaterial {
  set depthTex( v ) { 
    this._uniforms.depthTex.value = v;
  }
  constructor( ...args ) {
    super( ...args );
    this.onBeforeCompile = shader => {
      // ...
      this._uniforms = shader.uniforms;
    };
  }

}

@hybridherbst
Copy link
Contributor

having multiple libraries or components modifying built in materials

The same is true for Material.onBeforeCompile which isn't an event listener but a single assigned method, or am I understanding that wrong?

I'm just not sure how someone could take that sample and extend it; a common flow would be "this is a cool sample, now I want to load a glTF and keep the materials in that and apply that fancy occlusion." – how would someone do that? Need to be super careful then to not "miss" any material to modify it, can't use other onBeforeCompile callbacks, etc.

@gkjohnson
Copy link
Collaborator

gkjohnson commented Feb 7, 2024

The same is true for Material.onBeforeCompile which isn't an event listener but a single assigned method, or am I understanding that wrong?

You can design for this case into your app or design your wrapper class to take an onBeforeCompile function and call it if it's set (ie adding a getter / setter for onBeforeCompile to your class)

I'm just not sure how someone could take that sample and extend it; a common flow would be "this is a cool sample, now I want to load a glTF and keep the materials in that and apply that fancy occlusion."

This is always the case if you want to use a different material on a loaded model. My understanding is that onBeforeCompile is the recommended way to modify built in materials but don't get me wrong - I think it's more difficult than it should be right now (see my issues on override materials, global uniforms, etc). I'd like to see improvements but I think discussions around how to adjust the ergonomics should happen in a separate thread and not block this PR.

@cabanier
Copy link
Contributor Author

@Mugen87 it doesn't look like I have access to the internal shaders to extend them. What do you think of this change?
Alternatively, I can just make the changes to the webxr manager class and leave it up to authors to use the depth sensing texture.

@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 12, 2024

I would leave the PR in its current state then 👍 .

@mrdoob mrdoob modified the milestones: r162, r163 Feb 29, 2024
@odysseyjason
Copy link

Is it possible to query the depth texture per frame from the render-loop, to for instance bounce objects off of walls sensed through the depth.api? (the webxr cubes example currently has the cubes getting occluded behind walls, would it be possible to query the depth data to see if they should bounce off of them instead?)

I have looked at the code from the webxr cubes example and WebXRDepthSensing.js and WebXRManager.js but could not find where it was being defined if this depth data was GPU optimized texture or CPU optimized or understand where one could interject real time interaction possibilities with the depth data.

Any example that could show interaction with the Depth api texture data would be most helpful! Also, a CPU optimized depth texture example would be great!

@cabanier
Copy link
Contributor Author

Any example that could show interaction with the Depth api texture data would be most helpful! Also, a CPU optimized depth texture example would be great!

If you look at this PR, the getDepthTexture call returns you the depth texture. You can use it in your shader just like a regular WebGL texture.

@mrdoob mrdoob modified the milestones: r163, r164 Mar 29, 2024
@mrdoob mrdoob modified the milestones: r164, r165 Apr 25, 2024
@Mugen87
Copy link
Collaborator

Mugen87 commented May 30, 2024

@cabanier Do you mind rebasing this PR? Sorry for overlooking it. But now that depth sensing is fixed via #28530 the PR can be eventually merged.

@cabanier
Copy link
Contributor Author

@cabanier Do you mind rebasing this PR? Sorry for overlooking it. But now that depth sensing is fixed via #28530 the PR can be eventually merged.

I can do that!

@mrdoob mrdoob modified the milestones: r165, r166 May 31, 2024
@Mugen87
Copy link
Collaborator

Mugen87 commented May 31, 2024

It seems the E2E screenshot of the new example requires an update. Can you quickly regenerate the screenshot and see if it helps?

If regenerating on your side does not fix the failure, I'll try it myself after the merge.

@cabanier
Copy link
Contributor Author

It seems the E2E screenshot of the new example requires an update. Can you quickly regenerate the screenshot and see if it helps?

Would you mind regenerating it? I developer on a headless server and it doesn't have chrome.

@Mugen87 Mugen87 merged commit 78009b4 into mrdoob:dev Jun 1, 2024
11 of 12 checks passed
@Mugen87
Copy link
Collaborator

Mugen87 commented Jun 1, 2024

Would you mind regenerating it?

Np, see #28535.

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.

7 participants