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

Suggest separate parameters for diffuse and metal colours instead of "base_color" #140

Open
andre-ilm opened this issue Nov 11, 2023 · 13 comments

Comments

@andre-ilm
Copy link

In the OpenPBR specification, the diffuse albedo and the metallic F0 both share the base_color parameter, with base_metalness providing the control to mix between the dielectric-base and metal substrates.

We have found it to be useful to have these parameters specified independently as we typically paint these maps separately. Having them shared would make it difficult for us to use OpenPBR.

When Brent Burley chose this parameterisation, his aims were to make a simpler interface for BSDFs, primarily targeted at feature animation pipelines. We have found that for photo-realistic look development, having control of the diffuse separately from the metallic specular gives more artistic control.

Having these colours linked was one of the main barriers that prevented us from adopting Autodesk Standard Surface internally, as well.

Would it be possible to break base_color into metal_color and diffuse_color?

@portsmouth
Copy link
Contributor

portsmouth commented Nov 14, 2023

It must be true that the existing approach with a single base_color is less expressive than having separate colors, since obviously both the metal and diffuse base albedo must be scalar multiplies of this single color. If this impacts the ability to use the model for certain assets (so studios will reject the model specifically due to this problem), that seems really serious to me.

So in principle I'm in favor of changing this. Just I wish we had thought about this earlier as it's quite late to make such a change. I'm wondering what the minimal change we can make to the parameter set is to achieve it.

Possibly keeping base_color to mean non-metal base color, and adding a new parameter base_metal_color? If base_metalness is zero, in the UI this could be greyed out. At least then we are only adding 1 new parameter, with obvious meaning, that is consistent with the existing prefixes.

It seems not ideal to force users to dial in two colors if they want to work with an intermediate metalness. So another suggestion is to have base_metal_color default to black, then make the assumption that in this case (non-textured black) the metal F0 is taken from base_color. It is only if base_metal_color is textured, or constant and made non-zero in some channel, that we use it for metal F0. This would mean the existing OpenPBR behavior would be retained, and the custom metal color is "opt-in".

This would be a bit of a special case in OpenPBR though, since we don't currently have the notion of custom behavior depending on whether or not a parameter is textured. (Also possibly confusing when base_metal_color is black in the UI, but the metal is actually colored according to base_color. And you can't do a blend of diffuse and metal with constant black metal F0, but probably no-one ever wants that).

@portsmouth
Copy link
Contributor

I noticed this slide in the Unreal Substrate presentation. Is this "metalness limitation" the same issue?

image

@portsmouth
Copy link
Contributor

portsmouth commented Nov 21, 2023

To test the effect of constraining the metal and diffuse base colors to be equal, I did renders with OpenPBR of "rust on top of shiny metal" in two ways:

  • by making the base_color a blend of blue/rust weighted by base_metalness
  • by altering the shader so that the metal F0 color (blue) is independent from the diffuse color (rust)

In both cases the specular_weight is driven by the base_metalness, so that the rust has a diffuse look.

The resulting renders are below. It seems the blending does have a fairly obvious effect in this case, i.e. it creates a halo around the rust, instead of a crisp transition:

Base blend Separate diffuse/metal-F0

The base_metalness map used is close to a binary map, so the intermediate metalnesses (where the halo occurs) are presumably only due to texture filtering (in this case, though a map with larger areas of intermediate metalness is allowed of course in theory).

It seems there isn't any way to effectively get the version of the render without the halo currently in OpenPBR. This could be an issue in practical cases, since this use case of rendering a blend of diffuse on metal must be quite common (for e.g. rusty/painted metal) and the halo may not be wanted.

That said, there are other cases where such a halo occurs due to texture filtering (e.g. sharp transitions in roughness), so it could be considered to be a limitation fixed by higher texture resolution.

If we want to fix this, we need to be careful since we don't want to over-complicate the shader and proliferate parameters just for the sake of total control. We certainly don't want to disturb the very common "metalness" PBR workflow, where the base color is explicitly designed to contain both the dielectric and diffuse colors. (Which is obviously standard in various asset libraries etc.).

A possible approach then would be to add a specific override section for separate control over the metal F0, which would be opt-in (i.e. disabled by default). This could amount to a new metal_override checkbox (defaulting to off) and metal_color parameter (the particular names can be debated obviously). They could be seen as more advanced parameters for cases needing specific control over the metal F0 independent of the dielectric base. This would be minimally disruptive since it is strictly additive to the current model, and makes no difference by default. Though as noted, this does add some complexity to the parameter set, so we need to discuss it and agree it makes sense before moving forward.

We should also think about whether there are other such limitations introduced by the parametrization, that could similarly cause problems. For example to get the renders above, I had to make specular_weight equal to the base_metalness, as we want it to be 1.0 for metal, but 0.0 for dielectric, so the "rust" looks diffuse not shiny. In the transition region though, we will unavoidably get a blend of shiny metal and "shiny rust". Also, if there is metal edge-tint via specular_color (which models a physical effect in real metals), this will unavoidably produce tinted specular rust in the transition which probably isn't wanted. Fixing this could involve e.g., if metal_override is on having the specular_weight not apply to the metal and applying a specific metal_edge_tint parameter.

