Moved matrices to 3x4#386
Merged
Merged
Conversation
sebastienlagarde
approved these changes
May 18, 2020
stramit
approved these changes
May 25, 2020
pbbastian
approved these changes
May 28, 2020
pbbastian
suggested changes
May 28, 2020
Contributor
pbbastian
left a comment
pbbastian
left a comment
There was a problem hiding this comment.
misclicked "Approve" before :< but LGTM, just want some clarification on that version, and then maybe a define change depending on answer
pbbastian
approved these changes
May 28, 2020
cinight
approved these changes
Jun 4, 2020
Contributor
cinight
left a comment
cinight
left a comment
There was a problem hiding this comment.
Tested on Win DX11 & Mac Metal editor & player no visual regression.
CPU memory is improved.
*I've tested with Hybrid URP Template Project, it might be too small to see big FrameTime increase.
TomasKiniulis
approved these changes
Jun 5, 2020
Contributor
TomasKiniulis
left a comment
TomasKiniulis
left a comment
There was a problem hiding this comment.
No PR related visual issues found testing on Hybrid HDRP template on Dx11 and Metal editor and player.
I get significant memory usage decrease profiling player
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
Currently matrices are stored as 4x4 on the GPU. However we know that these are affine matrices so the last row is always (0, 0, 0, 1).
If we get rid of this row and make it explicit we will reduce matrix memory and bandwith requirements. It would also let the shader compiler constant fold better since last row will be explicitly known at compile time.
Testing status
Manual Tests
Tested with URP and HDRP hybrid test projects locally.
Automated Tests
No automated tests on Graphics side. But DOTS side PR contains edit mode tests to endure matrix uploads are correct.
Links
Yamato: (Select your branch) https://yamato.prd.cds.internal.unity3d.com/jobs/902-Graphics
Comments to reviewers
I will try to land this to both SRP and DOTS repos simultaneous. The DOTS repo will contain an if guard to ensure the code is only active there if the SRP version is something greater or equal to some preview version where we KNOW the SRP change will land.