AcademySoftwareFoundation / OpenPBR

Specification and reference implementation for the OpenPBR Surface shading model
Apache License 2.0
407 stars 18 forks source link

Specify the anisotropy direction #170

Closed virtualzavie closed 2 months ago

virtualzavie commented 4 months ago

This PR removes entirely the definitions specular_rotation and coat_rotation, to consider instead that the provided geometry_tangent is sufficient to define the direction of the anisotropy.

To avoid correlation between the NDF of the base layer and coat layer, geometry_coat_tangent is added.

Finally the Normal maps section is developed to mention how a tangent map can alter the tangent vector in the same way a normal map alters the normal vector.

image image image image image image image image
linux-foundation-easycla[bot] commented 4 months ago

CLA Signed

The committers listed above are authorized under a signed CLA.

virtualzavie commented 4 months ago

I've added several changes, as proposed in #155:

Naming (d7d7040):

image image image image

Specification of the direction as a property of the NDF (e321ff9):

image image image image image
portsmouth commented 4 months ago

I feel that we should keep the existing specular_rotation and coat_rotation parameters, applied as an additional angular offset on top of the flow-map generated T, B. This should normally only be used to globally rotate the highlights (but in some cases could be textured, with the caveat about the interpolation artifacts due to the discontinuity which we should warn about in the spec). This ability to globally rotate seems useful to retain (e.g. to switch the interpretation of the painted T from highlight-stretch to groove-stretch direction, by setting the rotation to constant 90 degrees, i.e. 0.25).

Also, there will be existing assets that texture the rotation. A case where it could genuinely make sense to texture the rotation would be to add some perturbing angular noise to procedurally break up the flow-map generated directions (which will not introduce any artifact provided the noise is continuous on the surface). I imagine doing this via the shader, rather than having to regenerate the flow-map, could be quite convenient.

glTF does this also, though it explicitly requires that the rotation parameter be untextured (which I think is unnecessary for us).

anisotropyRotation | number | The rotation of the anisotropy in tangent, bitangent space, measured in radians counter-clockwise from the tangent. When anisotropyTexture is present, anisotropyRotation provides additional rotation to the vectors in the texture

portsmouth commented 4 months ago

I like the new name specular_roughness_anisotropy, that clarifies the meaning as relating to the roughness.

Note that currently, the specular_roughness_direction specifies both of (T, B), where T is the surface tangent direction along which the NDF is stretched (and therefore also the highlight). So B is the direction corresponding to the "grooves/scratches". Arguably, B is what an artist would want to think of as the "roughness direction", but anyway the flow map provides both T and B. (We could have called it e.g. specular_flow_map, but I think its more intuitive to think of it as specifying the T (or B) direction on the surface).

What we specify is at least consistent with glTF, where they said:

Note: The direction vector of the anisotropy is the direction in which highlights will be stretched. The direction of the micro-grooves in the material causing the anisotropy will run perpendicular.

(where "direction vector" means T here).

portsmouth commented 4 months ago

Regarding the discussion in the meeting about what assumptions we should make about the tangent space, I assume the policy is roughly:

image

virtualzavie commented 4 months ago

Yes, good point. We already state the assumption of the existing frame of reference, but the exact transform is certainly lacking.

portsmouth commented 4 months ago

The direction is specified as a normalized 2D vector, with $(1, 0)$ being the original tangent vector direction.

I think we need to be more explicit that the values in the texture, $(R, G)$ (where $R, G \in [0,1]$ after dequantization), are mapped to $(t, b)$ via:

$$ t = 2R-1 $$

$$ b = 2G - 1 $$

where we require that $t^2 + b^2 =1$. Then the final tangent frame is given (in terms of the reference tangent frame) by:

$$ \mathbf{T}' = t \mathbf{T} + b \mathbf{B} $$

$$ \mathbf{B}' = t \mathbf{B} - b \mathbf{T} $$

Or as a 2x2 matrix equation (with a rotation matrix appearing, with $t=\cos(\theta)$, $b=\sin(\theta)$, with $\theta$ the angle of the line in the flow-map to the reference $\mathbf{T}$):

$$ \left( \begin{array}{c} \mathbf{T}' \ \mathbf{B}' \end{array} \right) = \left( \begin{array}{cc} t & b \ -b & t \ \end{array} \right) \left( \begin{array}{c} \mathbf{T} \ \mathbf{B} \end{array} \right) $$

Obviously, that guarantees that $(\mathbf{T}', \mathbf{B}')$ are orthonormal (if $\mathbf{T}$ and $\mathbf{B}$ are).

