Skip to content

[Shader Graph] Stack Master#344

Closed
Kink3d wants to merge 1839 commits into
masterfrom
sg/stack-master-v2-upgrade-hdrp
Closed

[Shader Graph] Stack Master#344
Kink3d wants to merge 1839 commits into
masterfrom
sg/stack-master-v2-upgrade-hdrp

Conversation

@Kink3d
Copy link
Copy Markdown
Contributor

@Kink3d Kink3d commented May 1, 2020

alt text

Purpose of this PR

Problem

Currently, the master node definition is too rigid and defined via leaky abstractions. This gives the the following problems:

  • Large list of individual master nodes. Changing master node means reconnecting all edges.
  • Many HDRP features require HDRP specific master nodes. This prevents the shader from compiling in URP.
  • Users cannot define new master nodes without internal Shader Graph access.
  • Ports from different stages are mixed into the same master node, this is confusing for users.

There are other problems involved but these are the main concerns because they seriously inhibit our ability to open up our API to customers (internal right now, external later). As Shader Graph adoption grows, this problem continues to get worse.

Solution

We propose a solution where, instead of providing a collection of master nodes, we provide a single, extensible stack. Users will enable and disable shader features, per render pipeline, in the Inspector. These features provide input ports on the stack, where the user then connects graph edges to provide data for those features. Different render pipelines then build a shader using the input data they support, based on the pipeline configuration. This ensures all features are available, but the shader is always in a valid state.

Goals

  • All features available from any render pipeline, still compiles on all render pipelines by ignoring unsupported features.
  • UX concepts applied meaningfully, and in line with other graph products.
  • Extensible design language for future feature work

What do these mean in practice?

  • No more master node explosion, all features should be available from one stack
  • All HDRP features must be available, and this shader should always have a fallback on URP
  • Better parity with VFX and VS, using the same visual language as their stack implementations
  • Better visual distinction between vertex and fragment stages.
  • Lay the foundations for…
    • Custom interpolators
    • Public “master node API”
    • Layering/custom passes

TODO

  • Fix remaining red tests
  • Fix known issues
  • Add new tests (edit/graphics)
  • Test for performance regressions in graph
  • Add changelog entries

Testing status

PR Template:

  • 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)

General Graph usage

  • Fill this out...

Upgrade Tests:

  • Upgrading V0 > V2
  • Upgrading V1 > V2
  • All master nodes
    • Various ports connected
    • Various settings enabled
    • Ports that require settings and connected
    • Default port values changed
  • PBR specular/metallic (only add one block)
  • Alpha clip upgraded from slot logic > bool (PBR/Unlit)
    • Alpha clip value changed
    • Alpha clip connected
  • Multiple master nodes
  • Master position > Context position
  • Sprite masters result in SpriteColor block (legacy)
    • If BaseColor or Alpha are added SpriteColor is ignored
    • SpriteColor cannot be readded
  • AxF shaders in URP package
  • Upgrading VFX SGs used in VFX graphs

Automated Tests:

  • To add. See TODO checklist.

Any test projects to go with this to help reviewers?
Upgrade graphs can be found at https://easyupload.io/rbhlsn (pw: unity)


Comments to reviewers

Kink3d and others added 30 commits April 15, 2020 11:12
…ph save (case 1230996) (#114)

* Follow references when unloading unneeded assets

* changelog

Co-authored-by: sebastienlagarde <sebastien@unity3d.com>
* Fix culling of reflection probes that change position

* Avoid null reference

* Avoid null reference - try 2

Co-authored-by: sebastienlagarde <sebastien@unity3d.com>
# Conflicts:
#	TestProjects/ShaderGraph/Assets/CommonAssets/Editor/EdgeTypeConversionTests.cs
#	com.unity.shadergraph/Editor/Data/Graphs/GraphData.cs
#	com.unity.shadergraph/Editor/Data/Nodes/AbstractMaterialNode.cs
#	com.unity.shadergraph/Editor/Data/Nodes/Input/PropertyNode.cs
#	com.unity.shadergraph/Editor/Data/Nodes/Math/Basic/MultiplyNode.cs
#	com.unity.shadergraph/Editor/Data/Nodes/Math/Matrix/MatrixSplitNode.cs
#	com.unity.shadergraph/Editor/Data/Nodes/Utility/CustomFunctionNode.cs
#	com.unity.shadergraph/Editor/Data/Nodes/Utility/KeywordNode.cs
#	com.unity.shadergraph/Editor/Data/Nodes/Utility/SubGraphNode.cs
#	com.unity.shadergraph/Editor/Data/SubGraph/SubGraphOutputNode.cs
#	com.unity.shadergraph/Editor/Drawing/MaterialGraphEditWindow.cs
#	com.unity.shadergraph/Editor/Drawing/PreviewManager.cs
#	com.unity.shadergraph/Editor/Drawing/SearchWindowProvider.cs
#	com.unity.shadergraph/Editor/Drawing/Views/GraphEditorView.cs
#	com.unity.shadergraph/Editor/Generation/Processors/Generator.cs
#	com.unity.shadergraph/Editor/Importers/ShaderGraphImporter.cs
#	com.unity.shadergraph/Editor/Importers/ShaderSubGraphImporter.cs
#	com.unity.shadergraph/Editor/Util/CopyPasteGraph.cs
#	com.unity.shadergraph/Editor/Util/MessageManager.cs
#	com.unity.shadergraph/Tests/Editor/UnitTests/MessageManagerTests.cs
* Redo branch. Upgrade to 2020.1.0b5, reduce texture res, new HDRP asset.

* Switch to HDRI, increase Garlic heap size to 4gb, turn on "Stop NaNs" on camera, decrease render target res.
…n the scene (#4)

* Set appropriate render datas before rendering cameras

* changelog

* update doc

* comment update

Co-authored-by: sebastienlagarde <sebastien@unity3d.com>
… has a clear coat (lit shader). (#18)

* Fixed an issue with the specularFGD term being used when the material has a clear coat (lit shader).

* update ssr screenshot

Co-authored-by: sebastienlagarde <sebastien@unity3d.com>
…refactor (#39)

* Added support for rasterized area light shadows in StackLit. Also refactor code to avoid early return (compiler bug) and fix lux meter debug mode when anisotropy for area lights is enabled.

* Doc: remove note about area light shadows for stacklit. [skipci]

Co-authored-by: sebastienlagarde <sebastien@unity3d.com>
Kink3d and others added 27 commits May 4, 2020 11:34
* WIP : converted to tracking most things by preview instead of node

* Fix for softlock when a non-compiling node exists on a graph

* WIP :  Fixed issues with Master Preview transform
Optimized property collection and timed node treatment

* Subgraph Previews working
Master node preview redraws when resized
Better early out of render code

* Moved preview mode computation to unified UpdateTopology()
Cleaned up code
@marctem
Copy link
Copy Markdown
Contributor

marctem commented May 13, 2020

This branch name was unwieldy and confusing and the PR owner has left the company, new PR from a renamed branch is at #451.

@marctem marctem closed this May 13, 2020
@sebastienlagarde sebastienlagarde deleted the sg/stack-master-v2-upgrade-hdrp branch September 1, 2021 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.