[2021.2] [ShaderGraph] URP Uber Shader #3705

cdxntchou · 2021-03-01T20:05:51Z

Purpose of this PR

This PR adds an "Allow Material Override" checkbox to Lit and Unlit subtargets, as well as controls for Cast and Receive Shadows, Depth Write and Depth Test.

By default, "Allow Material Override" will not be checked, both to preserve the behavior of existing shadergraphs, and to simplify the initial user experience. When checked, it allows the Material to control the following surface options (that were previously only controlled from the ShaderGraph):

For Lit subtargets:

Workflow Mode
Surface Type
Render Face
Depth Write
Depth Test
Alpha Clipping
Cast Shadows
Receive Shadows

For Unlit subtargets:

Surface Type
Render Face
Depth Write
Depth Test
Alpha Clipping
Cast Shadows

Example Unlit Material, when"Allow Material Override" is checked on the ShaderGraph:

It achieves this by modifying the subtargets to generate properties and keywords to control the surface options, in combination with a setup function that applies state updates to the Materials to control these properties and keywords: ShaderUtils.UpdateMaterial(material). To be able to run the correct update on each material, we created a ShaderID enum to identify the various Universal shader types, extending the existing ShaderPathID to include shadergraph generated shaders. We also attach metadata to Universal ShaderGraph assets, so we can tag them with their ShaderID. The helper function ShaderUtils.GetShaderID() is provided to get the ShaderID for any shader, either hand-written or shadergraph generated.

This PR also creates two new Material Inspectors: ShaderGraphLitGUI (for Lit subtarget), and ShaderGraphUnlitGUI (For Unlit subtarget) to handle displaying the surface controls appropriately, and calling the update function when needed.

These inspectors are based off of the BaseShaderGUI base class, so some changes are made to BaseShaderGUI so that it works with ShaderGraph shaders in addition to the existing hand-written shaders.

This PR also makes a few associated changes to ShaderGraph:

Adds "ShaderGraphShader"="True" tag to all ShaderGraph-built subshaders, allowing us to easily tell when a Material is a ShaderGraph (even if the ShaderGraph is not asset based). Added "Material.IsShaderGraph" utility function, and changed the old "Shader.IsShaderGraph" function to "Shader.IsShaderGraphAsset" to make it clear it only works on Assets.
Adds "Stages" dropdown to Keywords, allowing user to set up "Fragment" or "Vertex" only keywords to reduce compiled variants.
Adds linearGrey and red options as default texture mode, and changed the "bump" option to be called "Normal Map"
Texture2D properties using Normal Map (old "bump") default will now be marked with [NormalMap] tag, so that users are reminded to assign a normal map usage texture to that property.
Adds CastsShadows option (for Materials that use Lit/Unlit SubTargets)
Adds Depth Test control (for Materials that use Lit/Unlit SubTargets)
Made the SubTarget and Material controls look identical (same naming, same options available)

Testing status

Describe what manual/automated tests were performed for this PR

Tested:

Yamato:

ShaderGraph PR Job: 🟢
https://yamato.cds.internal.unity3d.com/jobs/902-Graphics/tree/sg%252Furp-uber/.yamato%252Fall-shadergraph.yml%2523PR_ShaderGraph_trunk/6559408/job/pipeline

Universal PR Job: matches equivalent master results 🟢
https://yamato.cds.internal.unity3d.com/jobs/902-Graphics/tree/sg%252Furp-uber/.yamato%252Fall-universal_split.yml%2523PR_Universal_Split_trunk/6560960/job/pipeline

