mrdoob / three.js

JavaScript 3D Library.
https://threejs.org/
MIT License
99.3k stars 35.12k forks source link

Feature Request: OnBeforeRender on Material #21305

Open DavidPeicho opened 3 years ago

DavidPeicho commented 3 years ago

Is your feature request related to a problem? Please describe.

Let's say I have a custom material that is shared by two meshes. This material uses the inverse transform of the mesh. In this case, I have no way to send the inverse transform because it's dependent from the actual bound mesh.

When creating an app with Three.js, it's not really an issue because we control the flow of data. However, when wrapping Three.js in another library, it makes it somewhat difficult to share a ShaderMaterial whose parameters aren't independent from the camera / mesh drawn.

Describe the solution you'd like

On the top of my mind:

Describe alternatives you've considered

Additional context

This is highly related to the issue #16922. I think the problem is similar.

mrdoob commented 3 years ago
  • Use onBeforeRender on each meshes. Annoying because all meshes using this material must be updated and a user would need to attach the callback himself

I was going to suggest that...

This whole onBefore*, onAfter* part of the API is ugly and fragile... Have you tested NodeMaterial?

DavidPeicho commented 3 years ago

I haven't because I will most likely just add my entire GLSL in one node 😄 Also Right now I like how my shaders are clearly separated and I just use includes. Maybe I should take a closer look at NodeMaterial, any feature there that could help me achieve that on a material level?

I wish I could just do it on the meshes but that's not that easy, because I just don't know in advanced where my material will be attached.

It's fragile I agree, but right now what's happening is that there is a custom path basically for the default materials. Ideally I think it would be nice to be able to do exactly the same thing with any custom material, including having a way to modify transforms / camera.

mrdoob commented 3 years ago

@sunag how do you think this could be done with NodeMaterial?

sunag commented 3 years ago

@sunag how do you think this could be done with NodeMaterial?

Yes. This already is possible in the new NodeMaterial system. https://github.com/mrdoob/three.js/blob/ea11ddad1e92e2cfdb1b6a03ba30225fe6fdef2a/examples/jsm/renderers/nodes/accessors/ModelNode.js#L17

You can update a shared uniform for each render object. https://github.com/mrdoob/three.js/blob/ea11ddad1e92e2cfdb1b6a03ba30225fe6fdef2a/examples/jsm/renderers/nodes/accessors/ModelNode.js#L39-L55

Is possible update per frame too with objects like camera https://github.com/mrdoob/three.js/blob/ea11ddad1e92e2cfdb1b6a03ba30225fe6fdef2a/examples/jsm/renderers/nodes/accessors/CameraNode.js#L16

sunag commented 3 years ago

