Improve a Cycles OSL nodes interface using a parameter metadata stored into the .oso file. This first patch improve how the 'booleans' options are draw in the node and also add the option to draw a 'pretty' name for each option in the Node. If this info is not present in the medatada, the parameter name is used instead.
Details
Details
- Reviewers
Brecht Van Lommel (brecht) - Maniphest Tasks
- T89741: Improve Cycles OSL nodes interface
- Commits
- rBe6bbbd965a44: Cycles: OSL metadata support for UI labels and checkboxes
Diff Detail
Diff Detail
Event Timeline
Comment Actions
This will need changes in blender_shader.cpp for Cycles to be able to convert these inputs to int sockets correctly.
| intern/cycles/blender/blender_python.cpp | ||
|---|---|---|
| 492 | Don't make a copy of the string, leave it as a ustring. ustring param_label = param->name; | |
| 493 | Don't use a set for this, just compare with the strings directly. That's both more readable and more efficient. | |
| 498 | Leave out .string(), no need to make a copy of the string for comparison. Same for two other lines below. | |
| 507–510 | Leave this page metadata placeholder out. | |
| 511–512 | Leave out the continue statement, it does nothing. | |
| 514–517 | Leave this min/max placeholder out. | |
| 519 | Leave out the continue statement, it does nothing. | |
| 561 | Always use {}. | |
| intern/cycles/blender/blender_python.cpp | ||
|---|---|---|
| 494 | you should modernize this loop see:https://clang.llvm.org/extra/clang-tidy/checks/modernize-loop-convert.html | |
| intern/cycles/blender/blender_python.cpp | ||
|---|---|---|
| 494 | Thanks for the tip! | |
| intern/cycles/blender/blender_shader.cpp | ||
|---|---|---|
| 198 | Have you tested if this actually works, that an OSL shader with this metadata correctly executes? From looking at the code, I think Cycles internally will create a socket with type SocketType::INT, and so that's what would need to be used here. | |
| intern/cycles/blender/blender_shader.cpp | ||
|---|---|---|
| 198 | In the tests I've done, this worked fine, but I can change it to 'SocketType::INT' and do some more testing. | |
Comment Actions
Issues found testing this patch:
- The label does not work for outputs
- When using a label different than the identifier, the node no longer works in Cycles. I think because blender_shader.cpp looks at the name and not the identifier of sockets
- When adding or changing a label, the socket does not refresh to show the new label
- Reading the boolean value needs to be changed to:
@@ -215,7 +217,12 @@ static void set_default_value(ShaderInput *input,
break;
}
case SocketType::INT: {
- node->set(socket, get_int(b_sock.ptr, "default_value"));
+ if (b_sock.type() == BL::NodeSocket::type_BOOLEAN) {
+ node->set(socket, get_boolean(b_sock.ptr, "default_value"));
+ }
+ else {
+ node->set(socket, get_int(b_sock.ptr, "default_value"));
+ }
break;
}
case SocketType::COLOR: {Comment Actions
Thanks for review, Brecht.
1: The label does not work for outputs
I only added code to change the names of the input parameters, not the output ones.
I think before adding it, I should solve the following problem..
2: When using a label different than the identifier, the node no longer works in Cycles. I think because blender_shader.cpp looks at the name and not the identifier of sockets
So what would be the solution? Don't rename the parameters, or change the way Cycles identifies the parameter by the socket name to the identifier?
I use this way with my nodes in the python side. Among other things, because using the identifier gives me other additional advantages.
Anyway, the method that checks the socket by name, node_find_input_by_name is only called once inside blender_shader.cpp.
3: When adding or changing a label, the socket does not refresh to show the new label
I guess it refers to when you change the parameter name in the text editor..
I think the error may be caused by the code that updates the node's sockets and tries to keep those that are of the same type. But I'm not sure..
else {
b_sock = b_node.inputs[param->name.string()];
/* remove if type no longer matches */
if (b_sock && b_sock.bl_idname() != socket_type) {
b_node.inputs.remove(b_data, b_sock);
b_sock = BL::NodeSocket(PointerRNA_NULL);
}
}4: Reading the boolean value needs to be changed to...
Done.
Comment Actions
OK. Last updates:
1: The label does not work for outputs
- Now is fixed. I added the same code as for inputs.
2: When using a label different than the identifier, the node no longer works in Cycles. I think because blender_shader.cpp looks at the name and not the identifier of sockets
- Now is solved. I have modified the code inside the node_find_input_by_name and node_find_output_by_name functions to use 'identifier' instead of 'name'.
3: When adding or changing a label, the socket does not refresh to show the new label
- I'm working to solve this..
4: Reading the boolean value needs to be changed to...
- Done.
Comment Actions
Updated patch.
3: When adding or changing a label, the socket does not refresh to show the new label
- As I suspected, this is the function that prevents the 'metadata' information from being updated when the code is modified in the editor.
else {
b_sock = b_node.inputs[param->name.string()];
/* remove if type no longer matches */
if (b_sock && b_sock.bl_idname() != socket_type) {
b_node.inputs.remove(b_data, b_sock);
b_sock = BL::NodeSocket(PointerRNA_NULL);
}
}The name of the parameters is obtained from the metadata, if the input is not updated, recompiling the node does not update the metadata for that input.
I have modified this function, so that it updates all the inputs when the node is recompiled, but this has a problem: a small change in the code is enough so that when updating the node, all connections are lost.
@Brecht Van Lommel (brecht) what do you think?
I think this is not practical and causes more problems than we are trying to solve.
We could now apply only the changes for the boolean options and leave the names of the parameters( and others improves..) as a pending task( after vacations..)
Cheers..
Comment Actions
The changes to blender_shader.cpp are breaking many of the Cycles tests, since the node_use_modified_socket_name mechanism does not work with identifiers. I'll commit a separate change to handle that.
I'll also fix it some socket labels can be changed without losing connections, and then commit this patch.