Fixed ambient probe convolution warning #6523
Conversation
… into hd/rg-skymanager
…U to avoid latency.
…ut volumetric buffer.
…d of a constant buffer generated through CPU readback
… into hd/gpu-ambient-probe
… into hd/gpu-ambient-probe
… into hd/gpu-ambient-probe # Conflicts: # com.unity.render-pipelines.high-definition/Runtime/Sky/SkyManager.cs
…ologies/Graphics into hd/rg-skymanager # Conflicts: # com.unity.render-pipelines.high-definition/Runtime/RenderPipeline/HDRenderPipeline.RenderGraph.cs # com.unity.render-pipelines.high-definition/Runtime/Sky/SkyManager.cs
… into hd/gpu-ambient-probe # Conflicts: # com.unity.render-pipelines.high-definition/CHANGELOG.md
…ologies/Graphics into hd/rg-skymanager
… into hd/rg-skymanager # Conflicts: # com.unity.render-pipelines.high-definition/Runtime/Sky/AmbientProbeConvolution.compute # com.unity.render-pipelines.high-definition/Runtime/Sky/SkyManager.cs
|
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. HDRP 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. |
|
It appears that you made a non-draft PR! |
… into hd/fix-convolution-warning # Conflicts: # com.unity.render-pipelines.high-definition/Runtime/Sky/SkyManager.cs
JulienIgnace-Unity
requested review from
sebastienlagarde and
FrancescoC-unity
| { | ||
| int c = 0; | ||
| for (c = 0; c < 3; c++) | ||
|
|
| // If we use local VGPRs as a scratch buffer we end up using too many register | ||
| // To avoid that we go through memory. | ||
| // This is quite messy and bad for performance but this shader should never be critical so it should be fine. | ||
| // Uint is used as it's the only format supported everywhere as read/write from same thread. |
There was a problem hiding this comment.
*on all platforms
Many platform do support typed loads on other formats
… into hd/fix-convolution-warning
… into hd/fix-convolution-warning
Purpose of this PR
Fixed ambient probe convolution warning
Unfortunately, this shader uses a lot of temp VGPR and we could not rewrite it to use less so we ended up using a temporary compute buffer in memory. This should not be a problem for performance as this shader is already super fast and not executed often generally.
Fixes: https://fogbugz.unity3d.com/f/cases/1388268/
Testing status
Ran graphics tests
Comments to reviewers
Need to merge #6035 first