Malt
Malt copied to clipboard
bnpr
Rework Node UX
This branch aims to improve the Nodes user experience with the help of @Kolupsy.
The goal is to make them work closer to the built-in Blender nodes and the MaltShadingEssentials plugin, while maintaining the convenience of declaring nodes from pure GLSL functions.
In the cases where auto-generation from GLSL is not enough, BlenderMalt specific nodes will be implemented.
Feedback is welcome.
Tasks:
- [x] Support META GLOBAL properties (Meta properties that apply to all the functions/structs of their file).
- [x] Support node categories instead of generating them from the GLSL path.
- [x] Support subcategories for generating nodes with a drop-down selector (like the Mix or the Math nodes in EEVEE).
- [x] Support label metaproperties (ie. Custom names for nodes and sockets in the UI).
- [x] Support enum properties.
- [x] Support min and max metaproperties.
- [x] Support Slider subtype.
- [ ] Improve the design of the built-in nodes.
- [x] Allow access to internal nodes for advanced use cases.
- [x] Support shift+a search functionality for subcategory items.
- [x] Find a way to allow plugins and custom libraries for specific node tree to declare nodes in the same categories as the buil-in ones.
- [x] Update the generated nodes documentation to reflect the new metaproperties.
- [ ] Use the default Blender socket colors where possible.
Known Issues:
- ~~Custom libraries for specific node trees are not yet supported.~~ (Node Tree Local Libraries are deprecated)
Find a way to allow plugins and custom libraries for specific node tree to declare nodes in the same categories as the buil-in ones.
Here is my idea for that: either constantly unregistering/registering the categories or creating a global pool of all node items accumulated from Malt itself and plugins and replacing the draw function of the categories when that pool updates. This is how I add node items to blenders default node categories.
When replacing the draw function you lose the search functionality, right?
Seems like the items parameter can be a callback that receives the current context, so that can be a good option.
When replacing the draw function you lose the search functionality, right?
You can only search the node items of a node cateogory that returns True when polling. Specifically NodeCategory.items( ) (sorry items is what I meant before, but accidentally typed draw)
So replacing the items function to return a generator with your own node items makes those items searchable. Also NodeItems AND subclasses count towards this so creating NodeItem subclasses with icons for example is possible too
Support subcategories for generating nodes with a drop-down selector (like the Mix or the Math nodes in EEVEE).
This is checked but for me this has no effect. The hash functions for example are still all separate and have no dropdown menu. Is this still in progress or was it supposed to work already?
It works, check Node Utils > Float, for example.
Those multiple hashes is because this new UI is not tracking function overloads yet (the old did).
I found an issue with the subcategories. When the enum updates you need to recompile the graph for the change to take effect (an issue that pops up a lot in MaltSE). I cant commit the change though because I need to request access for GitKraken to push to the repo (request now pending).
Support shift+a search functionality for subcategory items.
Not sure if we can do this, as mentioned before, the search function lists all node items in categories that can be polled. So I think the only way to make subcategory items be their own search items you would need to create and display a 'Misc' node category that contains all the subcategory items as their own items.
I've added support for enum properties. There's not any example in the branch, but this is how it can be used:
/*
META
@color: subtype=ENUM(Red,Green,Blue); default=1;
*/
vec3 test_enum(int color)
{
switch (color)
{
case 0: return vec3(1,0,0);
case 1: return vec3(0,1,0);
case 2: return vec3(0,0,1);
}
return vec3(0);
}
There are probably some things left to fix and tweak, but I think all the major features needed for the new API are already there.
for the min/max parameters. The 'bpy.types.UILayout.prop' function has a 'slider' keyword argument that we can use to display the parameters with both min and max as sliders. But I dont see any way to retrieve the malt subtype of a function input to reliably set that up. Where does the subtype go when its read by the parser? That part of malt is still pretty much a blackbox to me.
All meta properties are inside a "meta" dictionary in their function/parameter.
If you add
import pprint
pprint.pprint(functions)
to the preload_menus function in MaltNodeTree.py you can see the whole info.
This is how a single function looks like:
'Node Utils - vec3 - vec3_mix_float': {'file': 'Node Utils/vec3.glsl',
'meta': {'category': 'Math',
'label': 'Vec3 Mix Float',
'subcategory': 'Vector 3D'},
'name': 'vec3_mix_float',
'parameters': [{'io': 'in',
'meta': {'label': 'A',
'subtype': 'Vector'},
'name': 'a',
'size': 0,
'type': 'vec3'},
{'io': 'in',
'meta': {'label': 'B',
'subtype': 'Vector'},
'name': 'b',
'size': 0,
'type': 'vec3'},
{'io': 'in',
'meta': {'label': 'Factor'},
'name': 'factor',
'size': 0,
'type': 'float'}],
'signature': 'vec3 vec3_mix_float(vec3 '
'a, vec3 b, float factor)',
'type': 'vec3'},
The GLSL parameters are converter to MaltParameters inside MaltNode.setup_sockets: https://github.com/bnpr/Malt/blob/ceb2fda47da642c61f6f390eb546d06e579b5187/BlenderMalt/MaltNodes/MaltNode.py#L132
And then sent to the setup function of MaltPropertyGroup: https://github.com/bnpr/Malt/blob/ceb2fda47da642c61f6f390eb546d06e579b5187/BlenderMalt/MaltProperties.py#L114
In the case of the subtype, it always gets stored as part of the MaltPropertyGroup rna dictionary: https://github.com/bnpr/Malt/blob/ceb2fda47da642c61f6f390eb546d06e579b5187/BlenderMalt/MaltProperties.py#L267
So the info was already there. It just had to be retrieved from the draw function: https://github.com/bnpr/Malt/blob/ceb2fda47da642c61f6f390eb546d06e579b5187/BlenderMalt/MaltProperties.py#L579
/*
META
@factor: subtype=Slider; default=0.5; min=0.0; max=1.0;
*/
vec3 test_slider(float factor, vec3 a, vec3 b)
{
return mix(a,b, factor);
}
oooh. You know what? I always thought that these two methods of getting custom property meta data were the same:
But as you can see they are clearly not. I was always using the lower one in my tests. Maybe should have looked at the code instead of goin data mining in blender 😅
I've added a new Input file in Node Utils that implements new nodes more in line with the Cycles/EEVEE/ShadingEssentials style. What do you think?
seems like we ended up working on the same thing now 😅 I was gonna wait for the poll to end but I guess we can estimate the result of it? What I made is basically the same, I do have more outputs on the geometry node though. Also putting those in a separate file is smart. I did not do that. I will push my version then we can compare...
https://github.com/bnpr/Malt/pull/348/commits/7d757e9d505ab1aae9885cec5e8f2432c582f4b8 is a fix for the min and max meta properties. Before, unless both min AND max were set there wouldnt be any clamping. now either of them can be set and it still works
could you please review this commit: https://github.com/bnpr/Malt/pull/348/commits/294188bba923a28a3c8093ee641e27a98a5edfa8
The reason is that thanks to blender ~~awesome~~ design, the id_properties_ui.update function takes different amounts of arguments depending on the current value of the property. There would be an error if you attempted to set soft_min and soft_max if the current property values was a str value. I fixed that in the commit but I am not sure if thats the proper way to do it or if you would even be allowed to have a situation like that
7d757e9 is a fix for the min and max meta properties. Before, unless both min AND max were set there wouldnt be any clamping. now either of them can be set and it still works
Looks good, the code is nicer than before. I wonder why did you decided to use soft min/max, though?
could you please review this commit: https://github.com/bnpr/Malt/commit/294188bba923a28a3c8093ee641e27a98a5edfa8 I fixed that in the commit but I am not sure if thats the proper way to do it or if you would even be allowed to have a situation like that
I guess it would be possible if the default value is something like a function call, and the meta min/max is still set, which would be kind of weird, but it certainly doesn't hurt to check for it.
you set the entire file as internal. What about matcap_uv and 'hdri_uv`? do we not want to have those available as vector nodes? or was that an accidental side effect
I've added an HDRI and Matcap node to Texturing (that also outputs the UV). I think that should be enough?
Looks good, the code is nicer than before. I wonder why did you decided to use soft min/max, though?
thats what you used before. soft_min and soft_max is nice because it allows for the slider UI and natural clamping of colors but also allows you do go beyond the 'recommended' range even if it doesnt make sense in some cases? do you want to differentiate between min/max and soft_min/soft_max?
I guess it would be possible if the default value is something like a function call, and the meta min/max is still set, which would be kind of weird, but it certainly doesn't hurt to check for it.
In the case where I noticed the error I had a vec3 that auto-defaults to the Color malt_subtype and therefore tried to set soft_min and soft_max
I've added an HDRI and Matcap node to Texturing (that also outputs the UV). I think that should be enough?
I see a usecase for matcap UVs if you want to do procedural texturing using that mapping (eg procedural matcap) and HDRI UV I think is useful if you wanna use an image filter like blur but in combination with HDRI mapping.
thats what you used before. soft_min and soft_max is nice because it allows for the slider UI and natural clamping of colors but also allows you do go beyond the 'recommended' range even if it doesnt make sense in some cases? do you want to differentiate between min/max and soft_min/soft_max?
Ooops, I would have sworn it used min/max before for non colors. I guess that how much sense makes to go beyond the set range depends on the use case. In any case, it can't be enforced since you always can plug any value into the socket. Do you think it would be worth to support both min/max and soft_min/soft_max ?
In the case where I noticed the error I had a vec3 that auto-defaults to the Color malt_subtype and therefore tried to set soft_min and soft_max
I see, I didn't think about that one.
I see a usecase for matcap UVs if you want to do procedural texturing using that mapping (eg procedural matcap) and HDRI UV I think is useful if you wanna use an image filter like blur but in combination with HDRI mapping.
Yes, it has its uses and that why I added them to the Matcap and HDRI nodes. But I think the use case is niche enough that shouldn't have its own node. Also, that way, if a user searches for "Matcap" or "HDRI" they don't have to choose between multiple options.
Do you think it would be worth to support both min/max and soft_min/soft_max ?
I would prefer to just use soft_min and soft_max. As you said, since input values can ignore these limits anyway it means that actual clamping has to be done in the shader code anyway.
But I think the use case is niche enough that shouldn't have its own node.
Fair enough.
I just noticed that you made some changes to the nodes when merging from the node-ux---category-cleanup branch to this one. How do you use the uint data type? Is that the one replacing the uvec from before?
Also it seems that pixel_depth, pixel_world_size, MODEL, and the entire 'Camera' node are not exposed anymore. Do you think those should be replaced by other nodes? Because I do think that their outputs are useful in general.
416199447c5fe644fa40ddf67f59f6937563ff2a
I would prefer to just use soft_min and soft_max.
👍
I just noticed that you made some changes to the nodes when merging from the node-ux---category-cleanup branch to this one.
Yes, it was not intended as a final decision on the API, but it was needed to make the merge since there was a lot of duplication.
How do you use the uint data type? Is that the one replacing the uvec from before?
I guess you mean the Object ID? It' just the first channel of the ID, which is the only one that it's set by default and it's unique for every object. Maybe we should avoid using unsigned integers in the API for simplicity and convert them to signed integers?
Also it seems that pixel_depth, pixel_world_size, MODEL, and the entire 'Camera' node are not exposed anymore. Do you think those should be replaced by other nodes? Because I do think that their outputs are useful in general. https://github.com/bnpr/Malt/commit/416199447c5fe644fa40ddf67f59f6937563ff2a
Yes, I think some Input nodes are going to need more design iteration. I find the Blender design for some of those somewhat inconsistent and the fact that we need to extend them makes things even worse, so I would like to establish a clear "domain" and rules for each of those before going further. For example, I think pixel_depth would make more sense in Camera rather than Render, but it may make even more sense to simply add a drop-down to Geometry to select the coordinate space (Object, World, Camera, Screen) of the outputs. I should have consulted you on those before making the merge, but I wanted to do it ASAP before there was more overlap between the two branches.
I guess you mean the Object ID? It' just the first channel of the ID,
yeah that one. So its a kind of integer meaning the reason why it shows up as white is because its just a very large number?
Maybe we should avoid using unsigned integers in the API for simplicity and convert them to signed integers?
i am not familiar with potential benifits or drawbacks of using uints vs ints. But I guess if you could also replace this with an int then that would be better because we wouldnt introduce a new data type
Yes, I think some Input nodes are going to need more design iteration
Ok that sounds good. I think a camera node could be useful to store information such as the pixel depth, yeah.
I should have consulted you on those before making the merge, but I wanted to do it ASAP before there was more overlap between the two branches.
Ok I see. That makes sense. I wasnt sure why you merged it the way you merged it.
it may make even more sense to simply add a drop-down to Geometry to select the coordinate space (Object, World, Camera, Screen) of the outputs.
I can see and appreciate the logic behind it but I think that in the case of the basic inputs we should probably stick to blenders design closely. Instead of a dropdown for world/object/camera/screen we would have Geometry for the world space stuff, Object (Info) for object space stuff and other object-related data like location and ID, and Camera for both camera space and screen space stuff. I think it could make sense to merge these unless we plan on exposing a ton of outputs on them (which I dont see us doing yet 🤔 )
i am not familiar with potential benifits or drawbacks of using uints vs ints. But I guess if you could also replace this with an int then that would be better because we wouldnt introduce a new data type
A uint is just an int without the sign. So a (32 bit) int can represent any value from -(2³¹-1) to +(2³¹-1), while the unsigned range goes from 0 to (2³²-1).
From a programming pov, it feels kind of weird to use negative numbers for IDs, but it may be worth making the change. Another (user-friendlier?) option may be simply exposing IDs as 8bit/channel RGBA values.
Instead of a dropdown for world/object/camera/screen we would have Geometry for the world space stuff, Object (Info) for object space stuff and other object-related data like location and ID, and Camera for both camera space and screen space stuff. I think it could make sense to merge these unless we plan on exposing a ton of outputs on them (which I dont see us doing yet 🤔 )
But the Object Info in Blender doesn't actually have any Object-Space coordinate. (?) My idea was not to replace the Camera Data node with a drop-down option, just to allow retrieving geometry data in any coordinate space.
In any case, I'm ok with sticking to the way Blender does things as long as we have a clear definition of what kind of info each node is supposed to expose, so if we need to extend them we can do it in a consistent way.
I was thinking about something along these lines:
Object Info (Per object properties)
- Position
- Rotation
- Scale
- Matrix
- ID
Camera Info (Per camera properties, and data computed from camera-space coordinates)
- Camera Position
- View Direction
- Z Depth
- View Distance
- Is Orthographic
- Camera Matrix
- Projection Matrix
Geometry (Plain geometry data, in any coordinate space) Input:
- Coordinate Space Enum (Object, World, Camera, Screen)
Output:
- Position
- Normal
- True Normal
Geometry Data (Data computed from Geometry. Needs a better name) Input:
- Position
- Normal
- IOR
Output:
- Incoming
- Is Backfacing
- Facing
- Fresnel
- Reflection
- Refraction
Maybe Geometry and Geometry Data could be merged into 1 node. Object info could also hold data computed from object-space coordinates, like the "Generated" coordinates.
Just an idea. I'm open to anything else as long as we can define consistent rules for them.
Just an idea. I'm open to anything else as long as we can define consistent rules for them.
I like it. I would definitely merge the Geometry and Geometry Data node though. I think its ok to have 9 outputs on a node.
you can set that up if you want. I could do it too but lets not make the exact same changes simultaneously :D
From a programming pov, it feels kind of weird to use negative numbers for IDs, but it may be worth making the change. Another (user-friendlier?) option may be simply exposing IDs as 8bit/channel RGBA values.
Does the ID thats exposed to the shader have to be related to the ID that objects have represented in the render pipeline( Malt.Scene.Object). If not then I dont see any benefit from output uint instead of int.
Ok, I'll do the changes. We can revisit them later if needed.
Does the ID thats exposed to the shader have to be related to the ID that objects have represented in the render pipeline( Malt.Scene.Object). If not then I dont see any benefit from output uint instead of int.
Yes, it's the same value. I think I should try the idea of exposing them in nodes as colors instead.