-
Notifications
You must be signed in to change notification settings - Fork 23
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
Layer diagram improvements #189
Layer diagram improvements #189
Conversation
Suggest smooth approximation.
… into coat_roughening
# Conflicts: # index.html
images/layer_graph.png
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although this node diagram is now closer to the layering diagram (illustration in the PR description), it makes me realise we might have an inconsistency.
From this tree structure, I understand the dielectric specular lobe, represented by the "gloss" node, will only apply to the diffuse layer. If the translucent or subsurface weight is 1, both the diffuse lobe and its glossy lobe are nulled.
This seems in conflict with the descriptions of the dielectric layers, from which I understand all 3 dielectric cases share the same specular lobe.
Quoting the outline:
The interface (dielectric or metal) of this base layer produces the primary specular reflection lobe. The dielectric base represents either of three components, that can be statistically mixed:
I understand that to represent this we can choose from either:
one single gloss layer on top of all the dielectric components (translucent, subsurface and diffuse).a separate gloss layer on top of each of the dielectric components.omit the gloss layer from the diagram, and consider that each of the component produce their own specular lobe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re-reading the spec, I see the diffuse slab is specified differently from the translucent and subsurface one, where the diffuse slab only has a diffuse BRDF, whereas the other two have a dielectric specular BRDF and a volume.
index.html
Outdated
In the full light transport, the observed color of the coated base is darkened and saturated due to multiple internal reflections from the inside of the coat, which causes light to strike the underlying material multiple times and undergo more absorption, and the observed **`coat_color`** tint also darkens as the incidence angle changes due to the change in path length in the medium. Also, the presence of a rough coat will increase the apparent roughness of the BSDF lobes of the underlying base. We assume that in the ground truth appearance, all these physical effects are accounted for. In the following sub-sections, we detail recommendations for implementation of them. [^porosity] | ||
In the full light transport, the observed color of the coated base is darkened and saturated due to multiple internal reflections from the inside of the coat, which causes light to strike the underlying material multiple times and undergo more absorption, and the observed **`coat_color`** tint also darkens as the incidence angle changes due to the change in path length in the medium. Also, the presence of a rough coat will increase the apparent roughness of the BSDF lobes of the underlying base. We assume that in the ground truth appearance, all these physical effects are accounted for. In the following sub-sections, we detail recommendations for implementation of them. | ||
|
||
### Roughening |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you intend to include the coat roughening sub-section in this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's done in a separate PR, which is ready for merge:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see, the coat roughening change somehow got included in this PR as well.. That was a mistake. 😬
When we merge the roughening one, we can just sort it out in the merge back to this PR.. (Done in 1b69442)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
# Conflicts: # index.html # reference/open_pbr_surface.mtlx
# Conflicts: # reference/open_pbr_surface.mtlx
Merged main, ready to merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me, thanks @portsmouth.
de8fc5a
into
AcademySoftwareFoundation:main
Improved this diagram a bit: