-
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
Merged
jstone-lucasfilm
merged 18 commits into
AcademySoftwareFoundation:main
from
portsmouth:layer_graph_improve
May 14, 2024
Merged
Changes from 15 commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
face56c
Add suggested physically-based approximate formula for coat roughening.
portsmouth b5e7af8
Merge branch 'main' into coat_roughening
jstone-lucasfilm 6c66e3b
Take into account TT mode in coat roughening formula.
portsmouth c5b4a1b
Merge branch 'main' into coat_roughening
portsmouth 3b0fd12
Merge branch 'main' into coat_roughening
portsmouth 1edaca3
Merge branch 'main' into coat_roughening
portsmouth 5569167
Merge branch 'main' into coat_roughening
portsmouth d7de8b4
Remove approx. roughening formula.
portsmouth 7529555
Merge remote-tracking branch 'upstream/main' into coat_roughening
portsmouth 5eb4dbb
Add MaterialX implementation of the more realistic coat roughening fo…
portsmouth 930ce21
Merge branch 'coat_roughening' of https://github.com/portsmouth/OpenP…
portsmouth 54b5ec8
Fix mix node
portsmouth 9c018c1
Merge remote-tracking branch 'upstream/main' into coat_roughening
portsmouth 5a12a38
Improve wording.
portsmouth d56c85b
Improvements to layer tree diagram
portsmouth 69e25b9
Merge remote-tracking branch 'upstream/main' into layer_graph_improve
portsmouth 21cf8ca
Merge remote-tracking branch 'upstream/main' into layer_graph_improve
portsmouth 1b69442
Remove duplicated section
portsmouth File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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: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.