I can implement a solution for WebGL too after this PR ( https://github.com/mrdoob/three.js/pull/21117 )... I am currently implementing a light system for selective lights or global using NodeMaterial.

DavidPeicho commented 3 years ago

I had a look a bit at the Node system today. I still need to connect myself the mesh to the update though no? I don't see how that would work automatically.

Where is the frame.object attribute field?

EDIT: Ok my bad, now I understand the code to hook that up is only for WebGPU right now

sunag commented 3 years ago

https://raw.githack.com/sunag/three.js/nodematerial-shared-uniforms/examples/webgpu_instance_uniform.html

Source: https://github.com/sunag/three.js/blob/8f9cbfa9c15d45a5ce8d3edba8cebc246c039592/examples/webgpu_instance_uniform.html

I made a example, the color is updated per mesh.color in render object moment. But it seems there is a problem... WebGPURenderer still does not support material instance? @Mugen87

Mugen87 commented 3 years ago

WebGPURenderer still does not support material instance?

What do you mean with "material instance"?

sunag commented 3 years ago

What do you mean with "material instance"?

A single material for multiples meshes.

sunag commented 3 years ago

If replace materialTemp to material in addMesh() only a mesh is rendered on screen at a time. https://github.com/sunag/three.js/blob/8f9cbfa9c15d45a5ce8d3edba8cebc246c039592/examples/webgpu_instance_uniform.html#L104-L109

Mugen87 commented 3 years ago

Okay, after investigating this issue a refactoring of how bindings are managed is required. Right now, the binding and the actual data are handled in a single object. In the below example, bufferGPU represents the UBO and array the actual data.

https://github.com/mrdoob/three.js/blob/cb99e7e08cc97cb7a22f17e32cc7a22082022d67/examples/jsm/renderers/webgpu/WebGPUUniformsGroup.js#L22-L23

If a material is shared among multiple 3D objects, there is only one instance of bindings objects and thus only one data storage. This is not a problem for textures (because textures are defined on material level) however model related data like the MVP matrix can't be processed this way. They have to be managed per 3D object and the UBO needs to be updated per object with the correct data.

I need some time to think about a possible solution for this...

DavidPeicho commented 3 years ago

@sunag let me know if you need any help for the WebGL version (or even WebGPU).

sunag commented 3 years ago

@Mugen87 I found the problem, is related with the implementation of NodeMaterial... I will create um PR to fix this.

Mugen87 commented 3 years ago

I'm curious about your PR 🤓. This can also be fixed when multiple WebGPUUniformsGroup objects share a single bufferGPU. The render and material system just have to inject an existing UBO that is shared across multiple 3D objects. It is not required that such a UBO needs to exist per 3D object. If multiple 3D objects share a program, they can also share all WebGPU resources.

I'm waiting now with my PR until I've seen your change.

sunag commented 3 years ago

They have to be managed per 3D object and the UBO needs to be updated per object with the correct data.

I mean in case of just update the uniforms with the new mesh and draw again with the same material and uniforms but objects differents... This approach work with WebGL but with WebGPU I never tested.

Mugen87 commented 3 years ago

Ah okay, I only focus on WebGPU in this topic^^.

sunag commented 3 years ago

I'm curious about your PR

I did not do anything particular... I just made WebGPUNodes work via .get( objects ) instead of .get( material ). A NodeBuilder for each object.

Mugen87 commented 3 years ago

If this works, it's a good start! Fell free to file the PR 👍

I'd like to find out if it is possible to further optimize the renderer and material system by only having different bindings if required. However, this will take a while and some experimenting.

I guess we need similar logic anyway if objects should be able to share render pipelines.

DavidPeicho commented 3 years ago

Any news on the WebGL side? I am available few hours a week to help on that 😄

Mugen87 commented 3 years ago

@DavidPeicho In context of WebGL, we have to solve #21117 first.

DavidPeicho commented 2 years ago

Any news here now that #21117 is solved? If you need any help to go forward with that, let me know what you want me to work on and I can give some of my time for that!

WestLangley commented 2 years ago

@DavidPeicho

This material uses the inverse transform of the mesh

If the transform is unscaled, this may help you avoid passing the inverse transform to the shader: https://en.wikibooks.org/wiki/GLSL_Programming/Applying_Matrix_Transformations

DavidPeicho commented 2 years ago

@WestLangley thanks for the link but it was just one example out of several. It can happen that some data comes from the mesh and isn't easily accessible.

Mugen87 commented 2 years ago

The idea is to apply the pattern from the following WebGPU example to WebGLRenderer.

https://threejs.org/examples/webgpu_instance_uniform

However, this requires a deeper integration of the node material into WebGLRenderer so it's possible to process node updates per frame. #21117 was a first step but WebGLRenderer needs to call a similar method like below (which is currently used by WebGPURenderer) so it can utilize more features of the new node system:

https://github.com/mrdoob/three.js/blob/8837fac2a910f8ed18d956e53963e197db6c3074/examples/jsm/renderers/webgpu/nodes/WebGPUNodes.js#L44-L61

Mugen87 commented 2 years ago

WebGPURenderer uses a node system by default. However, we can not include the entire node system in the main build of three.js so we have to find another way for WebGLRenderer.

Right now, it's required that the app includes a certain file (WebGLNodes). However, I'm thinking about a different approach based on dependency injection. Meaning if users want to use the node path of WebGLRenderer, they would have to call a new method that injects node classes for further usage.

DavidPeicho commented 2 years ago

Thanks for the info.

If I sum up, right now we simply need to inject the same method that would allow to do the update? Because the build looks already injected by WebGLNodes.

Otherwise the other type of injection sounds nice. Something like renderer.setNodeBuilder() would be great.

I recently tried to start new shaders using the node system, but I have to admit it's a bit difficult and time consuming to use in WebGL in its current state.

sunag commented 2 years ago

@DavidPeicho Look like we have a WebGL version for that :) https://github.com/mrdoob/three.js/pull/22504

DavidPeicho commented 2 years ago

That's amazing news, thanks! I guess now I will have to convert my custom materials to the node system

DavidPeicho commented 2 years ago

Material.onBeforeRender is actually implemented now. Howerver, it's missing some doc. Will try to make a PR.

Mugen87 commented 4 months ago

Unfortunately, you can't use Material.onBeforeRender() to update per-object uniforms. Although both spheres use Material.onBeforeRender() as intended, they share the same opacity value depending on the render order.

https://jsfiddle.net/03xm49dp/

Material.onBeforeRender() was only introduced to make the new node material system work with WebGLRenderer. At some point, we should maybe stop this support and refer devs to WebGPURenderer with its WebGL backend instead. And then remove Material.onBeforeRender() since I don't think it adds any additional value (and right now produces only confusing).

LeviPesin commented 3 months ago

At some point, we should maybe stop this support 

I agree with stopping the support for WebGLRenderer+Nodes (regulated by the renderers/webgl-legacy/WebGLNodeBuilder.js). @sunag What do you think? IIRC it is in the legacy directory for quite a some time now, so isn't it time to officially announce its deprecation and then remove it after 10 releases?