In theory all of the "specular" parameters could be duplicated into metal-specific optional overrides (though do we want to add all these, even if they are disabled by default?).

@virtualzavie
Copy link
Contributor

I was aware of this limitation of the metal-roughness parametrisation, but I thought this was a generally accepted caveat.
If this happens to be a barrier to adoption, we should indeed address it, but remain very careful of not introducing another barrier (additional complexity and/or rendering cost) to other users in doing so.

I agree with your observations, and a strictly override type of control could be a solution indeed. That being said, it's not obvious how to define such a solution. In comparison, the F-82 tint model for example has the benefit of behaving like a Schlick model with a default value that makes sense with regard to that behaviour.

@portsmouth
Copy link
Contributor

@andre-ilm

For an extension to OpenPBR to allow for the use cases you describe, a minimal suggestion is to add parameters:

metal_override      // opt-in, false by default
metal_weight        // metal F0 multiplier
metal_color         // metal F0 color
metal_edge_tint     // metal F82 tint

Where metal_override is false by default, in which case all the metal_ params are ignored, and the metal is driven by F0=base_weight * base_color and F82-tint=specular_color as usual. (base_metalness drives the mix weight of the metal in all cases).

Would that be sufficient for the model to be usable in your studio, in your view?

It could be argued that if we do that, we might as well add these as well, so the metal roughness/NDF is independent from dielectric also:

metal_roughness
metal_anisotropy
metal_rotation

So basically completely decouple the metal properties from the dielectric base (only if opted-in).

This would be strictly additive to the current model, i.e. it behaves exactly the same as now unless metal_override is enabled. It would be artistically intuitive I think as it just makes the control over the metal more explicit and lifts a constraint. (Indeed some artists may prefer to always work with the metal override on?).

It's more parameters though, and not the default, so that adds some mental friction (so e.g. the metal override should probably not be in your face, e.g. be in a collapsed rollout).

What it does is pretty trivial to implement, though it does add parameters and checks which has some perf/memory cost (perhaps fairly negligible if opted-out though?).

Or is there some other scheme you can think of that would achieve what you need?

@portsmouth
Copy link
Contributor

Same experiment with a metalness map that has a more gradual transition. In this case, the result with separate metal and base looks like the rust "thinning" in the transition region (i.e. partial coverage of the rust), while the blend alters the brightness in the transition which looks considerably less realistic (as a rusty metal).

Base blend Separate diffuse/metal-F0

@portsmouth
Copy link
Contributor

By the way, I think metal_weight would need to be an overall multiplier of the metallic Fresnel, not just the F0. As otherwise there is no way to completely turn off the metal reflection (as making both the F0 metal_color and the F82-tint metal_edge_tint black does not turn off the grazing Fresnel). We will say the same about specular_weight.

@peterkutz
Copy link
Contributor

@portsmouth All of your reasoning makes sense. Thanks for thinking about this in depth.

I have a few questions about the details of your proposals.

It could be argued that if we do that, we might as well add these as well, so the metal roughness/NDF is independent from dielectric also:

Is there evidence that this would also be useful to eliminate artifacts? If not, it would be nice to avoid adding these parameters because they complicate the implementation. Specifically, when all specular lobes share an NDF, they can be implemented as a single microfacet surface with a reflection/transmission coefficient that takes both the dielectric and metal properties into account (which is done in the current Adobe implementation for example).

metal_weight

What would be the purpose of adding a metal_weight parameter? Could base_metalness be used for the same purpose? If metal_weight is a more consistent name, could we simply rename base_metalness to metal_weight? That new name might suggest that metal is not part of the base but instead a separate component, however that actually seems more consistent with transmission and transmission_weight (which also share the same NDF).

Considering all of this, my tentative modified proposal would be this:

  • rename base_metalness to metal_weight
  • add metal_override (bool, false by default)
  • add metal_override_color
  • add metal_override_edge_tint

@fpsunflower
Copy link

+1 to keeping a single NDF for now

I like the metal_weight idea too, it does seem more consistent with transmission.

@portsmouth
Copy link
Contributor

portsmouth commented Apr 2, 2024

Is there evidence that this would also be useful to eliminate artifacts? If not, it would be nice to avoid adding these parameters because they complicate the implementation. Specifically, when all specular lobes share an NDF, they can be implemented as a single microfacet surface with a reflection/transmission coefficient that takes both the dielectric and metal properties into account (which is done in the current Adobe implementation for example).

+1 to keeping a single NDF for now

For example, the case of rust on shiny metal, where the rust is represented as a "glossy-diffuse" dielectric with a rough spec.