Hopefully this corresponds to the format of "flow maps" generated by some existing tools (though I don't actually know).

virtualzavie commented 4 months ago

There were several points brought, so I'll try to address them one by one:

Did I forget anything?

portsmouth commented 4 months ago

On the additional rotation parameter: I am not convinced this value is worth the cost. This reintroduces the implementation complexity that the flowmap was meant to avoid. It also adds a parameter, which is something we should try to limit, for a control that's essentially redundant with the anisotropy direction. I hear the use case, but that sounds like a proposal to work around a potential limitation of the authoring tool. I would instead suggest to leave this proposed parameter out at least until we have a clear demand for it.

Sounds reasonable. At least, putting the rotation angle parameter back if users demand it would be strictly additive to the model so would not be a very disruptive change. The main thing I worry is that users may want to globally rotate the direction by 90 degrees, in case the flow direction was painted as "groove stretch" instead of "highlight stretch". Though of course it can always be done at the level of the flow map.

On the specification of the tangent space: agreed, will amend the PR to specify the relation between $T$ and $T'$

👍

I agree it makes more sense for the artist to draw the direction of the groves, but as you point out, the vector field will express both equivalently. So in the end I think we can conclude it's irrelevant, which means the decision to how best express it will be left to the authoring tool. On the specification side, we then should probably choose the simplest and clearest specification. I think "NDF stretching along T" would be it (even though it means the opposite of what I originally thought it meant 🙃).

Possibly i'm mistaken, but won't artists usually think of the lines (i.e. vector field) they are painting as corresponding to the "brush" direction in brushed metal? So e.g. to get the render below, wouldn't they want to paint circles around the base of the pan, rather than radial lines? (As if they are applying the brushing themselves to the metal). In that case, they would be specifying what we currently refer to as B. (Or in other words, to get that render with your current proposal, they would need to have painted radial lines, which seems a bit weird).

image

Personally I'd vote to swap the meaning of T and B, so that T actually means the groove direction (so B is the NDF highlight stretch direction). This is not what glTF did though, for whatever reason. Ideally we need to hear from more people who are familiar with the actual artist tools and workflows for this (maybe your Substance colleagues for example).

virtualzavie commented 4 months ago

Possibly i'm mistaken, but won't artists usually think of the lines they are painting as corresponding to the "brush" direction in brushed metal?

Oh yes, I completely agree. But whether we specify the grooves as being T or B won't matter, since it's up to the authoring tool to decide what a brush stroke of the artist will result in. Arguably, an authoring tool that doesn't give much thought into it will likely map our parameter as is, so that would be an argument to indeed align the grooves with T.

I'll try to gather some feedback internally.

portsmouth commented 4 months ago

NB, the Arnold tutorial for that pan render is here:

https://help.autodesk.com/view/ARNOL/ENU/?guid=arnold_for_3ds_max_ax_shading_ax_anisotropic_brushed_html

It's interesting that in this case, he didn't rely much (or at all?) on the tangents, instead the circles were generated using a bump map with circular "scratches" in it (which is kind of like authoring the "anisotropic microfacet distribution" with bumps, rather than the actual NDF). Which is another valid way to get "anisotropic roughness" at a geometrical level, I suppose, and looks more realistic since you can actually see the grooves/scratches.

Similarly, in this photograph the anisotropy is generated by high resolution scratches which are somewhat random but statistically biased to point along a "flow" direction. I can imagine that a high end asset may prefer to author anisotropy this way, rather than with the more crude stretching of the NDF.

image

portsmouth commented 3 months ago

On the additional rotation parameter: I am not convinced this value is worth the cost... suggest to leave this proposed parameter out at least until we have a clear demand for it.

Agreed.

On the specification of the tangent space: agreed, will amend the PR to specify the relation between $T$ and $T'$

👍

On the direction of the grooves vs direction of the highlight: your comment makes me realise I confused what the effect of stretching the NDF would be. I will have to amend the PR to fix that.

👍

On the specification side, we then should probably choose the simplest and clearest specification. I think "NDF stretching along T" would be it (even though it means the opposite of what I originally thought it meant 🙃).

Arguably, an authoring tool that doesn't give much thought into it will likely map our parameter as is, so that would be an argument to indeed align the grooves with T.

I think glTF opted to have the NDF stretch along T.

Personally, I think it's more intuitive though if the grooves aligned with T, so the highlight (and thus the NDF stretch) should align with B.

Though I suppose we should go with whatever convention is most consistent with existing flow-map tools (your Substance colleagues would be the best people to ask, and glTF people as to why they chose this convention).

On the mapping from [0,1] to [-1,1] (https://github.com/AcademySoftwareFoundation/OpenPBR/issues/155#issuecomment-2009614171) ... I was not sure there was a consensus, but I'll update the PR to reflect this.

Seems like there is a consensus (i.e. do same as in glTF).

Did I forget anything?

Nope. Only issue to decide is the stretch direction convention. Otherwise, proposed fixes to the spec sound good.

virtualzavie commented 3 months ago

I went with what I think is the simplest proposal.

Here are the relevant parts:

image image image image image image image image