Geometry Node: Input Color node: Custom color and value color is the same property #104909

New Issue

Andrew Ivory · 2023-02-18T06:32:34+01:00

Andrew Ivory commented

2023-02-18 06:32:34 +01:00

System Information
Operating system: Linux-5.15.0-60-generic-x86_64-with-glibc2.31 64 Bits
Graphics card: NVIDIA GeForce GTX 1060 3GB/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 515.86.01

Blender Version
Broken: version: 3.4.1, branch: blender-v3.4-release, commit date: 2022-12-19 17:00, hash: rB55485cb379f7
Worked: Never.

Short description of error

Custom color and value color is the same property. Impossible to set custom color to node, always edited value color.

Exact steps for others to reproduce the error

Create Geometry Node tree.
Create Input Color node
In N panel, Node page:
a. Enable Custom Color button.
b. Try to change custom color.

**System Information** Operating system: Linux-5.15.0-60-generic-x86_64-with-glibc2.31 64 Bits Graphics card: NVIDIA GeForce GTX 1060 3GB/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 515.86.01 **Blender Version** Broken: version: 3.4.1, branch: blender-v3.4-release, commit date: 2022-12-19 17:00, hash: `rB55485cb379f7` Worked: Never. **Short description of error** Custom color and value color is the same property. Impossible to set custom color to node, always edited value color. **Exact steps for others to reproduce the error** 1. Create Geometry Node tree. 2. Create `Input Color` node 3. In `N` panel, `Node` page: a. Enable `Custom Color` button. b. Try to change custom color.

2023-02-18 17-58-42.mp4

6.0 MiB

Andrew Ivory added the

labels 2023-02-18 06:32:34 +01:00

Iliya Katushenock changed title from ~~Unusual Node Custom Color Behavior for Color Node in Geometry Node~~ to Geometry Node: Input Color node: Custom color and value color is the same property

2023-02-18 16:00:26 +01:00

Iliya Katushenock added

and removed

labels 2023-02-18 16:00:43 +01:00

Philipp Oeser commented

2023-02-27 10:52:14 +01:00

@CharlieJolly , @HooglyBoogly : thats a nameclash.
Actually, I am not sure why we go through NodeStorage here at all? Seems like using the socket directly should be fine? (of course that should probably be done for all constant inputs then?)



diff --git a/source/blender/makesrna/intern/rna_nodetree.c b/source/blender/makesrna/intern/rna_nodetree.c
index 71d6c72c05d..09f0f90ff9a 100644
--- a/source/blender/makesrna/intern/rna_nodetree.c
+++ b/source/blender/makesrna/intern/rna_nodetree.c
@@ -5094,19 +5094,6 @@ static void def_texture(StructRNA *srna)
   RNA_def_property_update(prop, NC_NODE | NA_EDITED, "rna_Node_update");
 }
 
