Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Adding (refactoring to) Gbuffer optional slot 3 #4528

Merged
merged 96 commits into from
May 20, 2021
Merged

Adding (refactoring to) Gbuffer optional slot 3 #4528

merged 96 commits into from
May 20, 2021

Conversation

jonuuukas
Copy link
Contributor

Purpose of this PR

During the RenderPass Input attachment PR, I was asked to refactor GBuffer4 defines to use the existing OPTIONAL_SLOT define logic. This PR introduces those changes as we agreed to add it in a follow up PR.

Testing status

Manually tested on Editor, iOS and Android (Deferred renderer only, as forward is unaffected)

Comments to reviewers

Notes for the reviewers you have assigned.

manuele-bonanno and others added 30 commits February 26, 2021 11:10
@manuele-bonanno
enabled MSAA when the RenderPass API is used. Resolving implicitly to…
ddbee5f 
… the color target's resolve surface
@manuele-bonanno
Initial lazy merging prototype
8f7db74
@manuele-bonanno
Lazy merging prototype: all the needed attachments are now added to t…
fe5cadb 
…he RenderPass attachment list. Merging subpasses and attachments which are the same or compatible
@manuele-bonanno
initial work to add lazy merging to the deferred renderer (doesn't ha…
e50c046 
…ndle coorrectly edge cases)
@manuele-bonanno
MRT attachments merging improvements
3ba7c7b
@manuele-bonanno
fix for a crash because of incomplete Targets setup for merged render…
b66f183 
… passes
@jonuuukas
change m_ShadowmapWidth/Height variables to use renderTargetWidth
422ed1b
@jonuuukas
Merge branch 'universal/render-pass-conversion' of ssh://github.com/U…
9bdd3ef 
…nity-Technologies/Graphics into universal/render-pass-conversion
@jonuuukas
m_ActiveRenderPassQueue[i] to var renderPass
7d3c5ba
@jonuuukas
changing hash ctor logic to merge width/height and include depth rtID
6f5d3f0
@jonuuukas
fix setlastpass flag loop
7dc34fa
@manuele-bonanno
NativeArray memory leak fix
72d51d2
@manuele-bonanno
refactoring to remove duplicated code
73d666d
@jonuuukas
fix non-renderpass passes getting hashes and etc.
d86b828
@manuele-bonanno
more cleaning up, moving RenderPass code in separate functions for cl…
ac46e52 
…arity
@jonuuukas
lists to arrays and other gc.alloc related stuff
bc45166
@jonuuukas
Merge branch 'universal/render-pass-conversion' of ssh://github.com/U…
7b828d8 
…nity-Technologies/Graphics into universal/render-pass-conversion
@jonuuukas
Merge branch 'master' into universal/render-pass-conversion
44d350e
@jonuuukas
merge leftovers
e57a3ed
@manuele-bonanno
moving all the RenderPass API specific code in separate methods as a …
432dc6b 
…preparation step to move into different files (potentially)
@jonuuukas
remove Configure in SetLastPassFlag
bf030cb
@manuele-bonanno
refatoring (in progress): moving all the RenderPass API specific code…
f7d061a 
… to NativeRenderPassUtils
@jonuuukas
fixing depth-only offscreen camera and exposing usesNativeRenderPass
1b0b6bf
@jonuuukas
Merge branch 'universal/render-pass-conversion' of ssh://github.com/U…
6a6dd1f 
…nity-Technologies/Graphics into universal/render-pass-conversion
@jonuuukas
merge leftovers
f8a8e62
@manuele-bonanno
more cleaning up and refactoring
d84f779
@manuele-bonanno
splitting initial scene index setup from RenderPass data, and moved R…
ef3e215 
…enderPass data to the NativeRenderPass class
@manuele-bonanno
Revert "splitting initial scene index setup from RenderPass data, and…
eb3514d 
… moved RenderPass data to the NativeRenderPass class"