failing jobs:
URP_PostPro on Win_DX11_Standalone_XR_cache_mono_Linear on version trunk -- same 24/55 tests failed.
URP_PostPro on Win_DX11_playmode_XR_cache_mono_Linear on version trunk -- same 24/55 tests failed.
URP_PostPro on Win_DX12_Standalone_XR_cache_mono_Linear on version trunk -- same 24/55 tests failed.
URP_PostPro on OSX_Metal_playmode_cache_mono_Linear on version trunk -- success on re-run
URP_Lighting on Win_DX11_Standalone_XR_cache_mono_Linear on version trunk -- same 28/75 tests failed.
URP_Lighting on Win_DX11_playmode_XR_cache_mono_Linear on version trunk -- same 28/94 tests failed.
URP_Lighting on Win_DX12_Standalone_XR_cache_mono_Linear on version trunk -- same 28/75 tests failed.
URP_Foundation on Win_DX11_Standalone_cache_mono_Linear on version trunk -- success on re-run
URP_Foundation on Win_DX11_Standalone_XR_cache_mono_Linear on version trunk -- same 60/94 tests failed.
URP_Foundation on Win_DX11_playmode_XR_cache_mono_Linear on version trunk -- same 60/113 tests failed.
URP_Foundation on Win_DX12_Standalone_XR_cache_mono_Linear on version trunk -- same 60/94 tests failed.
URP_Foundation on OSX_Metal_playmode_cache_mono_Linear on version trunk -- success on re-run
URP_Terrain on Win_DX11_playmode_XR_cache_mono_Linear on version trunk -- same 8/68 tests failed.
URP_2D on Win_DX11_playmode_XR_cache_mono_Linear on version trunk -- same 18/74 tests failed.
URP_2D on OSX_OpenGLCore_editmode_mono_Linear on version trunk -- same 1/63 tests failed.
URP_2D on Win_DX11_editmode_mono_Linear on version trunk -- same 1/63 tests failed.

equivalent master version: ed96290

master failing job;
https://yamato.cds.internal.unity3d.com/jobs/902-Graphics/tree/master/.yamato%252Fall-universal_split.yml%2523Nightly_Universal_Split_trunk/6518789/job/pipeline

HDRP PR Job: 🟢
https://yamato.cds.internal.unity3d.com/jobs/902-Graphics/tree/sg%252Furp-uber/.yamato%252Fall-hdrp.yml%2523PR_HDRP_trunk/6433493/job/pipeline

latest after merge: lots of failures, but matches trunk 🟢
https://yamato.cds.internal.unity3d.com/jobs/902-Graphics/tree/sg%252Furp-uber/.yamato%252Fall-hdrp.yml%2523PR_HDRP_trunk/6567033/job/pipeline

rerunning:
Build HDRP on Win_DX11_mono_Linear_Standalone_cache_build_Player on version trunk
HDRP on Win_DX11_editmode_mono_Linear on version trunk

Comments to reviewers

Notes for the reviewers you have assigned.

Discovered Issues for emulating Lit.Shader / Unlit.Shader,
These won't necessarily be fixed in this PR, I'm just noting the issue for future work on the Lit.shadergraph:

no linearGrey default color option (this PR)
no MainTexture / MainColor tags (separate PR)
no ToggleOff tag
no Normal tag (this PR)
no HiddenInInspector tag exposed on blackboard properties (only internal in code) -- can hack serialized file
no Texture Scale and Offset support (separate PR)
keyword "stage" not exposed on blackboard keywords -- added drop down (this PR)
ShaderGraph Unlit subtarget casts shadows by default, Unlit.Shader does NOT, will need to make sure to flag Unlit.ShaderGraph as non-shadowcasting by default to maintain behavior.
ShaderGraph generated meta passes (for Global Illumination) do not do the same diffuse/specular color blend that Lit.shader does

Issues Found, that are Not Fixed / by design for now:

Transparent Multiply mode ignores alpha (pre-existing issue -- may be intended?)
Transparent Premultiply mode -- specular ignores alpha (pre-existing issue -- this is intended)
culling option currently also controls culling in shadow passes. May want independent shadow pass culling control like HDRP?
DepthOnly needs to be stripped when ZWrite is force disabled
should we strip DepthNormalOnly passes when ZWrite is force disabled?
Baked GI for alpha-clipped shadergraphs is not correct -- will ignore the alpha clipping. Seems to be a limitation of the meta-pass approach to gathering GI values?
meta pass GI also doesn't seem to take into account differing values for the front and back sides of the object -- even when double-sided GI is enabled on the Material.
Switching URP ==> HDRP does not currently update material keywords (until you open the material inspector)
Swtiching HDRP ==> URP does not currently update material keywords (until you open the material inspector)

…rGUI material inspector, cleaning up keyword code generation

…h zero stages

…um to match the material UI

…w toggles to the SubTarget UI

…erties for missing blocks

…lds and cleaning up code

…missive GI

# Conflicts: # com.unity.render-pipelines.universal/Editor/ShaderGUI/BaseShaderGUI.cs # com.unity.render-pipelines.universal/Editor/ShaderGUI/ShadingModels/LitGUI.cs # com.unity.render-pipelines.universal/Editor/ShaderGraph/Targets/UniversalLitSubTarget.cs # com.unity.shadergraph/Editor/Generation/Processors/Generator.cs

…lling PreviewMaterial setup on URP Lit shadergraphs (fixup code not designed to work on shadergraphs)

cdxntchou · 2021-04-22T16:55:35Z

@phi-lira @eh-unity @Verasl @LandonTownsendUnity This PR is ready to go from my standpoint, all yamato tests passing, all of the feedback addressed: the default behavior is now identical to the old behavior (Target control), and the property and keyword names match Lit.shader / Unlit.shader. Please check if there is anything else that should be changed!

I've got tests being added in a separate branch: sg/urp-uber-tests (all green so far there as well), but would like to get this large change in to unblock dependent work and avoid continuous master merge fixups and re-running yamato tests..

…entirely if we know it will never use zwrite enabled

…tion

…election

phi-lira

LGTM. Minor comments, missing public API docs.
Also, PBRMasterGUI.cs file was deleted but I didn't see any .meta file being deleted for that?

com.unity.render-pipelines.universal/Editor/AssetPostProcessors/MaterialPostprocessor.cs

com.unity.render-pipelines.universal/Editor/ShaderGUI/BaseShaderGUI.cs

phi-lira · 2021-04-26T12:29:25Z

com.unity.render-pipelines.universal/Editor/ShaderGUI/BaseShaderGUI.cs

+                if (isShaderGraph)
+                {
+                    // Lit.shadergraph or Unlit.shadergraph, but no material control defined
+                    // enable the pass in the material, so shader can decide...
+                    castShadows = true;
+                }
+                else
+                {
+                    // Lit.shader or Unlit.shader -- set based on transparency
+                    castShadows = Rendering.Universal.ShaderGUI.LitGUI.IsOpaque(material);
+                }


Thanks for handling this. I'll have a discussion with @Verasl I think URP team should align with ShaderGraph (we can do in follow up PR)

com.unity.render-pipelines.universal/Editor/ShaderGUI/BaseShaderGUI.cs

com.unity.render-pipelines.universal/Editor/ShaderGraph/Includes/DepthNormalsOnlyPass.hlsl

com.unity.render-pipelines.universal/Editor/ShaderGraph/Targets/UniversalTarget.cs

com.unity.shadergraph/Editor/Generation/TargetResources/Fields.cs

eh-unity

Looks pretty good to me.

eh-unity · 2021-04-26T13:17:24Z

com.unity.render-pipelines.universal/Editor/ShaderGraph/Targets/UniversalLitSubTarget.cs

+
+            // when the surface options are material controlled, we must show all of these blocks
+            // when target controlled, we can cull the unnecessary blocks
+            context.AddBlock(BlockFields.SurfaceDescription.Specular,           (workflowMode == WorkflowMode.Specular) || target.allowMaterialOverride);


