NPR: Shader Node Traits #130192

Miguel Pozo · 2024-11-12T20:22:45+01:00

Miguel Pozo commented

2024-11-12 20:22:45 +01:00

The NPR branch needs a way to tell whether the node tree is a group or an NPR tree for correct polling in the UI.
This follows a similar approach to GeometryNodeAssetTraits.

There's still the problem that any node group can still be added to any shader tree type (for example, it's possible to add a Material node group to a World node tree).
This trait system could eventually be used to detect which node groups have tree-type-specific nodes and allow adding them only to compatible trees.

The NPR branch needs a way to tell whether the node tree is a group or an NPR tree for correct polling in the UI. This follows a similar approach to `GeometryNodeAssetTraits`. There's still the problem that any node group can still be added to any shader tree type (for example, it's possible to add a Material node group to a World node tree). This trait system could eventually be used to detect which node groups have tree-type-specific nodes and allow adding them only to compatible trees.

Miguel Pozo added 4 commits 2024-11-12 20:22:57 +01:00

ShaderNodeTraits 8818ed4dd2

poll_group a147dcc622

versioning 1267f0f322

is_group 8e0b459f1b

Iliya Katushenock commented

2024-11-12 20:26:39 +01:00

It is too bad that instead of just use bNodeTreeType to different kinds of node trees in general there is another one kind of tree typing just to different shader node trees...

It is too bad that instead of just use `bNodeTreeType` to different kinds of node trees in general there is another one kind of tree typing just to different shader node trees...

Iliya Katushenock added this to the Nodes & Physics project 2024-11-12 20:27:01 +01:00

Miguel Pozo commented

2024-11-12 20:27:26 +01:00

For now this is an npr-prototype-only thing, but I would appreciate a quick look from @JacquesLucke or @HooglyBoogly, to ensure I'm not doing anything terrible.

For now this is an `npr-prototype`-only thing, but I would appreciate a quick look from @JacquesLucke or @HooglyBoogly, to ensure I'm not doing anything terrible.

Miguel Pozo removed this from the Nodes & Physics project 2024-11-12 20:28:06 +01:00

Miguel Pozo commented

2024-11-12 20:32:47 +01:00

It is too bad that instead of just use bNodeTreeType to different kinds of node trees in general there is another one kind of tree typing just to different shader node trees...

Geometry nodes work the same way. 🤔
We still need a way to share shader node groups between different shader tree types, so I'm not sure different bNodeTreeTypes would make things better.

In any case that would require a huge refactor that is out of the goals of the NPR prototype.

> It is too bad that instead of just use bNodeTreeType to different kinds of node trees in general there is another one kind of tree typing just to different shader node trees... Geometry nodes work the same way. 🤔 We still need a way to share shader node groups between different shader tree types, so I'm not sure different `bNodeTreeType`s would make things better. In any case that would require a huge refactor that is out of the goals of the NPR prototype.

Iliya Katushenock commented

2024-11-12 20:43:54 +01:00

Just to make sure, nodes use only bNodeType::poll to check if node group/separate built-in node can be placed in some node tree or not. Assets traits (geometry_node_asset_traits) is the way to say if geometry node tree can be runned as modifier/tool/operator/... . So this is like RenderTime/BakeTime shader difference. This mainly used by drag and drop and asset browser rahter then node editor.

Just to make sure, nodes use only `bNodeType::poll` to check if node group/separate built-in node can be placed in some node tree or not. Assets traits (`geometry_node_asset_traits`) is the way to say if geometry node tree can be runned as modifier/tool/operator/... . So this is like `RenderTime`/`BakeTime` shader difference. This mainly used by drag and drop and asset browser rahter then node editor.

Jacques Lucke reviewed 2024-11-12 23:17:13 +01:00

Jacques Lucke left a comment

Overall, it seems like this approach could work, but it's not obvious to me that this is the right way to go yet.
I don't have a fully formed opinion on it yet, sry.

