-
Notifications
You must be signed in to change notification settings - Fork 839
Generate a material as a subasset for ShaderGraphs #5795
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Hi! This comment will help you figure out which jobs to run before merging your PR. The suggestions are dynamic based on what files you have changed. Shader Graph Depending on the scope of your PR, you may need to run more jobs than what has been suggested. Please speak to your lead or a Graphics SDET (#devs-graphics-automation) if you are unsure. |
com.unity.shadergraph/Editor/Drawing/Controllers/ShaderInputViewController.cs
Outdated
Show resolved
Hide resolved
558d4c3
to
fe82d45
Compare
yes we should bump the importer version.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is happening if a users setup this on the fly material to an object and build. does it works in player? (i.e is the build step considering it is a true material and store it correctly in build data? Because shader graph don't exist at runtime.
It works fine, actually it's the same process as for the shader itself, which is an artifact generated by the shadergraph importer that is available at runtime |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
As we discussed, there might be an edge case if some script comes along and forces AssetDatabase to save the dynamic changes to the default material artifact. Not sure if that is even possible, but that should be an issue with the script if so.
@ellioman tagging you here although this will not directly impact URP, but for awareness |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UX approval
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ran some manual tests with the following exposed properties:
- float
- Texture 2D
- Color
Also ran tests to ensure naming conflicts don't cause issues with this auto generated material. No issues were encountered during these tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we have any what's new for shader graph?
* master: Add missing shader stages in shader keyword copying (#6008) [Yamato] Add all DX11 job for URP (#6075) [HDRP] Add more performance test coverage (#5814) Fix templates ISO date (#6073) Fix division by 0 when AO is 0 (#6078) Fixed grammar errors (#6077) [HDRP][Path Tracing] Improved robustness of the stacklit material (#6066) [APV] Cell streaming system (#5731) Update revision for URP update test project (#6061) [HDRP][Path Tracing] Camera ray misses now return a null value with Minimum Depth > 1 (#6067) Renable missing test (Lens Flare) (#5456) [HDRP][Path Tracing] Added proper support for interleaved tiling (#5953) Fix to render depth or depth/normal of waving grass (#4097) Remove ScreenSpaceShadowResolvePass (#6002) [HDRP][Docs] Update docs with RendererList related option (#6031) Layer drawer used in ray/path tracing now matches 100% with camera's. (#5956) Fix iridescence tooltip (#5950) [HDRP] Change RenderGraph Begin/Execute function pattern to avoid leaks (#5929) [HDRP] Fix errors when switching build targets in editor (#5918) Generate a material as a subasset for ShaderGraphs (#5795)
Purpose of this PR
This PR adds an automatically generated material subasset on ShaderGraphs:
I bumped the shadergraph importer version, so for existing projects every graph will be reimported and the material willl be generated.
Benefits:
Supported properties:
That's essentially all properties that can be exposed, I don't think I am missing one
This is part of this JIRA task https://unity3d.atlassian.net/browse/UR-1450
Testing status
See the gif above.
Tested undo/redo of shadergraph properties.
Tested closing shadergraph window with saving or discarding changes.
Tested that the generated material works on a mesh renderer in a standalone build.
The feature has also been tested by several artists in the context of material variants
I will add a graphic test.