Since there are these clear sections for different "kinds" of blocks, perhaps we should separate them into internal helper functions?

eh-unity · 2021-04-26T13:34:49Z

com.unity.render-pipelines.universal/Editor/ShaderGraph/Targets/UniversalLitSubTarget.cs

+                if (target.mayWriteDepth)
+                    result.passes.Add(CorePasses.DepthOnly(target));
+
+                result.passes.Add(LitPasses.DepthNormalOnly(target));


FYI, I added method chaining/fluent interface support to the shadergraph collections a while back. You could write these as result.passes.Add().Add().Add()...
I think this is fine as is not necessary to change.

erikabar

@cinight is on this PR and will test it, I am approving without testing.

…ACE_TYPE_TRANSPARENT) warnings

# Conflicts: # com.unity.render-pipelines.universal/CHANGELOG.md # com.unity.render-pipelines.universal/Editor/ShaderGraph/Targets/UniversalTarget.cs # com.unity.render-pipelines.universal/Editor/ShaderGraph/Targets/UniversalUnlitSubTarget.cs # com.unity.shadergraph/CHANGELOG.md

ghost

Developer manual testing is sufficient

ghost

That render queue override we saw during the pair testing doesn't exist on master - it looks like this PR caused that.

…shaders

…o what was there before

ghost

Approving now that the advanced controls are fixed.

cinight

All good. Test doc: https://docs.google.com/document/d/1Td6gGYRSYRDAXeFViWiYlR3IWktUeRA_BT8gdUdSZu8/edit#

Chris Tchou added 4 commits February 26, 2021 16:30

Working transparent/opaque switch

613a536

Working surface type, cull mode, and alpha clip

5d4ffac

Fix for Unlit alpha test shadows

aa88cf4

Workflow Mode switch working on Lit

c11b947

github-actions bot added Shader Graph universal labels Mar 1, 2021

Chris Tchou added 24 commits March 1, 2021 12:19

Lit AlphaTest working

9a9f688

Fix for Lit ReceiveShadows checkbox

ff00c31

Draw shadergraph properties in material inspector

10b540b

Cleanup, Setting up Target defaults correctly

ca653cb

Fix for error spew from inspector on blank graphs

d131202

Cast Shadows Toggle

4afe766

Adding LinearGrey and Red default texture options

954ab83

Adding stage options to Keywords, fixing Keyword control in BaseShade…

24ea748

…rGUI material inspector, cleaning up keyword code generation

Fix for collision with HDRP property names, fix keyworddescriptor wit…

39ea696

…h zero stages

Fix Previews, Convert target "Two Sided" checkbox to "Render Face" en…

4190ef4

…um to match the material UI

Fix for transparent blending with alpha

1a47133

Missing setup for transparent blending with alpha

2c94bc6

Fix for material alpha clip when shader alpha clip disabled

6589a2d

Make SubTarget UI the same as Material UI, and add cast/receive shado…

31637a2

…w toggles to the SubTarget UI

Remove debug log spew

6a5c9e6

Add workflow mode material control checkbox, fix default preview prop…

7772681

…erties for missing blocks

Fixing cull mode in shadow and depth-only passes, removing unused fie…

e126fca

…lds and cleaning up code

Removing unused PBRMasterGUI, cleaning up code and comments, fixing e…

8859752

…missive GI

Fixing predefined keyword -- should not define anything

aea47f5

Keyword test fix

c489b0c

Add "ShaderGraphShader" tag to all SubTargets. Fix for errors when ca…

7a56f74

…lling PreviewMaterial setup on URP Lit shadergraphs (fixup code not designed to work on shadergraphs)

Add versioning to shadergraph materials

1ab31e2

Use UniversalMetadata approach

8115bf5

cdxntchou requested a review from phi-lira April 22, 2021 16:50

Chris Tchou added 5 commits April 22, 2021 14:51

Removing ZTestMode.Disabled as an option

61e1f44