-static void def_fn_input_color(StructRNA *srna)
-{
-  PropertyRNA *prop;
-
-  RNA_def_struct_sdna_from(srna, "NodeInputColor", "storage");
-
-  prop = RNA_def_property(srna, "color", PROP_FLOAT, PROP_COLOR);
-  RNA_def_property_array(prop, 4);
-  RNA_def_property_float_sdna(prop, NULL, "color");
-  RNA_def_property_ui_text(prop, "Color", "");
-  RNA_def_property_update(prop, NC_NODE | NA_EDITED, "rna_Node_update");
-}
-
 static void def_fn_input_bool(StructRNA *srna)
 {
   PropertyRNA *prop;
diff --git a/source/blender/nodes/NOD_static_types.h b/source/blender/nodes/NOD_static_types.h
index cb97e88ac9f..aeff189dba9 100644
--- a/source/blender/nodes/NOD_static_types.h
+++ b/source/blender/nodes/NOD_static_types.h
@@ -267,7 +267,7 @@ DefNode(FunctionNode, FN_NODE_COMBINE_COLOR, def_fn_combsep_color, "COMBINE_COLO
 DefNode(FunctionNode, FN_NODE_COMPARE, def_compare, "COMPARE", Compare, "Compare", "")
 DefNode(FunctionNode, FN_NODE_FLOAT_TO_INT, def_float_to_int, "FLOAT_TO_INT", FloatToInt, "Float to Integer", "")
 DefNode(FunctionNode, FN_NODE_INPUT_BOOL, def_fn_input_bool, "INPUT_BOOL", InputBool, "Boolean", "")
-DefNode(FunctionNode, FN_NODE_INPUT_COLOR, def_fn_input_color, "INPUT_COLOR", InputColor, "Color", "")
+DefNode(FunctionNode, FN_NODE_INPUT_COLOR, 0, "INPUT_COLOR", InputColor, "Color", "")
 DefNode(FunctionNode, FN_NODE_INPUT_INT, def_fn_input_int, "INPUT_INT", InputInt, "Integer", "")
 DefNode(FunctionNode, FN_NODE_INPUT_SPECIAL_CHARACTERS, 0, "INPUT_SPECIAL_CHARACTERS", InputSpecialCharacters, "Special Characters", "")
 DefNode(FunctionNode, FN_NODE_INPUT_STRING, def_fn_input_string, "INPUT_STRING", InputString, "String", "")
diff --git a/source/blender/nodes/function/CMakeLists.txt b/source/blender/nodes/function/CMakeLists.txt
index 481ab82602b..a32950212ec 100644
--- a/source/blender/nodes/function/CMakeLists.txt
+++ b/source/blender/nodes/function/CMakeLists.txt
@@ -14,6 +14,8 @@ set(INC
   ../../makesrna
   ../../windowmanager
   ../../../../intern/guardedalloc
+  # RNA_prototypes.h
+  ${CMAKE_BINARY_DIR}/source/blender/makesrna
 )
 
 set(INC_SYS
diff --git a/source/blender/nodes/function/nodes/node_fn_input_color.cc b/source/blender/nodes/function/nodes/node_fn_input_color.cc
index 30a3bcf29ab..fad563c8f32 100644
--- a/source/blender/nodes/function/nodes/node_fn_input_color.cc
+++ b/source/blender/nodes/function/nodes/node_fn_input_color.cc
@@ -1,4 +1,5 @@
 /* SPDX-License-Identifier: GPL-2.0-or-later */
+#include "RNA_prototypes.h"
 
 #include "node_function_util.hh"
 
@@ -16,15 +17,22 @@ static void node_declare(NodeDeclarationBuilder &b)
 
 static void node_layout(uiLayout *layout, bContext * /*C*/, PointerRNA *ptr)
 {
-  uiTemplateColorPicker(layout, ptr, "color", true, false, false, true);
-  uiItemR(layout, ptr, "color", UI_ITEM_R_SPLIT_EMPTY_NAME, "", ICON_NONE);
+  bNode *node = (bNode *)ptr->data;
+  /* first output stores value */
+  bNodeSocket *output = (bNodeSocket *)node->outputs.first;
+  PointerRNA sockptr;
+  RNA_pointer_create(ptr->owner_id, &RNA_NodeSocket, output, &sockptr);
+
+  uiTemplateColorPicker(layout, &sockptr, "default_value", true, false, false, true);
+  uiItemR(layout, &sockptr, "default_value", UI_ITEM_R_SPLIT_EMPTY_NAME, "", ICON_NONE);
 }
 
 static void node_build_multi_function(blender::nodes::NodeMultiFunctionBuilder &builder)
 {
   const bNode &bnode = builder.node();
-  NodeInputColor *node_storage = static_cast<NodeInputColor *>(bnode.storage);
-  blender::ColorGeometry4f color = (ColorGeometry4f)node_storage->color;
+  bNodeSocket *output = (bNodeSocket *)bnode.outputs.first;
+  float *value = static_cast<bNodeSocketValueRGBA *>(output->default_value)->value;
+  blender::ColorGeometry4f color = (ColorGeometry4f)value;
   builder.construct_and_set_matching_fn<blender::mf::CustomMF_Constant<ColorGeometry4f>>(color);
 }

@CharlieJolly , @HooglyBoogly : thats a nameclash. Actually, I am not sure why we go through NodeStorage here at all? Seems like using the socket directly should be fine? (of course that should probably be done for all constant inputs then?) ``` diff --git a/source/blender/makesrna/intern/rna_nodetree.c b/source/blender/makesrna/intern/rna_nodetree.c index 71d6c72c05d..09f0f90ff9a 100644 --- a/source/blender/makesrna/intern/rna_nodetree.c +++ b/source/blender/makesrna/intern/rna_nodetree.c @@ -5094,19 +5094,6 @@ static void def_texture(StructRNA *srna) RNA_def_property_update(prop, NC_NODE | NA_EDITED, "rna_Node_update"); } -static void def_fn_input_color(StructRNA *srna) -{ - PropertyRNA *prop; - - RNA_def_struct_sdna_from(srna, "NodeInputColor", "storage"); - - prop = RNA_def_property(srna, "color", PROP_FLOAT, PROP_COLOR); - RNA_def_property_array(prop, 4); - RNA_def_property_float_sdna(prop, NULL, "color"); - RNA_def_property_ui_text(prop, "Color", ""); - RNA_def_property_update(prop, NC_NODE | NA_EDITED, "rna_Node_update"); -} - static void def_fn_input_bool(StructRNA *srna) { PropertyRNA *prop; diff --git a/source/blender/nodes/NOD_static_types.h b/source/blender/nodes/NOD_static_types.h index cb97e88ac9f..aeff189dba9 100644 --- a/source/blender/nodes/NOD_static_types.h +++ b/source/blender/nodes/NOD_static_types.h @@ -267,7 +267,7 @@ DefNode(FunctionNode, FN_NODE_COMBINE_COLOR, def_fn_combsep_color, "COMBINE_COLO DefNode(FunctionNode, FN_NODE_COMPARE, def_compare, "COMPARE", Compare, "Compare", "") DefNode(FunctionNode, FN_NODE_FLOAT_TO_INT, def_float_to_int, "FLOAT_TO_INT", FloatToInt, "Float to Integer", "") DefNode(FunctionNode, FN_NODE_INPUT_BOOL, def_fn_input_bool, "INPUT_BOOL", InputBool, "Boolean", "") -DefNode(FunctionNode, FN_NODE_INPUT_COLOR, def_fn_input_color, "INPUT_COLOR", InputColor, "Color", "") +DefNode(FunctionNode, FN_NODE_INPUT_COLOR, 0, "INPUT_COLOR", InputColor, "Color", "") DefNode(FunctionNode, FN_NODE_INPUT_INT, def_fn_input_int, "INPUT_INT", InputInt, "Integer", "") DefNode(FunctionNode, FN_NODE_INPUT_SPECIAL_CHARACTERS, 0, "INPUT_SPECIAL_CHARACTERS", InputSpecialCharacters, "Special Characters", "") DefNode(FunctionNode, FN_NODE_INPUT_STRING, def_fn_input_string, "INPUT_STRING", InputString, "String", "") diff --git a/source/blender/nodes/function/CMakeLists.txt b/source/blender/nodes/function/CMakeLists.txt index 481ab82602b..a32950212ec 100644 --- a/source/blender/nodes/function/CMakeLists.txt +++ b/source/blender/nodes/function/CMakeLists.txt @@ -14,6 +14,8 @@ set(INC ../../makesrna ../../windowmanager ../../../../intern/guardedalloc + # RNA_prototypes.h + ${CMAKE_BINARY_DIR}/source/blender/makesrna ) set(INC_SYS diff --git a/source/blender/nodes/function/nodes/node_fn_input_color.cc b/source/blender/nodes/function/nodes/node_fn_input_color.cc index 30a3bcf29ab..fad563c8f32 100644 --- a/source/blender/nodes/function/nodes/node_fn_input_color.cc +++ b/source/blender/nodes/function/nodes/node_fn_input_color.cc @@ -1,4 +1,5 @@ /* SPDX-License-Identifier: GPL-2.0-or-later */ +#include "RNA_prototypes.h" #include "node_function_util.hh" @@ -16,15 +17,22 @@ static void node_declare(NodeDeclarationBuilder &b) static void node_layout(uiLayout *layout, bContext * /*C*/, PointerRNA *ptr) { - uiTemplateColorPicker(layout, ptr, "color", true, false, false, true); - uiItemR(layout, ptr, "color", UI_ITEM_R_SPLIT_EMPTY_NAME, "", ICON_NONE); + bNode *node = (bNode *)ptr->data; + /* first output stores value */ + bNodeSocket *output = (bNodeSocket *)node->outputs.first; + PointerRNA sockptr; + RNA_pointer_create(ptr->owner_id, &RNA_NodeSocket, output, &sockptr); + + uiTemplateColorPicker(layout, &sockptr, "default_value", true, false, false, true); + uiItemR(layout, &sockptr, "default_value", UI_ITEM_R_SPLIT_EMPTY_NAME, "", ICON_NONE); } static void node_build_multi_function(blender::nodes::NodeMultiFunctionBuilder &builder) { const bNode &bnode = builder.node(); - NodeInputColor *node_storage = static_cast<NodeInputColor *>(bnode.storage); - blender::ColorGeometry4f color = (ColorGeometry4f)node_storage->color; + bNodeSocket *output = (bNodeSocket *)bnode.outputs.first; + float *value = static_cast<bNodeSocketValueRGBA *>(output->default_value)->value; + blender::ColorGeometry4f color = (ColorGeometry4f)value; builder.construct_and_set_matching_fn<blender::mf::CustomMF_Constant<ColorGeometry4f>>(color); } ```

Iliya Katushenock commented

2023-02-27 14:14:07 +01:00

@lichtwerk This is because only nodes need to own this kind of data in order to have a consistent api.

Philipp Oeser commented

2023-02-27 14:53:39 +01:00

@mod_moder : but other nodes (shader, compositing) do it differently (they use the socket directly as in my diff), so that seems inconsistent as well?

Iliya Katushenock commented

2023-02-27 14:58:02 +01:00

@lichtwerk I know that the geometry nodes are much new than other nodal systems, maybe just a legacy of generations. Indeed, it makes sense to expect a response from a more serious developer

Philipp Oeser referenced this issue

2023-08-24 12:58:13 +02:00

AOV output node has conflicting python parameter naming convention which makes them unselectable by name via python. #111450

Iliya Katushenock referenced this issue

2023-10-01 01:52:55 +02:00

Conflicts of identical attributes api complicate life. #113090

Jacques Lucke referenced this issue

2024-03-10 15:16:15 +01:00

Fix #113090, #111450, #104909: node rna properties collide with built-in names #119284

Jacques Lucke referenced this issue from a commit

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

Fix #113090, #111450, #104909: node rna properties collide with built-in names