Search Unity

  1. New Unity Live Help updates. Check them out here!

    Dismiss Notice

Feedback Features I would want to see

Discussion in 'Shader Graph' started by bgolus, Apr 8, 2019.

  1. bgolus

    bgolus

    Joined:
    Dec 7, 2012
    Posts:
    8,334
    This is based off of using the LWRP version 5.10

    Show Generated Shader Code
    Older versions of Shader Graph had the option for looking at the generated shader code. This is incredibly useful for setting up basic shaders and then allowing advanced users to extend the capabilities of the shader outside of the confines of Shader Graph’s abilities. Adding a lot more functionality would help this, but for my usual custom shader use cases and the existing limitations of Shader Graph I don’t see myself ever using it for production work. I fully admit I’m an advanced shader user though.
    edit: it's still there! this wasn't showing for me due to some UI bug causing right click to do nothing (among other things).

    Instanced property support
    Currently the master nodes themselves appear to be setup for instancing, but there’s no way to set material properties themselves to be instanced or not.

    Make Branch node actually an be an if
    Fairly simple. Right now the Branch node is a bit of a lie. Since it uses a lerp, it offers none of the advantages an actual if could offer, and is more expensive than just using a straight Lerp node that takes a float in nearly 100% of use cases as it avoids unnecessary float to bool back to float conversions that most users would likely end up doing. A real "if" fed by a material property boolean is nearly as efficient as a shader variant on modern hardware, and will end up being translated into a lerp or fast comparison for older APIs anyway. No need to do anything fancy with the shader generator as a reasonably smart shader compiler will automatically gather up most of the shader code that's only used by a particular branch.

    Comments
    The node groups are nice, but they only offer a single layer of comments, and because the text is so large using them for proper comments isn’t viable. A comment node would just be a resizable box that shows text inside it. Having it attach to nodes like the group node does is a “nice to have”, but in my mind far less important.
    edit: It's coming!
    https://forum.unity.com/threads/documenting-your-shadergraph.657418/#post-4406023

    Direct control over render state
    ZWrite, ZTest, Cull, Blend, BlendOp, ColorMask, Stencil, etc. It’s nice to have these hidden and auto set by the “Surface”, "Blend" and "Two Sided" settings for basic shaders and new users, but for many effects you need far more control.

    Feedback for when nodes won’t connect
    Simple cases like trying to connect a float to predicate should call this conflict out with a tooltip of some kind on hover rather than just disabling the dot. More advanced cases like using a Sample Texture node to modify the position just leave users confused at the moment. Should tell the user why the nodes won’t connect, and offer a solution, like “use a Sample Texture LOD node instead”. A link to documentation with a more thorough explanation would be good too.

    Static Switches
    This is a feature of Unreal materials, roughly akin to #pragma shader_feature. Would love to see both [Toggle] and [KeywordEnum] functionality, but toggle alone would be worth a ton. Would make shader graphs infinitely more useful. No need to do anything fancy with the shader generation either, just wrap a single pass-through variable in a #if defined() and let the shader compiler will strip unused code.

    Multi-Pass Support
    You can already add multiple master nodes to a shader graph, though any node past the "oldest" doesn't actually do anything. I understand part of this is on SRPs not supporting multiple passes beyond one lit and one unlit pass, but even that combination doesn't work with Shader Graph. Why allow multiple master nodes at all if they don't do anything? At the very least show which master node is actually "active", and maybe add the option to switch which is the "master" master node. If multi-pass support is at some point expected, there needs to be an order property.

    Shader Folder Path
    This is more a UI issue than a missing feature. You can change the folder hierarchy for Shader Graphs, but it isn't obvious that the "Shader Graph" text on the blackboard is editable. For one, the text is greyed out, which for all other cases throughout the Unity UI means "not editable", and you cannot modify the more obvious shader name itself just above as that is pulled from the file name directly. It really needs some kind of feedback that it's editable, and that it's the path. Something like have it highlight on mouse over, have a / at the end of the path, or literal "Path: " text before it. Something more than faded out text.

    Pass Through Node
    A node that does explicitly nothing, and exists purely for line organization purposes. Some other node based tools allow you to add handles mid-line rather than going through a node, which is a much more visually and functionally clean solution for the end user. Relative low priority, but definitely a nice to have. Would likely be useful for the other node based tools Unity has (ie VFX Graph).

    Create Node > All
    I find it very hard to find the node I want in the current hierarchy. I kind of want an "All" option that just shows a single monolithic list of every node sorted by name, maybe showing the folder hierarchy. Sometimes I can find the node I want by typing in my guess for what you might call it, but a few nodes aren't named anything remotely similar to the HLSL code equivalents. This is actually a complaint I have about the documentation as well that I prefered on the previous wiki site as it used to have a "show all" option. I used that exclusively for finding nodes as it was way faster than going through the hierarchy for me. There's also a lot of useful nodes that are simply buried so far deep that I'd never know they existed if I hadn't looked at the full list.

    Sampler Count and Approximate Instruction Count Stats
    Super useful to know how many samplers are in use, and while instruction counts aren't a 100% accurate way of knowing how performant a shader is, it's better than node count (since some nodes actually add a lot of instructions) or nothing at all.

    Let us use the Node API
    For a while there was the Custom Node API. This was clearly not good enough as many basic features simply didn't work. The latest versions of Shader Graph appear to have completely deleted that API, which is good, but has also made the Node API used for the built in nodes be internal. Why not let us write nodes using the same API the internal nodes are written in instead of exposing a custom one? For people writing their own SRPs, I suspect many of us will just end up modifying that code directly to do that anyway, but until then this is quite an antagonistic approach for the asset store.

    Loops
    This one will likely require some thought and modifications to the shader generation. Maybe something like in-graph sub-graphs or a group with inputs / outputs for setup. It's a feature I rarely see in node based tools, but they have been done. Many shaders end up being absolutely gigantic simply because every iteration of a loop has to be manually flattened out. This can make simple blur shaders that would be 10 lines in a vertex fragment shader be mess of a few hundred nodes.
     
    Last edited: Apr 9, 2019
  2. alexandral_unity

    alexandral_unity

    Unity Technologies

    Joined:
    Jun 18, 2018
    Posts:
    145
    "Pass Through node" already exists, see the "preview" node from the creation menu. c:

    The rest I don't have direct immediate responses for, but we'll look into your feedback!
     
    bgolus likes this.
  3. Kink3d

    Kink3d

    Unity Technologies

    Joined:
    Nov 16, 2015
    Posts:
    45
    Woah, big list. Ill try to address as much as I can :p

    Show Generated Shader Code
    This is still there? Its only available on master nodes and nodes with previews though (same as previous behaviour)

    Instanced property support
    Shader Graph is currently set up to assume SRP batcher use. Agreed that support to use regular GPU instancing would be a win. Its in the backlog.

    Make Branch node actually an be an if
    Yes, this is an oversight. The if is actually in the comparison node, it should be moved around so you can do a branch on a constant bool to achieve compile time stripping. Known and re-noted.

    Direct control over render state
    Agreed, weve been back and forth on this a lot, but with upcoming changes to render state already planned for the near future, ill be pushing for some sort of "advanced mode" to handle these cases (although it does get complex very quickly, to which passes does the custom state apply? In HDRP for example, setting it on all passes will break everything. Different use cases want it on different passes, how do you handle that etc etc)

    Feedback for when nodes won’t connect
    This is some quality UX feedback, ill pass it on the the UX team.

    Static Switches
    Been in the backlog for a long time, just keep having higher priority stuff to do :(

    Multi-Pass Support
    Multiple master nodes is more of a bug/lack of UX than anything. It needs to be resolved somehow, but not sure how right now as we need to handle switching the master node case...

    Shader Folder Path
    Again, will pass on to the UX team. Good feedback.

    Create Node > All
    See above :D

    Sampler Count and Approximate Instruction Count Stats
    This is something we are investigating right now, although not really sure what the plan is yet. Likely we will be rolling out performance information in multiple stages, probably starting with a much broader indicator for newer users.

    Let us use the Node API
    This is a big topic, and frankly not one that I want to engage in to have it buried inside a larger thread. If you really want to start a discussion about why the API was removed I would open another thread, but boy howdy is that a complicated discussion :p

    Loops
    Something we've had at the back of our minds for a long time, simply just a big feature and not the highest priority. But yes if/when it does land expect it to be like a group node with in/out/iterations etc. For now at least you can write your complex loop code manually with Custom Function Node :)

    Hopefully that helps ^^
     
  4. Horus_Sungod42

    Horus_Sungod42

    Joined:
    Oct 30, 2014
    Posts:
    75
    Super +1 for the Static Switches and the Operation/Sample count. Though it makes sense that these features are finishing touches, allowing the users to optimize their shaders (right now, just getting everything to work must be quite the job!)
     
  5. rsodre

    rsodre

    Joined:
    May 9, 2012
    Posts:
    199
    Would be very nice to see the actual value, in numbers.
    I know most times it depends on varying data, but I often need this to debug my graph, we could change the input for a constant and then get a glimpse of what's passing thru.

    Having worked a lot with PureData and Max, I miss that a lot.
     
  6. bgolus

    bgolus

    Joined:
    Dec 7, 2012
    Posts:
    8,334
    Ah ha! It shows something now when I right click on the Master node! It wasn't for a while, though I was also seeing other bugs like the default inputs to nodes weren't going away when another node was attached, and there was a lot of UI related log errors. So it must of been a bug related to the UI just generally being fudged from switching back and forth between versions.

    Glad to know it's a known issue. However I disagree that the comparison node is an if, those are all single line fast conditionals, which are different. A shader compiler may make an if into an conditional, or a lerp, or a dynamic branch, and they all have completely different behaviors. However a shader compiler will not make an conditional or lerp into a dynamic branch. Also the comparison node only outputs 0 or 1 rather, requiring the user to use a lerp to actually pick between two node outputs which adds 2-3 more instructions rather than letting the conditional itself do that for "free".
    So, I guess, the next thing would be:

    Let the user override the output of the Comparison node
    Default to 0 and 1, but the have inputs for "True" and "False" so they can assign the possible outputs rather than requiring another node to pick between them.

    For a hypothetical case of an "advanced" mode that would let me modify these settings? I'd want to have control over every pass individually, as well as control over what each pass outputs (in terms of which nodes are used), or if they even have the pass at all. This could be handled with adding more outputs on the Master node, or "#if defined" nodes that take two or more inputs. So like a "Shadow Caster Switch" node might take two different alpha threshold values for example, one for when the pass is a shadow caster, and one for everything else.

    I can imagine. I suspect there's a camp that doesn't want them at all, and think Sub Graphs should be enough (presumably once more functionality is added). It's an admirable goal, but other engines have been at the node based material "game" for over a decade and people still write custom nodes because it's often easier, makes for more efficient code, and because there's still functionality missing that you often can't do with a node graph. Having the custom function node that takes a file as input is a nice touch, but not terribly user friendly for something like an asset store thing.
     
    Jick87 and GameDevCouple like this.
  7. recursive

    recursive

    Joined:
    Jul 12, 2012
    Posts:
    640
    I'll add my humble programmer/semi-tech-artist opinion:

    +1 on Instanced Property Support
    Some specialized effects/rendering setup basically rely on this if you can't/don't use VFX graph for things.

    +1 on Static Switches
    +1 on Direct control over render state

    ComputeBuffer / StructuredBuffer support
    I cannot seem to find a way to declare structured buffer access.

    Declare shader includes
    I can't seem to find a way to add non-sub-graph or non built-in function shader file includes, I'd like the ability to wrap additionally programmer-optimized hlsl functions or snippets and expose them via custom hlsl node.

    All of which tie into some kind of solution:

    Some kind of "Declaration Injection Node" or other mechanism for special declarations.
    This would tie into solve all of the above issues, the main possible pain points being:
    * Duplicate/overlapping declarations for functions and cgincludes
    * Priority of declared code injections.

    I could see shadergraphcginclude shadergraphdeclaration asset file types that describe the minimum need and references to them can be merged to avoid at least the duplication issue.

    This would simplify being able to use the current iteration of custom function nodes for non-standard behaviour. Throw on the ability to use/declare a custom UI class for CustomFunctionNode and the need to go back to the old way of custom function node can be rendered moot.

    This would also simplify things for programmers/tech artists doing custom setup for artists that's still "within pipeline".
     
  8. bgolus

    bgolus

    Joined:
    Dec 7, 2012
    Posts:
    8,334
    Yeah, I understand the want to keep all nodes useful on all platforms, but it seems like the big point of the HDRP or even the LWRP, was to not be bogged down by the platform limitations the built in rendering paths were built around.

    The Custom Function node lets you select a file, which does an #include for that file.
     
    recursive likes this.
  9. Kink3d

    Kink3d

    Unity Technologies

    Joined:
    Nov 16, 2015
    Posts:
    45
    Part of the intention of allowing HLSL include from file via Custom Function Node was to unlock a lot of this stuff. As this is just included raw HLSL you can declare whatever data containers, uniforms and functions in there without any real consideration for the graph (as long as at the highest level you can map the interaction back to a node).

    Ill freely admit the solution isnt perfect, but it is more flexible than many realise.

    As for the other points:
    • User control of the difference between passes is a super complicated issue, given RP agnostic node front ends and our already overblown master node UIs. We are very aware of the desire to do this, but there is a lot more at play here than you probably realise.
    • Thanks for bringing the fast conditionals to my attention, I had forgotten they are expressed like that, this is no good and I will change it. I'll likely add an If node that operates how you requested (combining the Comparison/Branch nodes) for cases where you don't want to string other boolean logic together to reduce instructions.
     
    Jick87, recursive and bgolus like this.
  10. 479813005

    479813005

    Joined:
    Mar 18, 2015
    Posts:
    44
    Snipaste_2019-04-10_10-31-05.jpg
    I want the path to appear above the file name, which is more intuitive.
     
    Jick87, GameDevCouple and bgolus like this.
  11. bgolus

    bgolus

    Joined:
    Dec 7, 2012
    Posts:
    8,334
    ph_, recursive and 479813005 like this.
  12. SonicDD

    SonicDD

    Joined:
    Feb 21, 2017
    Posts:
    3
    Default binding on SubGraph input-slots
    Not only setting constant standard values for the input-slots, but also bind something like UV0 to a vector2 slot as default (like it was possible in the CustomFunctionNode-Class).

    Setting preview-type for SubGraph
    Option to set preview-type (3D / 2D / none) for SubGraphs
     
  13. Arycama

    Arycama

    Joined:
    May 25, 2014
    Posts:
    107
    Good suggestions, static switching/multi-compiles/shader variants are pretty important for production, especially for mobile when you really don't want to be wasting any instructions on features that not everything is using.

    A couple more features I think would be helpful:

    Controlling the output type/dimension of a sub-graph
    By default, sub-graphs seem to output a Vector4. Even if your subgraph might only output a Vector2. This makes it less intuitive. Eg "Why is my custom UV-scrolling node outputting a Vector4 when the UV is only 2 components?"

    Compiling preview shaders becomes very slow very quickly
    This happens even when preview nodes are not visible. It should at least be asynchronous, so that it doesn't block the editor every time you make a change to a node. It must also be compiling a huge amount of variants to be that slow for a "Preview" shader.

    Control over whether a calculation is performed per-vertex or per-pixel
    Once again, quite important when targeting performance-constrained mobile devices, especially newer devices with very high pixel densities. This is one of the main reasons I choose vert-frag shaders over surface shaders at work currently. This functionality can be hidden in some kind of "Advanced" section, so it doesn't complicate the workflow for users who don't need this functionality.

    Applying masks/swizzles on inputs/outputs without extra nodes
    An example is texture nodes. They have a single RGBA output, and then four individual R, G, B and A outputs. What if you want RG, or RGB, or some other combination? This requires extra nodes, and also makes the Texture node much bigger than it needs to be. Being able to apply arbitary swizzles/masks, such as .xzyw, rgrg, xxx, etc would be handy and help reduce the size of the shader graph.

    upload_2019-4-21_12-15-40.png


    Not a question, but does anyone know why properties named Vector1/2/3/4 instead of Float 1/2/3/4, when the generated code is HLSL, and the Unity Mathematics library also uses Float1/2/3/4?

     
    einWikinger, amisner2k and dadude123 like this.
  14. amisner2k

    amisner2k

    Joined:
    Jan 9, 2017
    Posts:
    30
    +1 to this. I noticed this oddity myself and have since managed to gloss over and mentally ignore it but it's still wrong and should be rectified. "Vector 1/2/3/4" should be "Float 1/2/3/4".
     
  15. SKoptev

    SKoptev

    Joined:
    Jan 15, 2019
    Posts:
    36
    One more wish: grouping of properties. You need to be able to split a large number of properties in a complex material into logical groups (just look on HDRP/Lit interface).
     
    yigitkahramn and amisner2k like this.
  16. lostminds

    lostminds

    Joined:
    Jan 17, 2019
    Posts:
    43
    Regarding multiple output nodes (or passes) one case I've run into is trying to make two sided and transparent materials ( https://forum.unity.com/posts/5210591/ ). Currently the master node lets you set "Two sided" or not (which is just front faces). And to get backsides only you have to use logic with the IsFrontFace node. It would be much more convenient if you could just specify the shader was for backsides as well (Change "Two sided" bool to a three-way setting Front, Back or Both). And ideally let you make a shader with one master node (or pass) for backsides and one for front sides.
     
  17. Reanimate_L

    Reanimate_L

    Joined:
    Oct 10, 2009
    Posts:
    2,395
    ShortCut/LocalVariable Node
    Something similar like this http://wiki.amplify.pt/index.php?title=Unity_Products:Amplify_Shader_Editor/Register_Local_Var
    this could help to clean the graph from messy spaghetti node connection

    Per Channel Output
    Add Per channel output just like Sampler Texture 2D Node to every node with multiple channel output, this could make the graph efficient and avoid using split node a lot.
    ie vector2 node have additional R and G output slot, vector3 node have additional R,G,B output slot
    upload_2020-1-23_16-35-7.png
     
    Last edited: Jan 23, 2020
  18. einWikinger

    einWikinger

    Joined:
    Jul 30, 2013
    Posts:
    39
    Control over whether a calculation is performed per-vertex or per-pixel
    I also very much want to see this functionality. No being able to declare custom interpolators where values are computed per-vertex and then interpolated per-fragment really kills the more advanced shaders. I don't want to recompute a complex transformed vector for every fragment when I could just compute it per vertex and interpolate it across the primitive.
     
  19. bgolus

    bgolus

    Joined:
    Dec 7, 2012
    Posts:
    8,334
    FYI, it's probably faster to do it in the fragment shader than it is to transfer it on anything but 3+ year old low end mobile. Certainly the difference between a float3 value being interpolated and doing a matrix multiplication from common data, the later wins.
     
  20. alexanderameye

    alexanderameye

    Joined:
    Nov 27, 2013
    Posts:
    1,032
    Register local var would clean up my graph so much. +1!!
     
unityunity