-
Notifications
You must be signed in to change notification settings - Fork 861
[ShaderGraph] [master] [bugfix 1296788] Bugfix for view direction node in tangent space being not using the c… #3044
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
Conversation
ghost
left a comment
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.
TransformWorldToTangent contains a normalization step, whereas the previous algorithm doesn't normalize. Can you remove the normalization step and see if the algorithm still works? This will allow us to backport it to versions where we don't plan on normalizing the view direction coming in from URP
ghost
left a comment
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.
Testing has confirmed that in some cases (speedtrees for example) tangent space is not orthogonal at the vertices, so the vertex templates need to be updates as well
Updated |
ghost
left a comment
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.
com.unity.shadergraph/CHANGELOG.md
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.
"Tanget" should be "Tangent"
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.
Nice catch. Fixed
0717705 to
2252a58
Compare
ghost
left a comment
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.
Typo fixed, reapproving
f055f81 to
edadb79
Compare
…orrect calculation.
|
Failures on HDRP match current state of master |
edadb79 to
b2446b6
Compare



…orrect calculation.
Do not merge before #2752
Please read the Contributing guide before making a PR.
Checklist for PR maker
need-backport-*label. After you backport the PR, the label changes tobackported-*.CHANGELOG.mdfile.Purpose of this PR
The calculation for the ViewNode in Tangent space used an old method that assumed the tangent space basis was orthonormal. The Transform node was updated but the view node wasn't.
Testing status
Tested the attached shader graph in the bug: https://fogbugz.unity3d.com/f/cases/1296788/
Comments to reviewers
Notes for the reviewers you have assigned.