Fix blend modes and separate preserve specular #1848

eh-unity · 2020-09-11T15:54:16Z

Checklist for PR maker

Have you added a backport label (if needed)? For example, the need-backport-* label. After you backport the PR, the label changes to backported-*.
Have you updated the changelog? Each package has a CHANGELOG.md file.
Have you updated or added the documentation for your PR? When you add a new feature, change a property name, or change the behavior of a feature, it's best practice to include related documentation changes in the same PR. If you do add documentation, make sure to add the relevant Graphics Docs team member as a reviewer of the PR. If you are not sure which person to add, see the Docs team contacts sheet.
Have you added a graphic test for your PR (if needed)? When you add a new feature, or discover a bug that tests don't cover, please add a graphic test.

Purpose of this PR

Fix bug case 1260085 (use straight premultiply blend mode)
Separate preserve specular feature from premultiplied blend. Preserve specular is default.
Define URP blends more explicitly (match HDRP) by setting alpha channel blend too and allow pure blend modes.
Added a blend test case.
Added alpha effect to multiply by lerping to white.
Refactored BakedLit and Unlit implementation shader into their own .hlsl files.
Moved universal shader graph template to universal side.

Testing status

Manual Tests: What did you do?

Opened test project + Run graphic tests locally
Built a player
Checked new UI names with UX convention
Tested UI multi-edition + Undo/Redo + Prefab overrides + Alignment in Preset
C# and shader warnings (supress shader cache to see them)
Checked new resources path for the reloader (in developer mode, you have a button at end of resources that check the paths)
Other:
Added a new test case for blending:
https://github.com/Unity-Technologies/Graphics/blob/8f876cb7beaf56f3ff603e52eb89879e04be39d2/TestProjects/UniversalGraphicsTest/Assets/ReferenceImages/Linear/OSXEditor/Metal/None/066_Blending.png

Automated Tests: What did you setup? (Add a screenshot or the reference image of the test please)

Yamato: (Select your branch):
https://yamato.prd.cds.internal.unity3d.com/jobs/902-Graphics

Any test projects to go with this to help reviewers?

Comments to reviewers

Did not account/fix blend modes in fog
Did not update particle or 2D shaders
Did not handle accumulate alpha (alpha channel blend with Src == Zero)
No tests for blend modes & transparencies in fog
No tests for blending modes with multiple overlapping transparent objects

This reverts commit df48573.

…lar.

Fix materials in URP test cases. Fix some shader issues.

# Conflicts: # com.unity.render-pipelines.universal/Shaders/Unlit.shader

phi-lira · 2020-09-22T08:33:44Z

Assigning @ellioman instead of me as reviewer as I have too many PRs under review atm.

Verasl

Seeing keyword changes but no material upgrader is worrying, a material upgrader will be nessesay here, to also add and set values for the new blend properties for alpha.

A material upgrader is a must.

ellioman · 2020-10-06T12:04:25Z

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

@@ -145,6 +149,7 @@ public virtual void FindProperties(MaterialProperty[] properties)
        {
            surfaceTypeProp = FindProperty("_Surface", properties);
            blendModeProp = FindProperty("_Blend", properties);
+            preserveSpecProp = FindProperty("_PreserveSpecular", properties, false);    // TODO: should be mandatory?


Create a discussion about this so we're not leaving a TODO for something that can be decided before this PR lands. :)

ellioman · 2020-10-06T12:11:16Z

com.unity.render-pipelines.universal/Shaders/BakedLit.shader

@@ -151,7 +157,7 @@ Shader "Universal Render Pipeline/Baked Lit"
                alpha = OutputAlpha(alpha, _Surface);

                return half4(color, alpha);
-            }
+            }*/


ellioman · 2020-10-06T12:12:41Z

com.unity.render-pipelines.universal/Shaders/BakedLit.shader

@@ -393,7 +402,7 @@ Shader "Universal Render Pipeline/Baked Lit"
                alpha = OutputAlpha(alpha, _Surface);

                return half4(color, alpha);
-            }
+            }*/


com.unity.render-pipelines.universal/Shaders/BakedLitForwardPass.hlsl

com.unity.render-pipelines.universal/Shaders/Unlit.shader

fra-frassineti-unity

NOT APPROVED:
"URP/Unlit" Shader does not compile on my machine after upgrading materials. For more info see the attached test doc

Version: 2021.1.0a1.495
Revision: trunk eff81b68729d
Built: Wed, 07 Oct 2020 11:32:35 GMT

Test Doc:
https://docs.google.com/document/d/1tBreRBnfM6R4cNIWqZS-ixZbO_u_FKHo29nFuNO-Dlw/edit?usp=sharing

Added stacked blending testcase.

fra-frassineti-unity · 2020-10-29T12:47:14Z

Since tomorrow will be my last day at Unity, I re-added @Unity-Technologies/gfx-qa-foundation

# Conflicts: # com.unity.render-pipelines.universal/Editor/ShaderGraph/Targets/UniversalTarget.cs # com.unity.render-pipelines.universal/Shaders/BakedLit.shader

cinight · 2020-11-10T10:22:00Z

Talked to Erik. PR is not ready yet. Please add "gfx-qa-foundation" tag as reviewer again when this is ready for testing again.

# Conflicts: # com.unity.render-pipelines.universal/Editor/ShaderGUI/BaseShaderGUI.cs # com.unity.render-pipelines.universal/Editor/ShaderGraph/Targets/UniversalTarget.cs # com.unity.render-pipelines.universal/Editor/ShaderGraph/UniversalFields.cs