Disabling DepthOnly pass when zwrite is off, removing it from shader …

4010912

…entirely if we know it will never use zwrite enabled

Fix syntax error, moving global illumination settings to advanced sec…

2684ca8

…tion

Moving "Allow Material Override" checkbox above the "workflow mode" s…

c187236

…election

Merge branch 'sg/urp-uber-tests' into sg/urp-uber

828cfae

cinight self-requested a review April 26, 2021 12:08

phi-lira approved these changes Apr 26, 2021

View reviewed changes

eh-unity approved these changes Apr 26, 2021

View reviewed changes

erikabar approved these changes Apr 27, 2021

View reviewed changes

Chris Tchou added 5 commits April 27, 2021 11:00

Addressing feedback

5c5f64c

Getting rid of error spew by matching property type to HDRP

e748dc8

Fixing unreachable code warnings, and fixing colliding keyword (_SURF…

e0d8306

…ACE_TYPE_TRANSPARENT) warnings

Fixing merge issues

bcbb7f9

ghost approved these changes Apr 27, 2021

View reviewed changes

ghost suggested changes Apr 27, 2021

View reviewed changes

Chris Tchou added 2 commits April 27, 2021 21:38

Removing automatic RenderQueue calculation behavior from ShaderGraph …

ce87a2f

…shaders

Ensuring the GI controls on URP ShaderGraph Materials are identical t…

f4e57f3

…o what was there before

ghost self-requested a review April 28, 2021 17:28

ghost approved these changes Apr 28, 2021

View reviewed changes

cdxntchou requested a review from marctem April 29, 2021 05:24

cinight approved these changes Apr 29, 2021

View reviewed changes

Merge branch 'master' into sg/urp-uber

ebe155c

marctem approved these changes Apr 29, 2021

View reviewed changes

marctem merged commit 973e3ef into master Apr 29, 2021

marctem deleted the sg/urp-uber branch April 29, 2021 18:34

jessebarker mentioned this pull request May 6, 2021

SRP-Picking changes #3419

Closed

PaulDemeulenaere mentioned this pull request Aug 30, 2021

[URP] Factorize template & SubTarget<UniversalTarget> #5467

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[2021.2] [ShaderGraph] URP Uber Shader #3705

[2021.2] [ShaderGraph] URP Uber Shader #3705

Uh oh!

cdxntchou commented Mar 1, 2021 •

edited

Loading

Uh oh!

cdxntchou commented Apr 22, 2021

Uh oh!

phi-lira left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

phi-lira Apr 26, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eh-unity left a comment

Uh oh!

eh-unity Apr 26, 2021

Uh oh!

eh-unity Apr 26, 2021

Uh oh!

erikabar left a comment

Uh oh!

ghost left a comment

Uh oh!

ghost left a comment

Uh oh!

ghost left a comment

Uh oh!

cinight left a comment

Uh oh!

Uh oh!

[2021.2] [ShaderGraph] URP Uber Shader #3705

[2021.2] [ShaderGraph] URP Uber Shader #3705

Uh oh!

Conversation

cdxntchou commented Mar 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose of this PR

Testing status

Comments to reviewers

Uh oh!

cdxntchou commented Apr 22, 2021

Uh oh!

phi-lira left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

phi-lira Apr 26, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eh-unity left a comment

Choose a reason for hiding this comment

Uh oh!

eh-unity Apr 26, 2021

Choose a reason for hiding this comment

Uh oh!

eh-unity Apr 26, 2021

Choose a reason for hiding this comment

Uh oh!

erikabar left a comment

Choose a reason for hiding this comment

Uh oh!

ghost left a comment

Choose a reason for hiding this comment

Uh oh!

ghost left a comment

Choose a reason for hiding this comment

Uh oh!

ghost left a comment

Choose a reason for hiding this comment

Uh oh!

cinight left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cdxntchou commented Mar 1, 2021 •

edited

Loading