[11.x.x] Soft Particle bugfixes. Cases 1162556, 1256953& 1226288 + 1268079 #1922

ellioman · 2020-09-17T13:58:50Z

Purpose of this PR

This PR fixes these three issues:

[LWRP] UNLIT PARTICLES SHADER IS NOT RENDERED WHEN SOFT PARTICLES ARE ENABLED ON BUILT APPLICATION
URP TRANSPARENT PARTICLE SYSTEM IS NOT VISIBLE / DISAPPEARS WHEN 'DEPTH TEXTURE' IS OFF AND 'SOFT PARTICLES' ARE ENABLED
[URP] DEPTH TEXTURE SAMPLE NODE DOES NOT USE CORRECT TEXTURE IN SOME FRAMES
[URP] OBJECTS THAT ARE USING SOFT PARTICLES ARE RENDERED OPAQUE WHEN OPENGL IS USED

How?

Issue 1, 2 & 3

The solution is to assign a black or white texture as the depth texture, depending on whether we're on a platform with reverse-z or not, if the frame doesn't have a prepass or copydepth pass.

We now also display a warning message in the material inspector when soft particles are enabled but depth texture is not in the asset.

Issue 4

The fix is to use a different LinearEyeDepth() method inside Particles.hlsl. Instead of using the one that uses _ZBufferParams it now uses the one that requires world position and view matrix, which is recommended if we've already computed the world space position.

This ended up being a larger fix as I needed to get access to the world position and instead of simply adding that parameter to the SoftParticles function, I decided to refactor some of the code, create a new struct (ParticleParams) with the common variables used in our particle shaders and create new functions using that struct. That allows us to modify the functions implementations in the future without having to change the API hopefully again.

As all of the particle shaders used the same attributes and varyings, except the editor ones, I decided to move those in to a separate hlsl file to reduce duplicate code.

QA

What has been tested

Tested the three projects sent with the bug reports on Metal and OpenGL in editor
Tested them with Lit, SimpleLit and Unlit particle shaders
Compiled the particle shaders and made sure we don't have any errors or warnings
Ran URP Graphics Tests locally

What needs testing

Soft Particles with and without depth textures
Different Graphics API's (GLES, Vulkan, etc) on builds as I've only tested in editor

Yamato

master: https://yamato.cds.internal.unity3d.com/jobs/902-Graphics/tree/universal%252Fsoft-particle-fixes
2020.2: https://yamato.cds.internal.unity3d.com/jobs/902-Graphics/tree/universal%252Fsoft-particle-fixes/.yamato%252F_abv.yml%2523all_project_ci_CUSTOM-REVISION/3977446/job

ellioman · 2020-09-18T15:13:37Z

com.unity.render-pipelines.universal/Runtime/Passes/CopyDepthPass.cs


                cmd.DrawMesh(RenderingUtils.fullscreenMesh, Matrix4x4.identity, m_CopyDepthMaterial);
-
+                cmd.SetGlobalVector(ShaderPropertyId.availableTexturesParams, new Vector4(0f, 0f, 0f, 1f));


This is probably not a good enough solution if/when we want to add other texture types to this parameter, such as the opaque texture. Could we add the data to RenderingData? Other ideas welcome.

phi-lira

LGTM. We should ask for cross pipeline team / HDRP on what is the design regarding checking if a texture is available in the shader.

com.unity.render-pipelines.universal/ShaderLibrary/Particles.hlsl

phi-lira · 2020-09-21T12:47:12Z

com.unity.render-pipelines.universal/Shaders/Particles/ParticlesInput.hlsl

+#ifndef UNIVERSAL_PARTICLES_INPUT_INCLUDED
+#define UNIVERSAL_PARTICLES_INPUT_INCLUDED
+
+#if PARTICLES_EDITOR


Why is it that we need different attributes/varyings for editor?

I don't know. We need to put that in our task list to look at that.

This smells very strange :)

If i'm reading it correctly, you are saying lighting is totally disabled/broken in the Editor (because you disregard the normal + tangent inputs)

I don't think you want to ship this as-is unless it's already in this broken state in master.

I could fix this in this PR as well but this how Editor Particles are currently on Master. I'm just moving the code into one separate file.