In the transition region between the metal and rust, the roughness has to ramp from some low value to high value. So in the middle of the transition band, with a single NDF we get a 50-50 blend of medium roughness metal and dielectric (i.e. the metal and dielectric are constrained to have the same roughness). That will look different to the result of a more general 50-50 blend of the separate BRDFs of the totally smooth metal and the rough dielectric, e.g. there will be no sharp highlight with the tied roughness. (Also, this blended roughness has to be manually authored in image editor or shader graph, rather than just authoring the low roughness metal and high roughness rust, and using the metalness map to implement the blend for you).

I don't have a render (we should do some), but I expect it looks similar to the halo artifact due to tying the base colors, though possibly a bit more subtle since it will only affect the spec. If it's subtle enough, we might consider it reasonable to avoid the extra parameters. My thought was just that if we're decoupling the metal, we might as well decouple all its properties to fully avoid these artifacts.

If you're implementing the base as a single microfacet model sharing an NDF, I think it would be valid as an approximation to just blend the metal and dielectric roughnesses internally (if metal_override is on, that is). At least then you're making a renderer-specific choice to ignore this roughness blending artifact (i.e. tying NDF of metal and dielectric), rather than us requiring all renderers to make that decision.

@portsmouth
Copy link
Contributor

portsmouth commented Apr 2, 2024

What would be the purpose of adding a metal_weight parameter? Could base_metalness be used for the same purpose? If metal_weight is a more consistent name, could we simply rename base_metalness to metal_weight?

Technically the metal_weight is needed to be able to kill the metal lobe completely to zero (as mentioned in a comment above). As without it, setting both metal_color and metal_edge_tint to zero doesn't kill the grazing highlight (i.e. Fresnel still goes to 1 at the edge). It would be the analogue/counterpart of specular_weight in the discussion in the current PR #157 here.

image

Also I think it's clearer to retain the top-level "base" section, which is the default convenient way to control the high-level blend of metal and dielectric in one place, i.e.:

  • base_weight: overall kill factor of base metal/dielectric for non-grazing/non-spec
  • base_color: color of base metal/dielectric for non-grazing/non-spec
  • base_roughness: overall roughness of base dielectric for non-grazing/non-spec
  • base_metalness: transition between the very different Fresnels of metal/dielectric

I would not really want to mess with that basic, well-known parametrization. (Though admittedly, I do find it confusing that base_roughness does nothing to a metal).

I prefer to reserve the metal_ parameters only for the more advanced, non-default workflow where the rule is simply that the metal BRDF completely ignores the specular_ params in favour of the corresponding metal_ ones.

Then the change is strictly additive to the current model, and could reasonably be hidden away in an "advanced" rollout or tab. It could also reasonably just be a no-op or omitted in an implementation, which will fall back to the current behaviour (i.e. metal_override is just always false).

That new name might suggest that metal is not part of the base but instead a separate component, however that actually seems more consistent with transmission and transmission_weight (which also share the same NDF).

I like the metal_weight idea too, it does seem more consistent with transmission.

Logically, the structure is that "base" is a blend of metal and dielectric, where the blend weight is base_metalness. Then the "transmission" lobe refers to the (transmission..) properties of the base dielectric only. As in the following hierarchy:

image

@peterkutz
Copy link
Contributor

Technically the metal_weight is needed to be able to kill the metal lobe completely to zero (as mentioned in a #140 (comment)). As without it, setting both metal_color and metal_edge_tint to zero doesn't kill the grazing highlight (i.e. Fresnel still goes to 1 at the edge). It would be the analogue/counterpart of specular_weight...

Makes sense. However it seems like base_metalness and metal_weight are redundant and that metal_weight could be used for both purposes.

For now, should we at least consider renaming base_metalness to metal_weight and considering the Metal component to be separate from the Base component?

That would leave the door open for adding more metal_ parameters in the future without changing the current behavior. It could also reduce the confusion about the base_roughness not affecting metals.

@portsmouth
Copy link
Contributor

portsmouth commented Apr 2, 2024

However it seems like base_metalness and metal_weight are redundant and that metal_weight could be used for both purposes.

WIth the proposed override parametrization, you need both the metal mix weight (called base_metalness currently), and an overall weight for the metal lobe to be able to kill the lobe (what I called metal_weight).

So I don't see how we could rename base_metalness to metal_weight, unless we rename metal_weight to something else (metal_tint perhaps?).

I see the point on the naming convention, but it does roughly follow the idea of the material structure (as in the diagram above) composed of:

  • a base "substrate", composed of a mixture of metal and dielectric. The base_metalness is the mix weight which defines the area fraction of metallic base and dielectric base.

  • the dielectric base is further composed of a "translucent base", and an "opaque base". The transmission_weight is the mix weight which defines the area fraction of translucent base versus opaque base.

  • opaque base is a mix of subsurface, and diffuse (both embedded/bounded by the dielectric base).

I think in that picture, base_metalness makes sense as a name. base_color * base_weight controls both the metal F0 and the diffuse albedo, being the color one sees if the medium is opaque. This color/metalness parametrization was designed (I assume) to correspond to the standard metalness/metallic workflow.

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

No branches or pull requests

5 participants