This reverts commit ef3e215.
@manuele-bonanno
fix for RenderPass MRT attachments being setup when the RenderPass AP…
b89f1e8 
…I is disabled
@jonuuukas
adding profiler samples to NativeRenderPass code
ebd2d04
@jonuuukas
first pass with input attachments changing hard return to warning msg
e78b327
@jonuuukas
reverting some changes for rendererFeatures, as they have renderpass …
86fa4a6 
…disable already
@jonuuukas
some comments and other stuff regarding review feedback
4b745cb
@jonuuukas
some depthcopy logic skip for RenderPass Deferred
4be1266
@jonuuukas
Merge branch 'master' into universal/render-pass-input
344b9e5
@jonuuukas
typo fix
ad80f20
@jonuuukas
Merge branch 'universal/render-pass-input' of ssh://github.com/Unity-…
496dc12 
…Technologies/Graphics into universal/render-pass-input
@jonuuukas
LOAD_FRAMEBUFFER_INPUT using same parameter for single channel read
39a2606
@jonuuukas
removing clipPos from inputData init
d258506
@jonuuukas
optional slot 3 added to GBuffer
0342131
@jonuuukas
additionl _render_pass_enabled check to determine correct type for op…
20b5672 
…tional slot
@jonuuukas
fixing the #ifdef logic a bit for optional slot2
22f3c70
@jonuuukas
Merge branch 'master' into universal/gbuffer-optional-slot3
d8e4d18
@jonuuukas jonuuukas requested review from a team as code owners May 13, 2021 13:29
@github-actions
Copy link

It appears that you made a non-draft PR!
Please convert your PR to draft (button on the right side of the page)
and cancel any jobs that started on Yamato.
See the PR template for more information.
Thank you!

kaychang-unity
kaychang-unity approved these changes May 13, 2021
View reviewed changes
Copy link
Contributor

@kaychang-unity kaychang-unity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@jonuuukas jonuuukas requested a review from a team May 18, 2021 06:58
@ernestasKupciunas ernestasKupciunas requested review from ernestasKupciunas and removed request for a team May 18, 2021 07:24
ernestasKupciunas
ernestasKupciunas approved these changes May 18, 2021
View reviewed changes
Copy link

@ernestasKupciunas ernestasKupciunas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. No issues found related to this PR. Tested using URP Template project.

Devices under testing:
VLNQA00217, Razer Phone 2, 9.0.0, Snapdragon 845 SDM845, Adreno 630
VLNQA00268, Samsung Galaxy S10+, 10.0.0, Exynos 9 9820, Mali-G76
OnePlus Nord, 10, Snapdragon 765G SM7250-AB, Adreno 620
iPad Air4, SoC: A14, iOS: 14.0
iPhone 12Pro, SoC: A14, iOS: 14.1

Unity used for testing:
Version: 2021.2.0a18.2582
Revision: trunk 5605789000ce
Built: Sun, 16 May 2021 07:07:00 GMT

hdb-unity
hdb-unity approved these changes May 19, 2021
View reviewed changes
Copy link
Contributor

@hdb-unity hdb-unity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Result Approving without manual testing. Did not check the automated test results (I expect they will be green to merge).

Why? Discussed this has isolated halo effect and is already manually tested.

@phi-lira
Copy link
Contributor

Let me know when it finishes, I'll look at failures.

@phi-lira phi-lira merged commit c619870 into master May 20, 2021
@phi-lira phi-lira deleted the universal/gbuffer-optional-slot3 branch May 20, 2021 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@phi-lira phi-lira phi-lira approved these changes

@ernestasKupciunas ernestasKupciunas ernestasKupciunas approved these changes

@kaychang-unity kaychang-unity kaychang-unity approved these changes

@hdb-unity hdb-unity hdb-unity approved these changes

Assignees
No one assigned
Labels
universal
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@jonuuukas @phi-lira @ernestasKupciunas @kaychang-unity @hdb-unity @manuele-bonanno