https://github.com/Unity-Technologies/Graphics/blob/master/com.unity.render-pipelines.universal/Shaders/Particles/ParticlesEditorPass.hlsl

No this file you linked isn't for rendering of particles in the editor, it's for rendering the editor specific passes, namely the selection outline and the picking buffer passes.

It seems like I should have added a comment in this file explaining this :)
(I added it when I added instanced particle rendering support, which needs custom passes for picking+selection outlines)

Ahh!! My bad!
Wondering what's the best solution here. Would we be fine with moving the define into the relevant passes and keep my change of having all the Particle attributes/varyings in the same file?

Perhaps add a define to the editor passes, then use it to exclude normal + tangents from the inputs?
You can find the passes by looking at any pass that uses the vertParticleEditor vertex program.

I was also thinking you could just wrap it in #if !UNITY_PASS_META but i think we use META passes for other stuff that may need those inputs. But im not sure about that. I don't know exactly what qualifies as a meta pass.

phi-lira · 2020-09-21T12:51:31Z

When testing, please compile shaders in editor to see if there are no error / warnings in any platform.

…epth texture to black/white if we're not doing a prepass or copydepth

erikabar

Verified bug reports (1162556, 1256953, 1226288, 1268079)
Verified that a message is thrown when Soft Particles are enabled but Depth Texture is disabled
smoke tested Particles FTP project on Mac, Android (Vulkan, OpenGLES3, OpenGLES2) and iOS (Metal)

commit ab21837 Author: Elvar Örn Unnþórsson <[email protected]> Date: Thu Oct 22 16:39:33 2020 +0200 changelog commit de2cb9f Merge: 129a26a 8bc58a4 Author: Elvar Örn Unnþórsson <[email protected]> Date: Thu Oct 22 16:36:24 2020 +0200 Merge branch 'master' into universal/soft-particle-fixes commit 129a26a Merge: 08aa34b ef305e5 Author: Elvar Örn Unnþórsson <[email protected]> Date: Tue Oct 20 20:37:30 2020 +0200 Merge branch 'master' into universal/soft-particle-fixes commit 08aa34b Author: Elvar Örn Unnþórsson <[email protected]> Date: Mon Oct 19 20:59:53 2020 +0200 Changelog commit 8ef7ea2 Merge: 2ca7a38 74151b2 Author: Elvar Örn Unnþórsson <[email protected]> Date: Mon Oct 19 20:48:24 2020 +0200 Merge branch 'master' into universal/soft-particle-fixes commit 2ca7a38 Author: Elvar Örn Unnþórsson <[email protected]> Date: Mon Oct 19 09:57:52 2020 +0200 Changing the message from info to warning commit 7e116bc Merge: 17c7e8a 2e401c6 Author: Elvar Örn Unnþórsson <[email protected]> Date: Mon Oct 19 09:23:33 2020 +0200 Merge branch 'master' into universal/soft-particle-fixes commit 17c7e8a Merge: f0cc212 08aecf9 Author: Elvar Örn Unnþórsson <[email protected]> Date: Wed Oct 7 14:46:24 2020 +0200 Merge branch 'master' into universal/soft-particle-fixes commit f0cc212 Merge: 66a1f75 9c25c9f Author: Elvar Örn Unnþórsson <[email protected]> Date: Mon Oct 5 15:38:28 2020 +0200 Merge branch 'master' into universal/soft-particle-fixes commit 66a1f75 Merge: b3dc096 168109e Author: Elvar Örn Unnþórsson <[email protected]> Date: Fri Sep 25 14:42:28 2020 +0200 Merge branch 'master' into universal/soft-particle-fixes commit b3dc096 Author: Elvar Örn Unnþórsson <[email protected]> Date: Thu Sep 24 15:46:20 2020 +0200 Renaming PARTICLES_EDITOR define to PARTICLES_EDITOR_META_PASS commit 9ac74d1 Author: Elvar Örn Unnþórsson <[email protected]> Date: Thu Sep 24 14:08:58 2020 +0200 Fixing things based on reviews commit 4c3dc37 Merge: 5a15af0 b9696e6 Author: Elvar Örn Unnþórsson <[email protected]> Date: Wed Sep 23 10:31:23 2020 +0200 Merge branch 'master' into universal/soft-particle-fixes commit 5a15af0 Author: Elvar Örn Unnþórsson <[email protected]> Date: Wed Sep 23 10:30:56 2020 +0200 Fixing an copy/paste error commit 0fcaad0 Author: Elvar Örn Unnþórsson <[email protected]> Date: Tue Sep 22 15:45:03 2020 +0200 Fixes based on review commit 03d0a2e Merge: 63af228 e35abbc Author: Elvar Örn Unnþórsson <[email protected]> Date: Tue Sep 22 15:36:17 2020 +0200 Merge branch 'master' into universal/soft-particle-fixes commit 63af228 Author: Elvar Örn Unnþórsson <[email protected]> Date: Tue Sep 22 15:35:50 2020 +0200 Fixing compile errors commit 776f167 Author: Elvar Örn Unnþórsson <[email protected]> Date: Tue Sep 22 13:34:41 2020 +0200 Removing HasDepthTexture() from the shaders and instead setting the depth texture to black/white if we're not doing a prepass or copydepth commit 739160a Author: Elvar Örn Unnþórsson <[email protected]> Date: Fri Sep 18 15:42:15 2020 +0200 Minor fix to ParticlesEditorPass commit ca416b1 Author: Elvar Örn Unnþórsson <[email protected]> Date: Fri Sep 18 15:41:37 2020 +0200 Fixing issue 1226288 commit 77d80bf Author: Elvar Örn Unnþórsson <[email protected]> Date: Thu Sep 17 15:45:47 2020 +0200 Bugfix 1162556

