`ambientColor` is not worked in PBRMaterial

yar2001 · July 28, 2024, 10:18am

Hi everyone. It seems that .ambientColor does not work in PBRMaterial. Here is the reproduction:

Is it expected or am I missing something? Thanks for reading!

roland · July 28, 2024, 12:25pm

Hi!
materialPBR.metallic = 0 otherwise it reflects the non existing environment.

yar2001 · July 28, 2024, 2:12pm

Thanks for your reply! After adding .metallic=0, the .ambientcolor does work.

However, I find that the parameter of .metallic in the inspector panel is inconsistent.

What is the default value of .metallic? If I comment the line of setting metallic to 0, the inspector still shows that the metallic value is 0. However, when I uncommented the code, the rendering results changed.

From what I’ve seen, if I hadn’t added this line, the rendering result would be like the
metallic value is 1. I finally knew why I couldn’t reproduce the rendering result of Blender. Why the default value is 1? This is unusual and seems not to be documented.

It also means that each PBRMaterial in Babylon.js should manually set metallic to 0 which is really unexpected since Three.js, Unity, Blender, and other 3D software all set PBR metalness to 0 by default.

Here is the playground you can replay:

roland · July 29, 2024, 7:45am

metallic is null by default, that’s why you need to set a value.

yar2001 · July 29, 2024, 9:15am

Yes. When the .metallic value is null, it will eventually set the value to 1. From this perspective, if we don’t set the value, its effect is equal to setting the value to 1.

github.com

