godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.07k stars 69 forks source link

Modify the built-in list of VisualShader nodes #2791

Open jcandres opened 3 years ago

jcandres commented 3 years ago

Describe the project you are working on

VFX demo reel

Describe the problem or limitation you are having in your project

VisualShader comes with a wealth of nodes, but I believe a few missing and some get in the way and could be cut. I'd like to propose a few changes that I've found while working, some additions and some removals.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Inclusions

Exclusions:

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

See above.

If this enhancement will not be used often, can it be worked around with a few lines of script?

I've tried to limit this proposal to nodes and features that already exist, but could use improvement. The two UV manipulation nodes could be scripted or part of a library but as explained there's no readily available solution and even if the code is simple, implementing them it's not a trivial task for most users.

Is there a reason why this should be core and not an add-on in the asset library?

See above.

Chaosus commented 3 years ago

Add an INSTANCE_CUSTOM input node, vec4 and accessible both from fragment and vertex. This is already in the engine and used by Particles and MultiMesh, but it's not exposed to fragment shaders and not available as a VisualShader node. Currently accessing it involves messy workarounds: the way I've been using it is reading it in vertex using a expression node that passes INSTANCE_CUSTOM.xyz to... UV2, to be used in fragment. This is obviously a bad way to do it and needs an alternative. This feature would be useful to anybody working with particles, since syncing to particle lifetime is pretty much mandatory.

Yeah, there are plenty of new built-ins introduced to 4.0 which are not yet added to Visual shaders. I've recently filled a Godot documentation with them (for example https://docs.godotengine.org/en/latest/tutorials/shaders/shader_reference/spatial_shader.html). I planned to add them, prior to the release of course (or even before alpha).

Add a Billboard node, or a MODELVIEW_MATRIX manipulation node. Billboards are built-in in a StandardMaterial, but there's no clean way to do this in a VisualShader. Implmenting this in VisualShader is messy: have to code in a expression node, give it an unused output and wire it into... something. I've been using UV2 since I use it rarely, but this is a hack for something that's very commonl used and already part of the engine. If this can't be implemented as a node (I believe it would also have to be added to the list of outputs), then it could be added to the inspector under the VisualShader properties, similar to how it works in a SpatialMaterial.

Ah, yes there was even an old issue about it: https://github.com/godotengine/godot/issues/16561. I forget about it but thanks for the reminder.

Add two UV manipulation nodes. UV panning (offset over time) and UV scaling (around pivot point). @Chaosus mentioned that this are too high level and would become engine bloat. In my opinion these are a must-have: most games need texture movement, and to achieve it these are the first two nodes you'll be adding to your shaders. This includes any shader with scrolling UV like fire, smoke, electricity, trails and ribbons (uv displacement along mesh). Creating them from scratch using nodes is tedious and takes lots of visual space for a simple task. While it's true that they can be created as a custom node, it's not trivial. @Chaosus linked two existing libraries for Visualshader nodes: Maujoe/Godot-Visual-Shader-Node-Library Is higher level and lacks these nodes. arkology/ShaderV Has a bugged implementation. (I can provide code for my own implementation, but I'm not a programmer and it would have to be reviewed/tested).

Ok, if you think that they are important (I don't know) - I could implement them to Godot. Provide a code below, please.

Remove the VisualShaderNodeColorOp nodes (Screen, Softlight, Burn...). They are very specific color operations that seem to mimic Photoshop blend modes. A few of them are redundant (Darken and Lighten are Min and Max, renamed), others are easy to replicate using existing nodes. In my experience (daily Photoshop usage for 10+ years) you can replace most blend modes with basic math operations and a strength variable to finetune the outcome. Also, they are poorly named. "Screen" and "Lighten" make sense in a graphical app, but here they are competing with more relevant nodes:

It was originally be implemented by @reduz, not sure that it is a good idea. I think you should create a separate proposal to see how other users look at this idea.

I'm unsure about MultiplyAdd in Godot 4. Is there a reason why this is a node? Seems oddly specific to me, and I haven't found a use for it so far.

It was a part of the implementation of fma function from GLSL4, but yeah kinda redundant.

In Godot 4 there's a wealth of uniforms and consts since they are now typed. Right now the list is long and involves much scrolling until you find the right input and type. Usability would improve if they were bundled into a single node for easy access, like in Godot 3. For example, a generic "input" node can default to float (most used type) but allow to select vector, int, or bool from a dropdown list, and set to uniform/const with a checkbox. Not sure how feasible is this, but I wanted to propose it in case it's easy to implement.`

So you want some kind of bloated construction node? But the key of VisualShader's is simplicity. Why you need it if you can get anything from the 'Add Node' menu?

Calinou commented 3 years ago

I'm unsure about MultiplyAdd in Godot 4. Is there a reason why this is a node? Seems oddly specific to me, and I haven't found a use for it so far.

Using fma() is more optimized than doing multiply/add calls separately, unless the shader compiler manages to optimize this out somehow (it's not guaranteed on all platforms). When you use it consistently, you can get real-world GPU time savings up to 10%.

Chaosus commented 3 years ago

Add a Billboard node, or a MODELVIEW_MATRIX manipulation node.

It will be fixed in https://github.com/godotengine/godot/pull/49157

jcandres commented 3 years ago

@Chaosus Thank you for the hard work.

So you want some kind of bloated construction node? But the key of VisualShader's is simplicity. Why you need it if you can get anything from the 'Add Node' menu?

My point here is that the node list feels a bit bloated with all the typed uniform and consts. But you're right, that would overcomplicate the actual node.

Perhaps a better idea would be to have a single node for each typed input (Float, Int, Bool...) and in the actual node a checkbox to make it uniform or const. The benefits from the user's point of view would be that it's easier to find the right node because they are half of them, and you can decide what properties are exposed as uniforms without having to create and rewire a replacement node.

jcandres commented 3 years ago

@Chaosus Here's the code the the UV panning and scaling nodes, for Godot 3.x https://gist.github.com/jcandres/c8e9361aa6b0c09f1d3f293121d86cb6 I'm not sure if UVPanner is the best name for the node, I just named it following Unreal's convention. In Unity this node is called "Tilling and Offset" which is even worse. Feel free to change it if you find a better name.