It would be quite nice to reduce the different tree types we have instead of creating more different kinds, but we don't know how to do that exactly yet.

Overall, it seems like this approach could work, but it's not obvious to me that this is the right way to go yet. I don't have a fully formed opinion on it yet, sry. It would be quite nice to reduce the different tree types we have instead of creating more different kinds, but we don't know how to do that exactly yet.

👍 1

source/blender/blenloader/intern/versioning_400.cc

						
				@ -1259,6 +1259,20 @@ void do_versions_after_linking_400(FileData *fd, Main *bmain)

				    version_legacy_actions_to_layered(bmain);

				  }

				  if (!MAIN_VERSION_FILE_ATLEAST(bmain, 404, 6)) {

Jacques Lucke commented

2024-11-12 23:13:55 +01:00

Seems like the versioning code should also initialize the type for all the node trees embedded in materials, world, etc?

Miguel Pozo commented

2024-11-13 16:36:16 +01:00

This already happens in the node_tree_set_type call from ntree_blend_read_after_liblink, which runs before do_versions_after_linking.

This already happens in the `node_tree_set_type` call from `ntree_blend_read_after_liblink`, which runs before `do_versions_after_linking`.

JacquesLucke marked this conversation as resolved

Miguel Pozo commented

2024-11-13 16:35:54 +01:00

Overall, it seems like this approach could work, but it's not obvious to me that this is the right way to go yet.
I don't have a fully formed opinion on it yet, sry.

Ok, that's fine, this doesn't have to be the final design.
I mainly wanted to know if you saw something that could explode badly.
We still have plenty of time until we start looking into merging NPR features into main.

It would be quite nice to reduce the different tree types we have instead of creating more different kinds, but we don't know how to do that exactly yet.

As long as we have different contexts where some nodes are supported and others aren't we need some kind of classification.
At best we can "implement" them as no-ops so at least shaders compile, but there's no meaningful way to support the Principled BSDF in the World shader, for example.

>Overall, it seems like this approach could work, but it's not obvious to me that this is the right way to go yet. >I don't have a fully formed opinion on it yet, sry. Ok, that's fine, this doesn't have to be the final design. I mainly wanted to know if you saw something that could explode badly. We still have plenty of time until we start looking into merging NPR features into main. >It would be quite nice to reduce the different tree types we have instead of creating more different kinds, but we don't know how to do that exactly yet. As long as we have different contexts where some nodes are supported and others aren't we need some kind of classification. At best we can "implement" them as no-ops so at least shaders compile, but there's no meaningful way to support the Principled BSDF in the World shader, for example.

Iliya Katushenock commented

2024-11-13 16:49:46 +01:00

It would be quite nice to reduce the different tree types we have instead of creating more different kinds, but we don't know how to do that exactly yet.

As long as we have different contexts where some nodes are supported and others aren't we need some kind of classification.
At best we can "implement" them as no-ops so at least shaders compile, but there's no meaningful way to support the Principled BSDF in the World shader, for example.

I mean you can do the same thing in flat list of types and in type system where each type have some its internal property, only used by it,, it is only about generalization of the code to maintain\

> >It would be quite nice to reduce the different tree types we have instead of creating more different kinds, but we don't know how to do that exactly yet. > As long as we have different contexts where some nodes are supported and others aren't we need some kind of classification. > At best we can "implement" them as no-ops so at least shaders compile, but there's no meaningful way to support the Principled BSDF in the World shader, for example. I mean you can do the same thing in flat list of types and in type system where each type have some its internal property, only used by it,, it is only about generalization of the code to maintain\

Miguel Pozo merged commit 30a526a183 into npr-prototype

2024-11-14 15:45:56 +01:00

Miguel Pozo referenced this issue from a commit

2024-11-14 15:45:57 +01:00

NPR: Shader Node Traits

Miguel Pozo deleted branch npr-shader-node-traits

2024-11-14 15:46:00 +01:00

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

NPR: Shader Node Traits #130192