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

New Material Type For Threejs #28543

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from
Open

New Material Type For Threejs #28543

wants to merge 5 commits into from

Conversation

ManishJu
Copy link
Contributor

@ManishJu ManishJu commented Jun 2, 2024

This material is a much better approximation for diffuse materials for dusty/porous
surfaces that exhibits back-scattering and saturation effects that are missing from prior diffuse models like Lambert. It has a very low cost as compared to MeshStandard /MeshPhysical material. Can be used as a cheap alternative for surfaces like skin/clouds/pots...etc .

The method :

It has been directly taken from the recent 2021 paper : https://d1qx31qr3h6wln.cloudfront.net/publications/lambertsphereBRDF.pdf

111

Copy link

github-actions bot commented Jun 2, 2024

📦 Bundle size

Full ESM build, minified and gzipped.

Filesize dev Filesize PR Diff
678.5 kB (168.2 kB) 684.7 kB (168.8 kB) +6.16 kB

🌳 Bundle size after tree-shaking

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

Filesize dev Filesize PR Diff
456.7 kB (110.3 kB) 462.6 kB (111 kB) +5.95 kB

@ManishJu
Copy link
Contributor Author

ManishJu commented Jun 2, 2024

Can somebody help me with the unit test. Thanks !

@Mugen87
Copy link
Collaborator

Mugen87 commented Jun 3, 2024

Looking at the code, it seems the computations are less expensive than the standard material but more expensive than the normal lambert implementation. So I don't think it would be a candidate for replacing the so far cheap lambert material.

Besides, do you think it's possible to implement this in examples/jsm/materials instead.

Or even better, do you think you can implement this material with the new node based material system? If you write the shader code with TSL, it will work for WebGL and WebGPU at the same time.

At the current state of the project, it's probably better to not add more default materials in src (meaning just for WebGLRenderer) unless there is a good reason. TBH, I don't see this for MeshSphereLambertMaterial so far.

@ManishJu
Copy link
Contributor Author

ManishJu commented Jun 3, 2024

Looking at the code, it seems the computations are less expensive than the standard material but more expensive than the normal lambert implementation. So I don't think it would be a candidate for replacing the so far cheap lambert material.

Yes that is true but it should not be candidate for replacement for MeshLambertMaterial. As that is extremely cheap, this is just a few steps more but a lot better approximation .

Besides, do you think it's possible to implement this in examples/jsm/materials instead.

I think I can do that but should not it be a new materia in the Three.js library?

Or even better, do you think you can implement this material with the new node based material system? If you write the shader code with TSL, it will work for WebGL and WebGPU at the same time.

I have not looked into the node based material yet. I will look into it .

At the current state of the project, it's probably better to not add more default materials in src (meaning just for WebGLRenderer) unless there is a good reason. TBH, I don't see this for MeshSphereLambertMaterial so far.

Sure, I feel it's a good candidate as a separate cheap back-scattering material . Tell me if you feel otherwise .

@WestLangley
Copy link
Collaborator

Related #8354.

I think it would be nice to have a diffuse material that exhibits retro-reflection at grazing angles. The moon has often been used as a canonical use case.

Some interesting images here: https://www.rombo.tools/2021/12/15/lambert-sphere/.

If the material could be added as a Node Material, I think that would be great. If you need help, there are contributors here who can help you.

@Mugen87
Copy link
Collaborator

Mugen87 commented Jun 3, 2024

I think I can do that but should not it be a new materia in the Three.js library?

Well, not in the core. But putting it in examples/jsm/materials will make it usable like MeshGouraudMaterial which should be sufficient.

That said, a node-based implementation would be the preferred, more future-proof approach.

vec3 wo = directLight.direction;
vec3 wi = geometryViewDir;
vec3 irradiance = shadeLambertianSphereBRDF(wi,wo,geometryNormal,kd);
reflectedLight.directDiffuse += irradiance; // no need to multiply with BRDF_Lambert( material.diffuseColor );
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is going to need to be corrected.

The quantity on the left of the equals sign has units of radiance, but the quantity on the right you are calling irradiance. So that is incorrect.

(The BRDF has units of inverse steradians, and converts the irradiance units to radiance units.)

Units matter.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Changing a variable name does not fix the problem. The units on the left side and right side of the equals sign must match.


}
/* struct IncidentLight {
vec3 color;
Copy link
Collaborator

Choose a reason for hiding this comment

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

The color and intensity of the incident light is missing from this implementation.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is fixed by modifying the implementation to support the incident light color and intensity. It is not fixed by removing a comment.

@@ -9,6 +9,7 @@ import { MeshPhongMaterial } from './MeshPhongMaterial.js';
import { MeshToonMaterial } from './MeshToonMaterial.js';
import { MeshNormalMaterial } from './MeshNormalMaterial.js';
import { MeshLambertMaterial } from './MeshLambertMaterial.js';
import { MeshSphereLambertMaterial } from './MeshSphereLambertMatertial.js';
Copy link
Contributor

Choose a reason for hiding this comment

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

- import { MeshSphereLambertMaterial } from './MeshSphereLambertMatertial.js';
+ import { MeshSphereLambertMaterial } from './MeshSphereLambertMaterial.js';

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.

None yet

4 participants