…, which is incorrect.

phi-lira · 2020-10-27T15:37:32Z

merging to stage branch

* [11.x.x] Fixing NullReferenceException with Lift Gamma Gain & upgrading URP Package (#2386) * Fixing issue 1283588 * Similar fix to ShadowsMidtonesHighlightsEditor.cs * Minor fixes * More minor fixes * [11.x.x] Soft Particle bugfixes. Cases 1162556, 1256953& 1226288 + 1268079 (#1922) * Bugfix 1162556 * Fixing issue 1226288 * Minor fix to ParticlesEditorPass * Removing HasDepthTexture() from the shaders and instead setting the depth texture to black/white if we're not doing a prepass or copydepth * Fixing compile errors * Fixes based on review * Fixing an copy/paste error * Fixing things based on reviews * Renaming PARTICLES_EDITOR define to PARTICLES_EDITOR_META_PASS * Changing the message from info to warning * Changelog * changelog * Bugfix: The code was overriding the depth texture for overlay cameras, which is incorrect. * [11.x.x] MSAA fix for Metal MacOS/Editor (#2391) * Metal MacOS/Editor MSAA fix. Require an explicit MSAA resolve pass unless the platform is mobile * Added changelog entry * [11.x.x] Fixed per camera MSAA regression (#2293) * Default to XR off when creating the camera and no XR provider is installed. Added runtime XR on/off check for XR camera. * Updated CHANGELOG.md and small format fixes. * Reverted changes in URPCameraEditor and URPCameraData. * [11.x.x] Early camera color target assignment (#2377) * Assign the camera color target early in case it is needed during AddRenderPasses * Update to reflect PR feedback * Get rid of warnings from accessing cameraColorTarget during AddRenderPasses * Update CHANGELOG.md * Removed tile shaders are currently unused. (#2383) Co-authored-by: Elvar Örn Unnþórsson <[email protected]> Co-authored-by: manuele-bonanno <[email protected]> Co-authored-by: thomas-zeng <[email protected]> Co-authored-by: Peter Bay Bastian <[email protected]> Co-authored-by: Kay Chang <[email protected]>

…68079 (#1922) * Bugfix 1162556 * Fixing issue 1226288 * Minor fix to ParticlesEditorPass * Removing HasDepthTexture() from the shaders and instead setting the depth texture to black/white if we're not doing a prepass or copydepth * Fixing compile errors * Fixes based on review * Fixing an copy/paste error * Fixing things based on reviews * Renaming PARTICLES_EDITOR define to PARTICLES_EDITOR_META_PASS * Changing the message from info to warning * Changelog * changelog * Bugfix: The code was overriding the depth texture for overlay cameras, which is incorrect. # Conflicts: # com.unity.render-pipelines.universal/CHANGELOG.md

* [11.x.x] Fixing NullReferenceException with Lift Gamma Gain & upgrading URP Package (#2386) * Fixing issue 1283588 * Similar fix to ShadowsMidtonesHighlightsEditor.cs * Minor fixes * More minor fixes # Conflicts: # com.unity.render-pipelines.universal/CHANGELOG.md * [11.x.x] Soft Particle bugfixes. Cases 1162556, 1256953& 1226288 + 1268079 (#1922) * Bugfix 1162556 * Fixing issue 1226288 * Minor fix to ParticlesEditorPass * Removing HasDepthTexture() from the shaders and instead setting the depth texture to black/white if we're not doing a prepass or copydepth * Fixing compile errors * Fixes based on review * Fixing an copy/paste error * Fixing things based on reviews * Renaming PARTICLES_EDITOR define to PARTICLES_EDITOR_META_PASS * Changing the message from info to warning * Changelog * changelog * Bugfix: The code was overriding the depth texture for overlay cameras, which is incorrect. # Conflicts: # com.unity.render-pipelines.universal/CHANGELOG.md * [11.x.x] MSAA fix for Metal MacOS/Editor (#2391) * Metal MacOS/Editor MSAA fix. Require an explicit MSAA resolve pass unless the platform is mobile * Added changelog entry # Conflicts: # com.unity.render-pipelines.universal/CHANGELOG.md * [11.x.x] Fixed per camera MSAA regression (#2293) * Default to XR off when creating the camera and no XR provider is installed. Added runtime XR on/off check for XR camera. * Updated CHANGELOG.md and small format fixes. * Reverted changes in URPCameraEditor and URPCameraData. # Conflicts: # com.unity.render-pipelines.universal/CHANGELOG.md * [11.x.x] Early camera color target assignment (#2377) * Assign the camera color target early in case it is needed during AddRenderPasses * Update to reflect PR feedback * Get rid of warnings from accessing cameraColorTarget during AddRenderPasses * Update CHANGELOG.md # Conflicts: # com.unity.render-pipelines.universal/CHANGELOG.md Co-authored-by: Elvar Örn Unnþórsson <[email protected]> Co-authored-by: manuele-bonanno <[email protected]> Co-authored-by: thomas-zeng <[email protected]> Co-authored-by: Peter Bay Bastian <[email protected]>

Bugfix 1162556

77d80bf

ellioman added the universal label Sep 17, 2020

ellioman added 2 commits September 18, 2020 15:41

Fixing issue 1226288

ca416b1

Minor fix to ParticlesEditorPass

739160a

ellioman requested a review from phi-lira September 18, 2020 14:02

ellioman commented Sep 18, 2020

View reviewed changes

phi-lira approved these changes Sep 21, 2020

View reviewed changes

ellioman added 6 commits September 22, 2020 13:34

Removing HasDepthTexture() from the shaders and instead setting the d…

776f167

…epth texture to black/white if we're not doing a prepass or copydepth

Fixing compile errors

63af228

Merge branch 'master' into universal/soft-particle-fixes

03d0a2e

Fixes based on review

0fcaad0

Fixing an copy/paste error

5a15af0

Merge branch 'master' into universal/soft-particle-fixes

4c3dc37

erikabar self-requested a review September 23, 2020 10:06

ellioman added 2 commits September 24, 2020 14:08

Fixing things based on reviews

9ac74d1

Renaming PARTICLES_EDITOR define to PARTICLES_EDITOR_META_PASS

b3dc096

ellioman marked this pull request as ready for review September 25, 2020 12:40

ellioman requested a review from a team as a code owner September 25, 2020 12:40

ellioman added 3 commits September 25, 2020 14:42

Merge branch 'master' into universal/soft-particle-fixes

66a1f75

Merge branch 'master' into universal/soft-particle-fixes

f0cc212

Merge branch 'master' into universal/soft-particle-fixes

17c7e8a

ghost approved these changes Oct 8, 2020

View reviewed changes

ellioman changed the title ~~[10.x.x] Soft Particle bugfixes. Cases 1162556, 1256953 and 1226288~~ [10.x.x] Soft Particle bugfixes. Cases 1162556, 1256953& 1226288 + 1268079 Oct 13, 2020

erikabar approved these changes Oct 16, 2020

View reviewed changes

ellioman added 5 commits October 19, 2020 09:23

Merge branch 'master' into universal/soft-particle-fixes

7e116bc

Changing the message from info to warning

2ca7a38

Merge branch 'master' into universal/soft-particle-fixes

8ef7ea2

Changelog

08aa34b

Merge branch 'master' into universal/soft-particle-fixes

129a26a

ellioman added 2 commits October 22, 2020 16:36

Merge branch 'master' into universal/soft-particle-fixes

de2cb9f

changelog

ab21837

ellioman added a commit that referenced this pull request Oct 26, 2020

Backporting #1922

df2c4c0

ellioman mentioned this pull request Oct 26, 2020

[10.2.0] Several bugfixes for 10.2 #2372

Closed

phi-lira mentioned this pull request Oct 26, 2020

[11.x.x] Batch of PRs #2380

Closed

ellioman changed the title ~~[10.x.x] Soft Particle bugfixes. Cases 1162556, 1256953& 1226288 + 1268079~~ [11.x.x] Soft Particle bugfixes. Cases 1162556, 1256953& 1226288 + 1268079 Oct 26, 2020

phi-lira mentioned this pull request Oct 27, 2020

[11.x.x] Batch of PRs v2 #2390

Merged

Bugfix: The code was overriding the depth texture for overlay cameras…

7270d06

…, which is incorrect.

phi-lira approved these changes Oct 27, 2020

View reviewed changes

phi-lira added the needs-backport-10.x label Oct 27, 2020

phi-lira changed the base branch from master to universal/staging-v3 October 27, 2020 15:37

phi-lira merged commit 09588b3 into universal/staging-v3 Oct 27, 2020

phi-lira deleted the universal/soft-particle-fixes branch October 27, 2020 15:37

phi-lira mentioned this pull request Oct 27, 2020

[11.x.x] Batch of PRs v3 #2401

Merged

phi-lira added the backported-10.x label Oct 29, 2020

sebastienlagarde removed the needs-backport-10.x label Jan 18, 2021

phi-lira restored the universal/soft-particle-fixes branch July 13, 2021 16:03

phi-lira added a commit that referenced this pull request Jul 13, 2021

Backport of #1922

600780b

phi-lira mentioned this pull request Jul 13, 2021

Backport of #1922 #5135

Merged

sebastienlagarde deleted the universal/soft-particle-fixes branch September 1, 2021 10:20

phi-lira added a commit that referenced this pull request Sep 21, 2021

Backport of #1922 (#5135)

05bbeff


		cmd.DrawMesh(RenderingUtils.fullscreenMesh, Matrix4x4.identity, m_CopyDepthMaterial);

		cmd.SetGlobalVector(ShaderPropertyId.availableTexturesParams, new Vector4(0f, 0f, 0f, 1f));

[11.x.x] Soft Particle bugfixes. Cases 1162556, 1256953& 1226288 + 1268079 #1922

[11.x.x] Soft Particle bugfixes. Cases 1162556, 1256953& 1226288 + 1268079 #1922

Uh oh!

Conversation

ellioman commented Sep 17, 2020 • edited by phi-lira Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose of this PR

How?

Issue 1, 2 & 3

Issue 4

QA

What has been tested

What needs testing

Yamato

Uh oh!

ellioman Sep 18, 2020

Choose a reason for hiding this comment

Uh oh!

phi-lira left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

phi-lira Sep 21, 2020

Choose a reason for hiding this comment

Uh oh!

ellioman Sep 22, 2020

Choose a reason for hiding this comment

Uh oh!

ghost Sep 23, 2020

Choose a reason for hiding this comment

Uh oh!

ellioman Sep 24, 2020

Choose a reason for hiding this comment

Uh oh!

ghost Sep 24, 2020 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost Sep 24, 2020

Choose a reason for hiding this comment

Uh oh!

ellioman Sep 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost Sep 24, 2020

Choose a reason for hiding this comment

Uh oh!

phi-lira commented Sep 21, 2020

Uh oh!

erikabar left a comment

Choose a reason for hiding this comment

Uh oh!

phi-lira commented Oct 27, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ellioman commented Sep 17, 2020 •

edited by phi-lira

Loading

ghost Sep 24, 2020 •

edited by ghost

Loading

ellioman Sep 24, 2020 •

edited

Loading