Fix incorrect flattening behavior for hidden nodes#3759
Fix incorrect flattening behavior for hidden nodes#3759JustJ01 wants to merge 4 commits intoGraphiteEditor:masterfrom
Conversation
| } | ||
| }; | ||
| TaggedValue::from_type_or_none(&guaranteed_type) | ||
| TaggedValue::from_type(&guaranteed_type).expect("Failed to construct TaggedValue for identity type") |
There was a problem hiding this comment.
This can crash when used on types from old documents which are no longer handled by the macro. A log::error! with the from_type_or_none would be safer
| #[derive(Clone, Debug, PartialEq, Hash, serde::Serialize, serde::Deserialize)] | ||
| pub enum Visible { | ||
| Passthrough, | ||
| Value(Type), //kept for backward compatibility |
There was a problem hiding this comment.
backward compatibility with what? This is a new enum.
| pub struct HiddenNodeInput { | ||
| pub input_index: usize, | ||
| pub tagged_value: TaggedValue, | ||
| } |
There was a problem hiding this comment.
I would expect this to include the NodeId so it can be matched with the downstream (toward the export) node
| #[serde(default = "return_true")] | ||
| pub visible: bool, | ||
| #[serde(default = "return_visible_true")] | ||
| pub visible: Visible, |
There was a problem hiding this comment.
This needs to be an option. None means visible, Some means hidden with the appropriate handling.
|
|
||
| #[derive(Clone, Debug, PartialEq, Hash, serde::Serialize, serde::Deserialize)] | ||
| pub enum Visible { | ||
| Passthrough, |
There was a problem hiding this comment.
Passthrough means to use the current hiding logic, where it is replaced with a Passthrough node
| pub enum Visible { | ||
| Passthrough, | ||
| Value(Type), //kept for backward compatibility | ||
| TaggedValues(Vec<HiddenNodeInput>), |
There was a problem hiding this comment.
Tagged values only occurs for nodes with no inputs, where valid input values must be chosen for the nodes it is connected to
There was a problem hiding this comment.
Also it might be more clear to rename this to Hidden, since it is just behavior for Hidden nodes
| }; | ||
| let Some(node) = network.nodes.get_mut(node_id) else { | ||
| log::error!("Could not get node {node_id} in set_visibility"); | ||
| if is_visible { |
There was a problem hiding this comment.
This logic is generally wrong. For nodes with no inputs, we want to use the downstream traversal and type lookup solution. For nodes with any number of inputs they just become pass through nodes as is the current behavior.
2 participants
Closes #3629
When a node like Instance Position or Pointer Position is hidden, the flattening logic incorrectly replaces it with a default
()value.This causes:
ConvertNode<DVec2>)Cannot construct default value for hidden nodeentered unreachable code: tried to resolve not flattened nodeExpected Behavior
Hiding a node should:
()unless the node’s real output type is()Solution
Screen.Recording.2026-02-14.021205.mp4
Updated
flatten_with_fnshandling forVisible::Value(output_type).New behavior:
If a compatible input exists
If no compatible input exists
Prevent incorrect
()propagation()unless the node’s actual output type is unit.Result
Instance Positionno longer crashes.ConvertNode<DVec2>no longer receives().