{{ message }}
Moving WS view direction from vertex to fragment to lower varying size#5509
Merged
phi-lira merged 7 commits intoNov 10, 2021
Merged
Conversation
Contributor
Author
cdxntchou
approved these changes
Sep 22, 2021
cdxntchou
left a comment
cdxntchou
left a comment
There was a problem hiding this comment.
Looks good!
HDRP seems to already use this approach, so only URP has the changes...
simon-engelbrecht-soerensen
approved these changes
Oct 5, 2021
simon-engelbrecht-soerensen
left a comment
simon-engelbrecht-soerensen
left a comment
There was a problem hiding this comment.
Did a comparison test and it seems to work. 👍
GrantLamb-Unity
left a comment
GrantLamb-Unity
left a comment
Contributor
There was a problem hiding this comment.
@simon-engelbrecht-soerensen 's comparison tests should be enough for this change 👍
phi-lira
approved these changes
Nov 10, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
Purpose of this PR
These optimizations were discussed with Apple.
mobile optimizations to lower bandwidth:
completely removed viewDirectionWS variable declaration from StructField.Varyings struct and also the usage of the variable from the shader template as well.
For WorldSpaceViewDirection that is used in the ShaderGraph node, replaced the calculation from:
normalize(input.viewDirectionWS)
to:
normalize(GetWorldSpaceViewDir(input.positionWS)
Also changed the field dependency of the WorldSpaceViewDirection node to now requiring StructFields.Varyings.positionWS variable.
Testing status
local comparison test for the View Direction Node in editor and on an iOS device
all URP/all Shadergraph on Yamato
Comments to reviewers