# Conflicts: # com.unity.render-pipelines.universal/Editor/ShaderGUI/BaseShaderGUI.cs # com.unity.render-pipelines.universal/Editor/ShaderGraph/Targets/UniversalLitSubTarget.cs # com.unity.render-pipelines.universal/Editor/ShaderGraph/Targets/UniversalTarget.cs # com.unity.render-pipelines.universal/Editor/ShaderGraph/Targets/UniversalUnlitSubTarget.cs # com.unity.render-pipelines.universal/Editor/ShaderGraph/Templates.meta

eh-unity · 2021-09-22T14:30:46Z

Stale. Ended up redoing the change. Link to PR in desc. Closing.

eh-unity added 12 commits September 4, 2020 01:03

Remove _ALPHAPREMULTIPLY_ON from shaders.

df48573

Add blend-mode test case.

b113d42

Revert "Remove _ALPHAPREMULTIPLY_ON from shaders."

dee7300

This reverts commit df48573.

More work on fixing premultiplied blend and separating preserve specu…

84e9c95

…lar.

Move move/duplicate PassMesh.template for Universal.

ba1937a

Match other shaders with Lit.

a7af75c

Add metallic reflective spheres as a test case.

ce17970

Add test case to build options.

c186c5d

Fix test cases.

2d8fbd4

Fix materials in URP test cases. Fix some shader issues.

Fix multiply mode and finish blending test.

a48bf25

Merge branch 'master' into universal/fix-premultiplied-alpha

67ddc06

# Conflicts: # com.unity.render-pipelines.universal/Shaders/Unlit.shader

Fix blending test and update ref.

8f876cb

eh-unity requested review from phi-lira and Verasl September 11, 2020 15:54

github-actions bot added Shader Graph testing universal labels Sep 11, 2020

phi-lira requested review from ellioman and removed request for phi-lira September 22, 2020 08:33

marctem removed the Shader Graph label Sep 29, 2020

Verasl suggested changes Oct 5, 2020

View reviewed changes

Verasl requested a review from a team October 5, 2020 09:32

ellioman reviewed Oct 6, 2020

View reviewed changes

com.unity.render-pipelines.universal/Shaders/BakedLitForwardPass.hlsl Show resolved Hide resolved

ellioman reviewed Oct 6, 2020

View reviewed changes

com.unity.render-pipelines.universal/Shaders/Unlit.shader Show resolved Hide resolved

fra-frassineti-unity requested review from fra-frassineti-unity and removed request for a team October 9, 2020 07:16

fra-frassineti-unity suggested changes Oct 9, 2020

View reviewed changes

eh-unity added 4 commits October 9, 2020 14:45

Fix minor issues.

68aa8d4

Merge branch 'master' into universal/fix-premultiplied-alpha

cfaf31d

Test code for branched logic instead of variants.

defe075

Added stacked blending testcase.

Merge branch 'master' into universal/fix-premultiplied-alpha

7e4d72e

fra-frassineti-unity requested a review from a team October 29, 2020 12:45

Merge branch 'master' into universal/fix-premultiplied-alpha

85eea47

# Conflicts: # com.unity.render-pipelines.universal/Editor/ShaderGraph/Targets/UniversalTarget.cs # com.unity.render-pipelines.universal/Shaders/BakedLit.shader

cinight removed the request for review from a team November 10, 2020 10:21

eh-unity added 3 commits November 20, 2020 14:56

Refactor

da69989

Fix incorrect merge.

d831110

eh-unity mentioned this pull request Feb 4, 2021

Universal/debug views/merged #1799

Closed

10 tasks

eh-unity added 2 commits March 2, 2021 13:05

Move blending test into new test project.

d2f1d85

eh-unity closed this Sep 22, 2021

eh-unity deleted the universal/fix-premultiplied-alpha branch November 17, 2021 13:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix blend modes and separate preserve specular #1848

Fix blend modes and separate preserve specular #1848

Uh oh!

eh-unity commented Sep 11, 2020 •

edited

Loading

Uh oh!

phi-lira commented Sep 22, 2020

Uh oh!

Verasl left a comment

Uh oh!

ellioman Oct 6, 2020

Uh oh!

ellioman Oct 6, 2020

Uh oh!

ellioman Oct 6, 2020

Uh oh!

Uh oh!

Uh oh!

fra-frassineti-unity left a comment

Uh oh!

fra-frassineti-unity commented Oct 29, 2020

Uh oh!

cinight commented Nov 10, 2020

Uh oh!

eh-unity commented Sep 22, 2021

Uh oh!

Uh oh!

Fix blend modes and separate preserve specular #1848

Fix blend modes and separate preserve specular #1848

Uh oh!

Conversation

eh-unity commented Sep 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist for PR maker

Purpose of this PR

Testing status

Comments to reviewers

Uh oh!

phi-lira commented Sep 22, 2020

Uh oh!

Verasl left a comment

Choose a reason for hiding this comment

Uh oh!

ellioman Oct 6, 2020

Choose a reason for hiding this comment

Uh oh!

ellioman Oct 6, 2020

Choose a reason for hiding this comment

Uh oh!

ellioman Oct 6, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

fra-frassineti-unity left a comment

Choose a reason for hiding this comment

Uh oh!

fra-frassineti-unity commented Oct 29, 2020

Uh oh!

cinight commented Nov 10, 2020

Uh oh!

eh-unity commented Sep 22, 2021

Uh oh!

Uh oh!

eh-unity commented Sep 11, 2020 •

edited

Loading