BabylonJS/Babylon.js/blob/c331cf2714f60276791eb60a5ac225651daec3c7/packages/dev/core/src/Materials/PBR/pbrBaseMaterial.ts#L2181


      
              }

          }

          

          // Point size

          if (this.pointsCloud) {

              ubo.updateFloat("pointSize", this.pointSize);

          }

          

          // Colors

          if (defines.METALLICWORKFLOW) {

              TmpColors.Color3[0].r = this._metallic === undefined || this._metallic === null ? 1 : this._metallic;

              TmpColors.Color3[0].g = this._roughness === undefined || this._roughness === null ? 1 : this._roughness;

              ubo.updateColor4("vReflectivityColor", TmpColors.Color3[0], 1);

          

              const ior = this.subSurface?._indexOfRefraction ?? 1.5;

              const outsideIOR = 1; // consider air as clear coat and other layers would remap in the shader.

          

              // We are here deriving our default reflectance from a common value for none metallic surface.

              // Based of the schlick fresnel approximation model

              // for dielectrics.

              const f0 = Math.pow((ior - outsideIOR) / (ior + outsideIOR), 2);

Is it possible to improve this behavior? It is a little bit uncommon and I guess it may confuse some users.

roland · July 29, 2024, 10:08am

There could be so many apps running in production which relies on the default value of 1 already. I don’t see this as a technical bug more like a discrepancy from the specification. I’d like to have the default value as written in the specs too, however it would introduce a breaking change.

Let’s wait for the others…

yar2001 · July 29, 2024, 1:17pm

OK. The ambientColor problem is solved, and it works on PBRMaterial after setting the .metallic value to 0.

For the PBR default metallic value problem, I’m posting it at: PBR default metallic value is not 0 which is unusual

And notice that the inspector’s inconsistent metallic display value issue is still not resolved.

sebavan · July 29, 2024, 4:44pm

This is only the case if you have a texture so the texture value multiplies with 1 ensuring you can set the texture only. In other cases you won t use the metallic workflow.

We won t change the default as it is too big of a breaking change.

yar2001 · July 30, 2024, 1:23am

Hi, thank you for your reply. The playground posted above explains that if an empty PBRMaterial is created, its rendering result is equivalent to metallic set to 1. So it doesn’t seem to matter if there’s texture. When a user creates PBRMaterial without any other operation, its metallic effect is equal to 1. I think this behavior may be clarified in the documentation.

sebavan · July 30, 2024, 5:57am

By default you are using the Spec/Gloss workflow, those steps are needed to use a metal/rough one:
https://doc.babylonjs.com/features/featuresDeepDive/materials/using/masterPBR#from-metallicroughness-to-pbrmaterial

In spec/gloss the reflectivityColor (aka specular) defaults to white.

I wonder whether the confusion is more about the current default value (white) or the default workflow ? Asking to better target the doc update.

yar2001 · July 30, 2024, 7:10am

The playground above has already set .roughness to 1, which means it uses Metallic/Roughness mode.

Even if PBRMetallicRoughnessMaterial is being used, the default metallic value is still 1.

github.com

BabylonJS/Babylon.js/blob/c331cf2714f60276791eb60a5ac225651daec3c7/packages/dev/core/src/Materials/PBR/pbrMetallicRoughnessMaterial.ts#L70


      
           * Instantiates a new PBRMetalRoughnessMaterial instance.

           *

           * @param name The material name

           * @param scene The scene the material will be use in.

           */

          constructor(name: string, scene?: Scene) {

              super(name, scene);

              this._useRoughnessFromMetallicTextureAlpha = false;

              this._useRoughnessFromMetallicTextureGreen = true;

              this._useMetallnessFromMetallicTextureBlue = true;

              this.metallic = 1.0;

              this.roughness = 1.0;

          }

          

          /**

           * @returns the current class name of the material.

           */

          public override getClassName(): string {

              return "PBRMetallicRoughnessMaterial";

          }

sebavan · July 30, 2024, 7:49am

Make sense, I ll update the doc.

mawa · July 30, 2024, 7:53am

I agree with @sebavan (and sure we can always improve the doc). In the end I think (my opinion) we are doing a lot of ‘fuss’ (sry) around something that need to be learned ‘per app’ or ‘per design’ no matter what. But then again, that’s just my opinion.

yar2001 · July 30, 2024, 9:09am

You are probably right. Maybe this is just a ‘fuss’. But it took me several days to tune the rendering just because I didn’t know the metallic value was 1. I feel like it could also happen to someone else. Are there any other default parameters in Babylon.js that differ from common expectations? Now I’m even less sure. The design principle seems to be based on historical reasons rather than user experience.

Of course, Babylon.js team definitely has more contextual information on this, so I’m just passing on what I perceive.

sebavan · July 30, 2024, 11:57am

This is a choice we made to allow setting textures without factors issues. It is also 1 to be consistent with the specular case or any case where this is used as a factor so it prevents to set a value when a texture is used. If we were setting 0 by default, you would probably have posted that you use a texture and do not understand the value is unused and you spent several days figuring it out

Also if you think about common expectations, GLTF is probably one of the few standard nowadays and it is 1 as well:

So, yes, this is an arbitrary choice aligned with an ISO standard simplifying the use of textures so as long as the doc explains it it should not be an issue. If you want to be sure the doc is the most helpful, feel free to do the PR there and I will gladly merge it.

roland · July 30, 2024, 12:02pm

I can’t agree more.

You have the answer:

and you have a solution to your problem:

and you’re still fussing around

Dude, it took years for hundreds of contributors to give you babylon.js, for free.

Peace!

mawa · July 30, 2024, 12:11pm

OMG. I can feel the heat May be you should cool down a little with a fresh beer and a break ?

Hum? Likely true and then, I wasn’t here at the beginning neither I don’t think this kind of comment is appropriate (sry). We should be happy that there are people wanting to help improve and normalize the framework to push it even further and to the next level, shouldn’t we? In my opinion, it’s just a matter of finding a balance between the pros and cons. Nothing easy, nothing easy for sure.

roland · July 30, 2024, 12:21pm

What heat? I’m OK I believe @yar2001 will not take it personally because I didn’t mean to harm anyone’s feelings. You know me, I’m the clown, the comedian, the jester, the punster, the meme maker around here

@yar2001 one more time - sry if you’ve had bad feelings after you saw my meme - but it’s damned funny and you’ll take it as an advice just to move on for now with that what we have available…

roland · July 30, 2024, 12:24pm

If it’s not neither his comment about spending days to find out something is appropriate. Trying to convince others of your truth by how much time you spent on it is not appropriate. That’s why I responded like that.

@yar2001 's other technical observations are ABSOLUTELY appropriate.

mawa · July 30, 2024, 12:35pm

Well, one would first have to define “common expectations”. Common in which sense? A 3D engine, a real time 3D engine? Unity, unreal, three.js (they are already all different)? Common expectation as you say, is not a norm or a convention.

Well, as far as I know, the design principle is historical in that it is based on the pillars of the BJS experience: Open, free, simple, reliable, powerful. One of these pillar being ‘reliable’ (read ‘backwards compatible’). So if your question is: “Are there choice of design made for ‘simplicity’ or to ensure the largest use case, per design?” The answer is “absolutely”.

If your question is: “Are there some disparities, approximations in the names or default values of parameters?” The answer is “I’m afraid there is”.

If your question is: “Is BJS going to change it all at once and while doing so, break its promess and temper with one or more of the pillars that’s part of its DNA?” The answer is: “I sure hope it won’t… I sure hope the Team won’t allow this to happen… and if so, you’re gonna hear my voice for sure ”.

Fair enough. That’s what we all do in the beginning. I totally remember doing this for a while. But in the end, the framework and the values aren’t just based on ‘perception’. They cannot be. I believe the most important is that it remains both ‘consistent’ and ‘true to its values’. Again, as always, my opinion only. And meanwhile, have yourself a great day

Topic		Replies	Views
Is my PBRMaterial broken or is dev dumb? Questions material , texture	6	346	March 2, 2023
Confusion with PBR + Bug with inspector? Questions material , pbr	26	3032	August 12, 2020
Albedo/lightmap Texture on PBR material + albedo color Questions pbr , texture	3	1191	October 27, 2022
Light updates not applied in a custom PBRMaterial? Questions	1	205	February 10, 2022
Setting material ambient color by vertices Questions	1	374	December 2, 2019

`ambientColor` is not worked in PBRMaterial

Related topics