Skip to content
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

Gradient Nodes #310

Merged
merged 14 commits into from
Apr 20, 2018
Merged

Gradient Nodes #310

merged 14 commits into from
Apr 20, 2018

Conversation

Kink3d
Copy link
Contributor

@Kink3d Kink3d commented Mar 29, 2018

Gradient Nodes

alt text

Description

This PR adds gradient functionality via two new nodes. Sample Gradient node samples a gradient given a Time parameter. This gradient can be defined on the Gradient slot control view. The Gradient Asset node defines a gradient that can be sampled by multiple Sample Gradient nodes using different Time parameters.

These nodes support all functionality of the C# gradient type, including 2-8 color keys, 2-8 alpha keys and a mode value (Blend or Fixed). Internally to the shader each gradient is defined as a struct, wherein its variables are hardcoded in the case of the final shader, or assigned to multiple shader properties in the preview shader, to facilitate editing gradients without recompilation.

Details

New Nodes

  • Gradient Asset
  • Sample Gradient

New Features

  • Gradient Material Slot
  • Gradient Input Material Slot (and Control)
  • Gradient Control
  • Gradient Shader Property
  • Gradient Slot Type (and concrete)
  • Gradient Property type
  • Gradient Slot Value
  • Add Gradient Type to Subgraph and CodeFunctionNode
  • Add Gradient Function to Functions.hlsl

Risk

  • Changed GetPreviewProperty to GetPreviewProperties. Now takes a list of PreviewProperty with ref keyword to collect properties. This adds support for multiple preview properties from a single slot. This is needed for Gradient type.

@Kink3d Kink3d self-assigned this Mar 29, 2018
@keijiro
Copy link
Contributor

keijiro commented Mar 30, 2018

  • I think you don't have to use ref in GetPreviewProperties. List<T> is a reference type, so you can modify the content without ref.
  • If we know that there are only up to eight keys in gradients, we can easily unroll the for-loops in Unity_SampleGradient. I think it can be implemented without dynamic branching (no ifs and fors).

Unroll loops
Remove control flow
Remove dynamic indexing
@Kink3d
Copy link
Contributor Author

Kink3d commented Apr 4, 2018

@keijiro Changes made, thanks :)

@keijiro
Copy link
Contributor

keijiro commented Apr 5, 2018

@Kink3d It looks good to me. Although I have some ideas to try for further micro-optimization (e.g. step() is discouraged to use in GCN-based GPUs), I think this is good enough for an initial implementation.

@pbbastian
Copy link
Contributor

"Gradient Asset" should just be "Gradient" as it doesn't actually represent an asset.

Copy link
Contributor

@pbbastian pbbastian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Getting these warnings:

Assets/ScriptableRenderPipeline/ShaderGraph/com.unity.shadergraph/Editor/Drawing/Controls/GradientControl.cs(48,28): warning CS0414: The private field `UnityEditor.ShaderGraph.Drawing.Controls.GradientControlView.m_SerializedProperty' is assigned but its value is never used

Assets/ScriptableRenderPipeline/ShaderGraph/com.unity.shadergraph/Editor/Drawing/Views/Slots/GradientSlotControlView.cs(24,28): warning CS0414: The private field `UnityEditor.ShaderGraph.Drawing.Slots.GradientSlotControlView.m_SerializedProperty' is assigned but its value is never used

@Kink3d
Copy link
Contributor Author

Kink3d commented Apr 19, 2018

@pbbastian fixed

@pbbastian pbbastian merged commit f760653 into master Apr 20, 2018
pbbastian added a commit that referenced this pull request May 3, 2018
@Kink3d Kink3d deleted the gradient-nodes branch May 9, 2018 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants