Fix for volumetric clouds performing Read and Write on same texture [Unsupported on some platforms] #4356

FrancescoC-unity · 2021-04-28T10:39:59Z

Fix for https://fogbugz.unity3d.com/f/cases/1316069/

As the title mentioned, clouds where not supported on some platforms (Metal and I assume Intel GPUs) as they were performing Read/Write on the same resource which format is 11-11-10.

This change will perform an extra copy of the color buffer to use in those cases and sample that instead.

I did try the following alternatives:

Write full res clouds in the upsample without combining and then blending separately. [Slower]
Try to rewrite the shader from compute to pixel [Stop soon enough given that there are a lot of samples so, LDS will be a big benefit]

This option turned out to be the cheapest. It adds on PS4-base [where it won't run anyway, just used to profile] around 0.15ms of cost.

What did I test forced the option on and off on both PC (NVIDIA card) and PS4 and check that same result is there.

NOTE FOR QA: I don't have a dev-ready Metal machine to test, so we need to test on metal.

github-actions · 2021-04-28T10:40:26Z

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!

iM0ve

TL:DR
Its now working but there are still some issues. Not sure if I should approve and we address them later or we want to fix them within this PR

Whats tested:

Checked that every Volumetric Clouds parameter works on Windows
Checked that every Volumetric Clouds parameter works on Mac
Built player on mac to see that the clouds are present

Concerns:

Only on Scene view, only on mac. There is this weird ring under the clouds
https://user-images.githubusercontent.com/36502659/116417194-b6ea0f80-a843-11eb-8621-88461618fd08.mov
On my mac the framerate is really slow on a build. Its way faster in the editor
Build
https://user-images.githubusercontent.com/36502659/116417697-32e45780-a844-11eb-94cb-f0f0a6e2b37a.mov

Editor

Screen.Recording.2021-04-28.at.17.08.53.mov

FrancescoC-unity · 2021-04-28T14:14:00Z

TL:DR
Its now working but there are still some issues. Not sure if I should approve and we address them later or we want to fix them within this PR

Whats tested:

Checked that every Volumetric Clouds parameter works on Windows

Checked that every Volumetric Clouds parameter works on Mac

Built player on mac to see that the clouds are present

Concerns:

Only on Scene view, only on mac. There is this weird ring under the clouds
https://user-images.githubusercontent.com/36502659/116417194-b6ea0f80-a843-11eb-8621-88461618fd08.mov

On my mac the framerate is really slow on a build. Its way faster in the editor
Build
https://user-images.githubusercontent.com/36502659/116417697-32e45780-a844-11eb-94cb-f0f0a6e2b37a.mov

Editor

Screen.Recording.2021-04-28.at.17.08.53.mov

I'd say those are likely different issues I'd open new tickets for those.

johnpars · 2021-04-28T17:48:56Z

Hi! I just realized that this PR potentially fixes the same issue as this one: #4345 (however with the focus on MSAA).
The solution in this PR is better (I perform hardware blending from an intermediate target into the MSAA target rather than a copy), and maybe we can expand it to handle the MSAA case and I can close the other one?

johnpars · 2021-04-28T18:01:18Z

...nes.high-definition/Runtime/Lighting/VolumetricLighting/HDRenderPipeline.VolumetricClouds.cs

+                (SystemInfo.graphicsDeviceType == GraphicsDeviceType.Direct3D11 ||
+                    SystemInfo.graphicsDeviceType == GraphicsDeviceType.Direct3D12 ||
+                    SystemInfo.graphicsDeviceType == GraphicsDeviceType.Metal ||
+                    SystemInfo.graphicsDeviceType == GraphicsDeviceType.Vulkan));


If we can add one more check here for MSAA, I think we can close #4345

yes good point, MSAA fix should be integrate in this PR

sebastienlagarde · 2021-04-28T18:13:28Z

Note to self: redirect the PR to master once hd/bugfix is merge

FrancescoC-unity · 2021-04-29T08:20:29Z

Actually @johnpars the problem is slightly different here as blending in place still is happening. I just avoid the Read+Write from same texture, but here the issue is still that with MSAA we'd need to change the UAV declaration to be multi-sampled and do per-sample blending.

I think for MSAA your solution is probably still more efficient, but let's have a chat when you are online

iM0ve

