[ShaderGraph][2022.1] Fix direction transform from view to world space #5614
Conversation
|
Hi! This comment will help you figure out which jobs to run before merging your PR. The suggestions are dynamic based on what files you have changed. Shader Graph Depending on the scope of your PR, you may need to run more jobs than what has been suggested. Please speak to your lead or a Graphics SDET (#devs-graphics-automation) if you are unsure. |
|
Backports on this one? @marctem |
jessebarker
changed the title
| @@ -189,7 +189,7 @@ public void GenerateNodeCode(ShaderStringBuilder sb, GenerationMode generationMo | |||
| { | |||
| if (conversion.to == CoordinateSpace.World) | |||
| { | |||
| transformString = string.Format("mul(UNITY_MATRIX_I_V, $precision4({0}, 1)).xyz", inputValue); | |||
There was a problem hiding this comment.
Won't this make TransformViewToWorld wrong for position? I believe this needs a conditional like the otehr cases (see ViewToObject below).
There was a problem hiding this comment.
Good catch. Fixed. I think this also means the test images don't need to be updated, but I'm re-running to confirm.
There was a problem hiding this comment.
Actually, if the images don't need to be updated, this implies we don't have a test for both scenarios. Maybe adding new node tests might be good.
Purpose of this PR
This PR fixes https://fogbugz.unity3d.com/f/cases/1362034/
Transform direction from view to world was effectively doing a position transform rather than direction. This change fixes that.
Testing status
Manual testing according to the repro case in the bug (confirmed that the transform from world->view and then back from view->world yields vec3(0, 0, 0) when subtracted from the original vec3.
Yamato details below.
Here are the built-in test results that got "erased" when the ShaderGraph test project reference images got updated (reference images from TestProjects/ShaderGraph have no impact on the two built-in target test projects):
Comments to reviewers
Notes for the reviewers you have assigned.