Today I opened my project again to file the fogbugz for concerns. The artefact is just gone and the framerate stabilized after I waited for ~20 seconds. So no more outstanding issues from my part. All looks good

sebastienlagarde · 2021-04-29T10:37:28Z

will wait for discussion with John

* Fix null exception on Raytracing SSS volume component (#4322) * Add null check * changelog * Fix issue with Eye shader and area light (#4310) * Fix Ray Tracing Light Cluster debug mode (#4327) * Remove unneeded position from input layout of light cluster * Changelog Co-authored-by: sebastienlagarde <sebastien@unity3d.com> * Skip adding the LOD's renderer to the checked renderer if it's null (#4313) * Skip adding the LOD's renderer to the checked renderer if it's null * changelog * Add LODgroup missing renderer to "check scene content for ray tracing" * formatting test * formatting Co-authored-by: sebastienlagarde <sebastien@unity3d.com> * Update doc of transparent motion vectors to specify TAA of object behind will be impacted (#4325) * Update doc * Fix doc * Fixed Look Dev breaking when docked and rendered at zero size. (#4331) * Fixed an issue where sometime a docked lookdev could be rendered at zero size and break. * Update changelog # Conflicts: # com.unity.render-pipelines.high-definition/CHANGELOG.md Co-authored-by: sebastienlagarde <sebastien@unity3d.com> * [HDRP] Added a bit about scaling issues when dealing with history buffers (#4333) * Updated troubleshooting custom pass doc * Update Custom-Pass-Troubleshooting.md * Fixed an issue where runtime debug window UI would leak game objects. (#4337) * Fixed an issue where runtime debug window UI would leak game objects. * Update changelog * Updated documentation to support #4313 (#4336) Co-authored-by: Sebastien Lagarde <sebastien@unity3d.com> * Fixed NaNs when denoising pixels where the dot product between normal and view direction is near zero (#4342) Co-authored-by: sebastienlagarde <sebastien@unity3d.com> * updated tooltip (#4334) * Doc for custom matrix for fog (#4352) * [Fogbugz 1324000] Fixing unlit emissive reflections (#4340) * Fix ray tracing unlit emissive exposure. Changelog * Automated tests coverage * update scenes Co-authored-by: Remi Chapelain <remi.chapelain@unity3d.com> Co-authored-by: sebastienlagarde <sebastien@unity3d.com> * [Fogbugz 1329173] Fixing wrong pyramid color dimensions when hardware DRS is on. (#4346) * Fixing bad pyramid color source texture size. DRS hanlder must be queried since the mip levels depend on the source hardware resolution * Changelog Co-authored-by: sebastienlagarde <sebastien@unity3d.com> * Fix RTHandle scale bias [1329977] (#4320) * Fix RTHandle scale bias * Add changelog * Fix history sacle * Fix Co-authored-by: sebastienlagarde <sebastien@unity3d.com> * Fixed emission material properties not being animable (#4366) * Fixed emission material properties not being animable * Updated changelog Co-authored-by: sebastienlagarde <sebastien@unity3d.com> * Fix fog precision (#4364) Co-authored-by: sebastienlagarde <sebastien@unity3d.com> * Fix erroneous tile coordinate calculations in contact shadows (#4335) * Fix wrong tile coord calculations * Changelog * Update test scene for contact shadow to cover data * update references screenshots Co-authored-by: Sebastien Lagarde <sebastien@unity3d.com> * [HDRP]Fix history buffer allocation for AOVs when the request does not come in first frame (#4361) * Fix history buffer allocation for AOVs when the request does not come in first frame * Add a break in the for loop Co-authored-by: sebastienlagarde <sebastien@unity3d.com> * Fix for volumetric clouds performing Read and Write on same texture [Unsupported on some platforms] (#4356) * Fix for usage of camera color RW when not supported. * Re-enable on metal * changelog Co-authored-by: sebastienlagarde <sebastien@unity3d.com> * Update TestCaseFilters.asset * update metal screenshots * missing meta * [Fogbugz 1330769] Fixing bloom bicubic filtering for DRS (#4370) * Fixing bloom bicubic filtering for DRS, bicubic was using the wrong dimensions / texel sizes * Adding better comment to explain offseting of bicubic texture offset fix * update reference screenshots Co-authored-by: sebastienlagarde <sebastien@unity3d.com> Co-authored-by: FrancescoC-unity <43168857+FrancescoC-unity@users.noreply.github.com> Co-authored-by: Rémi Chapelain <57442369+remi-chapelain@users.noreply.github.com> Co-authored-by: JulienIgnace-Unity <julien@unity3d.com> Co-authored-by: Antoine Lelievre <antoinel@unity3d.com> Co-authored-by: Lewis Jordan <lewis.jordan@hotmail.co.uk> Co-authored-by: Pavlos Mavridis <pavlos.mavridis@unity3d.com> Co-authored-by: Adrien de Tocqueville <adrien.tocqueville@unity3d.com> Co-authored-by: Kleber Garcia <kleber.garcia@unity3d.com> Co-authored-by: Remi Chapelain <remi.chapelain@unity3d.com> Co-authored-by: skhiat <55133890+skhiat@users.noreply.github.com>

FrancescoC-unity added 4 commits April 28, 2021 12:29

Fix for usage of camera color RW when not supported.

baf875e

Re-enable on metal

27fe607

Merge branch 'hd/bugfix' into HDRP/fix-clouds-color-rw

2f44632

changelog

79b1807

FrancescoC-unity added the HDRP label Apr 28, 2021

FrancescoC-unity requested review from sebastienlagarde and a team April 28, 2021 10:39

FrancescoC-unity self-assigned this Apr 28, 2021

iM0ve reviewed Apr 28, 2021

View reviewed changes

johnpars reviewed Apr 28, 2021

View reviewed changes

sebastienlagarde changed the base branch from hd/bugfix to master April 28, 2021 18:13

sebastienlagarde changed the base branch from master to hd/bugfix April 28, 2021 18:13

sebastienlagarde approved these changes Apr 28, 2021

View reviewed changes

iM0ve approved these changes Apr 29, 2021

View reviewed changes

Merge branch 'hd/bugfix' into HDRP/fix-clouds-color-rw

fc7b11a

sebastienlagarde merged commit 81eacb2 into hd/bugfix Apr 29, 2021

sebastienlagarde deleted the HDRP/fix-clouds-color-rw branch April 29, 2021 14:52

sebastienlagarde mentioned this pull request Apr 30, 2021

Merge Hd/bugfix #4388

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix for volumetric clouds performing Read and Write on same texture [Unsupported on some platforms] #4356

Fix for volumetric clouds performing Read and Write on same texture [Unsupported on some platforms] #4356

Uh oh!

FrancescoC-unity commented Apr 28, 2021 •

edited

Loading

Uh oh!

github-actions bot commented Apr 28, 2021

Uh oh!

iM0ve left a comment

Uh oh!

FrancescoC-unity commented Apr 28, 2021

Uh oh!

johnpars commented Apr 28, 2021 •

edited

Loading

Uh oh!

johnpars Apr 28, 2021

Uh oh!

sebastienlagarde Apr 28, 2021

Uh oh!

sebastienlagarde commented Apr 28, 2021

Uh oh!

FrancescoC-unity commented Apr 29, 2021 •

edited

Loading

Uh oh!

iM0ve left a comment

Uh oh!

sebastienlagarde commented Apr 29, 2021

Uh oh!

Uh oh!

Fix for volumetric clouds performing Read and Write on same texture [Unsupported on some platforms] #4356

Fix for volumetric clouds performing Read and Write on same texture [Unsupported on some platforms] #4356

Uh oh!

Conversation

FrancescoC-unity commented Apr 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Apr 28, 2021

Uh oh!

iM0ve left a comment

Choose a reason for hiding this comment

Uh oh!

FrancescoC-unity commented Apr 28, 2021

Uh oh!

johnpars commented Apr 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

johnpars Apr 28, 2021

Choose a reason for hiding this comment

Uh oh!

sebastienlagarde Apr 28, 2021

Choose a reason for hiding this comment

Uh oh!

sebastienlagarde commented Apr 28, 2021

Uh oh!

FrancescoC-unity commented Apr 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iM0ve left a comment

Choose a reason for hiding this comment

Uh oh!

sebastienlagarde commented Apr 29, 2021

Uh oh!

Uh oh!

FrancescoC-unity commented Apr 28, 2021 •

edited

Loading

johnpars commented Apr 28, 2021 •

edited

Loading

FrancescoC-unity commented Apr 29, 2021 •

edited

Loading