Skip to content

Fix hardcoded input indices #3829

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Tyooughtul wants to merge 5 commits into
base: master
Choose a base branch
from
+280 −172
Open

Fix hardcoded input indices #3829

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
945e712
fix: replace hardcoded input indices with ::INDEX constants
Tyooughtul Feb 25, 2026
d4ae26e
fix: restore the debug information of the master branch
Tyooughtul Feb 25, 2026
8ea25ac
fix:
Tyooughtul Mar 3, 2026
c0d60b7
fix:
Tyooughtul Mar 4, 2026
1fb2cfa
fix: revert unrelated float constant changes
Tyooughtul Mar 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions editor/src/messages/portfolio/document/document_message_handler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1900,7 +1900,12 @@ impl DocumentMessageHandler {

// If there's already a boolean operation on the selected layer, update it with the new operation
if let (Some(upstream_boolean_op), Some(only_selected_layer)) = (upstream_boolean_op, only_selected_layer) {
network_interface.set_input(&InputConnector::node(upstream_boolean_op, 1), NodeInput::value(TaggedValue::BooleanOperation(operation), false), &[]);
const BOOLEAN_OPERATION_INDEX: usize = 1;
network_interface.set_input(
&InputConnector::node(upstream_boolean_op, BOOLEAN_OPERATION_INDEX),
NodeInput::value(TaggedValue::BooleanOperation(operation), false),
&[],
);

responses.add(NodeGraphMessage::RunDocumentGraph);

Expand Down Expand Up @@ -2828,8 +2833,9 @@ impl DocumentMessageHandler {
.popover_layout({
// Showing only compatible types for the layer based on the output type of the node upstream from its horizontal input
let compatible_type = selected_layer.and_then(|layer| {
const LAYER_SECONDARY_INPUT_INDEX: usize = 1;
self.network_interface
.upstream_output_connector(&InputConnector::node(layer.to_node(), 1), &[])
.upstream_output_connector(&InputConnector::node(layer.to_node(), LAYER_SECONDARY_INPUT_INDEX), &[])
.and_then(|upstream_output| self.network_interface.output_type(&upstream_output, &[]).add_node_string())
});

Expand Down
39 changes: 23 additions & 16 deletions editor/src/messages/portfolio/document/graph_operation/graph_operation_message_handler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,8 @@ impl MessageHandler<GraphOperationMessage, GraphOperationMessageContext<'_>> for
}
}
GraphOperationMessage::SetUpstreamToChain { layer } => {
let Some(OutputConnector::Node { node_id: first_chain_node, .. }) = network_interface.upstream_output_connector(&InputConnector::node(layer.to_node(), 1), &[]) else {
const LAYER_SECONDARY_INPUT_INDEX: usize = 1;
let Some(OutputConnector::Node { node_id: first_chain_node, .. }) = network_interface.upstream_output_connector(&InputConnector::node(layer.to_node(), LAYER_SECONDARY_INPUT_INDEX), &[]) else {
return;
};

Expand Down Expand Up @@ -145,15 +146,18 @@ impl MessageHandler<GraphOperationMessage, GraphOperationMessageContext<'_>> for

// Set the bottom input of the artboard back to artboard
let bottom_input = NodeInput::value(TaggedValue::Artboard(Table::new()), true);
network_interface.set_input(&InputConnector::node(artboard_layer.to_node(), 0), bottom_input, &[]);
const ARTBOARD_BASE_INDEX: usize = 0;
network_interface.set_input(&InputConnector::node(artboard_layer.to_node(), ARTBOARD_BASE_INDEX), bottom_input, &[]);
} else {
// We have some non layers (e.g. just a rectangle node). We disconnect the bottom input and connect it to the left input.
network_interface.disconnect_input(&InputConnector::node(artboard_layer.to_node(), 0), &[]);
network_interface.set_input(&InputConnector::node(artboard_layer.to_node(), 1), primary_input, &[]);
const ARTBOARD_BASE_INDEX: usize = 0;
const ARTBOARD_CONTENT_INDEX: usize = 1;
network_interface.disconnect_input(&InputConnector::node(artboard_layer.to_node(), ARTBOARD_BASE_INDEX), &[]);
network_interface.set_input(&InputConnector::node(artboard_layer.to_node(), ARTBOARD_CONTENT_INDEX), primary_input, &[]);

// Set the bottom input of the artboard back to artboard
let bottom_input = NodeInput::value(TaggedValue::Artboard(Table::new()), true);
network_interface.set_input(&InputConnector::node(artboard_layer.to_node(), 0), bottom_input, &[]);
network_interface.set_input(&InputConnector::node(artboard_layer.to_node(), ARTBOARD_BASE_INDEX), bottom_input, &[]);
}
}
responses.add_front(NodeGraphMessage::SelectedNodesSet { nodes: vec![id] });
Expand Down Expand Up @@ -193,9 +197,10 @@ impl MessageHandler<GraphOperationMessage, GraphOperationMessageContext<'_>> for
let first_new_node_id = new_ids[&NodeId(0)];
responses.add(NodeGraphMessage::AddNodes { nodes, new_ids });

const LAYER_SECONDARY_INPUT_INDEX: usize = 1;
responses.add(NodeGraphMessage::SetInput {
input_connector: InputConnector::node(layer.to_node(), 1),
input: NodeInput::node(first_new_node_id, 0),
input_connector: InputConnector::node(layer.to_node(), LAYER_SECONDARY_INPUT_INDEX),
input: NodeInput::node(first_new_node_id, NodeInput::PRIMARY_OUTPUT_INDEX),
});
}
// Move the layer and all nodes to the correct position in the network
Expand Down Expand Up @@ -251,13 +256,14 @@ impl MessageHandler<GraphOperationMessage, GraphOperationMessageContext<'_>> for
return;
};

artboard_data.insert(
artboard.to_node(),
ArtboardInfo {
input_node: NodeInput::node(document_node.inputs[1].as_node().unwrap_or_default(), 0),
const ARTBOARD_CONTENT_INDEX: usize = 1;
artboard_data.insert(
artboard.to_node(),
ArtboardInfo {
input_node: NodeInput::node(document_node.inputs[ARTBOARD_CONTENT_INDEX].as_node().unwrap_or_default(), NodeInput::PRIMARY_OUTPUT_INDEX),
output_nodes: network_interface
.outward_wires(&[])
.and_then(|outward_wires| outward_wires.get(&OutputConnector::node(artboard.to_node(), 0)))
.and_then(|outward_wires| outward_wires.get(&OutputConnector::node(artboard.to_node(), OutputConnector::PRIMARY_OUTPUT_INDEX)))
.cloned()
.unwrap_or_default(),
merge_node: node_id,
Expand All @@ -281,14 +287,15 @@ impl MessageHandler<GraphOperationMessage, GraphOperationMessageContext<'_>> for
// Go through all artboards and connect them to the merge nodes
for artboard in &artboard_data {
// Modify downstream connections
responses.add(NodeGraphMessage::SetInput {
input_connector: InputConnector::node(artboard.1.merge_node, 1),
input: NodeInput::node(artboard.1.input_node.as_node().unwrap_or_default(), 0),
const MERGE_CONTENT_INDEX: usize = 1;
responses.add(NodeGraphMessage::SetInput {
input_connector: InputConnector::node(artboard.1.merge_node, MERGE_CONTENT_INDEX),
input: NodeInput::node(artboard.1.input_node.as_node().unwrap_or_default(), NodeInput::PRIMARY_OUTPUT_INDEX),
});

// Modify upstream connections
for outward_wire in &artboard.1.output_nodes {
let input = NodeInput::node(artboard_data[artboard.0].merge_node, 0);
let input = NodeInput::node(artboard_data[artboard.0].merge_node, NodeInput::PRIMARY_OUTPUT_INDEX);
let input_connector = match artboard_data.get(&outward_wire.node_id().unwrap_or_default()) {
Some(artboard_info) => InputConnector::node(artboard_info.merge_node, outward_wire.input_index()),
_ => *outward_wire,
Expand Down
38 changes: 29 additions & 9 deletions editor/src/messages/portfolio/document/graph_operation/transform_utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ use crate::messages::portfolio::document::utility_types::network_interface::{Inp
use glam::{DAffine2, DVec2};
use graph_craft::document::value::TaggedValue;
use graph_craft::document::{NodeId, NodeInput};
use graphene_std::NodeInputDecleration;
use graphene_std::subpath::Subpath;
use graphene_std::transform_nodes::transform::*;
use graphene_std::vector::PointId;

/// Convert an affine transform into the tuple `(scale, angle, translation, shear)` assuming `shear.y = 0`.
Expand Down Expand Up @@ -37,10 +39,10 @@ pub fn update_transform(network_interface: &mut NodeNetworkInterface, node_id: &
let rotation = rotation.to_degrees();
let shear = DVec2::new(shear.x.atan().to_degrees(), shear.y.atan().to_degrees());

network_interface.set_input(&InputConnector::node(*node_id, 1), NodeInput::value(TaggedValue::DVec2(translation), false), &[]);
network_interface.set_input(&InputConnector::node(*node_id, 2), NodeInput::value(TaggedValue::F64(rotation), false), &[]);
network_interface.set_input(&InputConnector::node(*node_id, 3), NodeInput::value(TaggedValue::DVec2(scale), false), &[]);
network_interface.set_input(&InputConnector::node(*node_id, 4), NodeInput::value(TaggedValue::DVec2(shear), false), &[]);
network_interface.set_input(&InputConnector::node(*node_id, TranslationInput::INDEX), NodeInput::value(TaggedValue::DVec2(translation), false), &[]);
network_interface.set_input(&InputConnector::node(*node_id, RotationInput::INDEX), NodeInput::value(TaggedValue::F64(rotation), false), &[]);
network_interface.set_input(&InputConnector::node(*node_id, ScaleInput::INDEX), NodeInput::value(TaggedValue::DVec2(scale), false), &[]);
network_interface.set_input(&InputConnector::node(*node_id, SkewInput::INDEX), NodeInput::value(TaggedValue::DVec2(shear), false), &[]);
}

// TODO: This should be extracted from the graph at the location of the transform node.
Expand Down Expand Up @@ -74,14 +76,26 @@ impl LayerBounds {

/// Get the current affine transform from the transform node's inputs
pub fn get_current_transform(inputs: &[NodeInput]) -> DAffine2 {
let translation = if let Some(&TaggedValue::DVec2(translation)) = inputs[1].as_value() {
let translation = if let Some(&TaggedValue::DVec2(translation)) = inputs[TranslationInput::INDEX].as_value() {
translation
} else {
DVec2::ZERO
};
let rotation = if let Some(&TaggedValue::F64(rotation)) = inputs[2].as_value() { rotation } else { 0. };
let scale = if let Some(&TaggedValue::DVec2(scale)) = inputs[3].as_value() { scale } else { DVec2::ONE };
let shear = if let Some(&TaggedValue::DVec2(shear)) = inputs[4].as_value() { shear } else { DVec2::ZERO };
let rotation = if let Some(&TaggedValue::F64(rotation)) = inputs[RotationInput::INDEX].as_value() {
rotation
} else {
0.
};
let scale = if let Some(&TaggedValue::DVec2(scale)) = inputs[ScaleInput::INDEX].as_value() {
scale
} else {
DVec2::ONE
};
let shear = if let Some(&TaggedValue::DVec2(shear)) = inputs[SkewInput::INDEX].as_value() {
shear
} else {
DVec2::ZERO
};

let rotation = rotation.to_radians();
let shear = DVec2::new(shear.x.to_radians().tan(), shear.y.to_radians().tan());
Expand All @@ -91,7 +105,13 @@ pub fn get_current_transform(inputs: &[NodeInput]) -> DAffine2 {

/// Extract the current normalized pivot from the layer
pub fn get_current_normalized_pivot(inputs: &[NodeInput]) -> DVec2 {
if let Some(&TaggedValue::DVec2(pivot)) = inputs[5].as_value() { pivot } else { DVec2::splat(0.5) }
const ORIGIN_OFFSET_INDEX: usize = 5;

if let Some(&TaggedValue::DVec2(pivot)) = inputs[ORIGIN_OFFSET_INDEX].as_value() {
pivot
} else {
DVec2::splat(0.5)
}
}

/// Expand a bounds to avoid div zero errors
Expand Down
40 changes: 29 additions & 11 deletions editor/src/messages/portfolio/document/graph_operation/utility_types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ impl<'a> ModifyInputsContext<'a> {
let mut post_node_input_connector = if parent == LayerNodeIdentifier::ROOT_PARENT {
InputConnector::Export(0)
} else {
InputConnector::node(parent.to_node(), 1)
const LAYER_SECONDARY_INPUT_INDEX: usize = 1;
Copy link

@cubic-dev-ai cubic-dev-ai bot Mar 5, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Duplicated local const LAYER_SECONDARY_INPUT_INDEX defined in two separate methods. Consider defining it once at module level (or as an associated constant like InputConnector::PRIMARY_INPUT_INDEX) so both call sites share a single source of truth, consistent with how other index constants are handled in this PR.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At editor/src/messages/portfolio/document/graph_operation/utility_types.rs, line 74:

<comment>Duplicated local `const LAYER_SECONDARY_INPUT_INDEX` defined in two separate methods. Consider defining it once at module level (or as an associated constant like `InputConnector::PRIMARY_INPUT_INDEX`) so both call sites share a single source of truth, consistent with how other index constants are handled in this PR.</comment>

<file context>
@@ -71,7 +71,8 @@ impl<'a> ModifyInputsContext<'a> {
 			InputConnector::Export(0)
 		} else {
-			InputConnector::node(parent.to_node(), 1)
+			const LAYER_SECONDARY_INPUT_INDEX: usize = 1;
+			InputConnector::node(parent.to_node(), LAYER_SECONDARY_INPUT_INDEX)
 		};
</file context>
Fix with Cubic

InputConnector::node(parent.to_node(), LAYER_SECONDARY_INPUT_INDEX)
};
// Skip layers based on skip_layer_nodes, which inserts the new layer at a certain index of the layer stack.
let mut current_index = 0;
Expand All @@ -91,7 +92,7 @@ impl<'a> ModifyInputsContext<'a> {
current_index += 1;
}
// Input as a sibling to the Layer node above
post_node_input_connector = InputConnector::node(*next_node_in_stack_id, 0);
post_node_input_connector = InputConnector::node(*next_node_in_stack_id, InputConnector::PRIMARY_INPUT_INDEX);
} else {
log::error!("Error getting post node: insert_index out of bounds");
break;
Expand All @@ -107,7 +108,7 @@ impl<'a> ModifyInputsContext<'a> {
match pre_node_output_connector {
Some(OutputConnector::Node { node_id: pre_node_id, .. }) if !network_interface.is_layer(&pre_node_id, &[]) => {
// Update post_node_input_connector for the next iteration
post_node_input_connector = InputConnector::node(pre_node_id, 0);
post_node_input_connector = InputConnector::node(pre_node_id, InputConnector::PRIMARY_INPUT_INDEX);
// Insert directly under layer if moving to the end of a layer stack that ends with a non layer node that does not have an exposed primary input
let primary_is_exposed = network_interface.input_from_connector(&post_node_input_connector, &[]).is_some_and(|input| input.is_exposed());
if !primary_is_exposed {
Expand Down Expand Up @@ -320,7 +321,8 @@ impl<'a> ModifyInputsContext<'a> {
// If inserting a 'Path' node, insert a 'Flatten Path' node if the type is `Graphic`.
// TODO: Allow the 'Path' node to operate on table data by utilizing the reference (index or ID?) for each row.
if node_definition.identifier == "Path" {
let layer_input_type = self.network_interface.input_type(&InputConnector::node(output_layer.to_node(), 1), &[]);
const LAYER_SECONDARY_INPUT_INDEX: usize = 1;
let layer_input_type = self.network_interface.input_type(&InputConnector::node(output_layer.to_node(), LAYER_SECONDARY_INPUT_INDEX), &[]);
if layer_input_type.compiled_nested_type() == Some(&concrete!(Table<Graphic>)) {
let Some(flatten_path_definition) = resolve_proto_node_type(graphene_std::vector_nodes::flatten_path::IDENTIFIER) else {
log::error!("Flatten Path does not exist in ModifyInputsContext::existing_node_id");
Expand Down Expand Up @@ -367,23 +369,23 @@ impl<'a> ModifyInputsContext<'a> {
let Some(blend_node_id) = self.existing_proto_node_id(graphene_std::blending_nodes::blending::IDENTIFIER, true) else {
return;
};
let input_connector = InputConnector::node(blend_node_id, 1);
let input_connector = InputConnector::node(blend_node_id, graphene_std::blending_nodes::blending::BlendModeInput::INDEX);
self.set_input_with_refresh(input_connector, NodeInput::value(TaggedValue::BlendMode(blend_mode), false), false);
}

pub fn opacity_set(&mut self, opacity: f64) {
let Some(blend_node_id) = self.existing_proto_node_id(graphene_std::blending_nodes::blending::IDENTIFIER, true) else {
return;
};
let input_connector = InputConnector::node(blend_node_id, 2);
let input_connector = InputConnector::node(blend_node_id, graphene_std::blending_nodes::blending::OpacityInput::INDEX);
self.set_input_with_refresh(input_connector, NodeInput::value(TaggedValue::F64(opacity * 100.), false), false);
}

pub fn blending_fill_set(&mut self, fill: f64) {
let Some(blend_node_id) = self.existing_proto_node_id(graphene_std::blending_nodes::blending::IDENTIFIER, true) else {
return;
};
let input_connector = InputConnector::node(blend_node_id, 3);
let input_connector = InputConnector::node(blend_node_id, graphene_std::blending_nodes::blending::FillInput::INDEX);
self.set_input_with_refresh(input_connector, NodeInput::value(TaggedValue::F64(fill * 100.), false), false);
}

Expand All @@ -392,7 +394,7 @@ impl<'a> ModifyInputsContext<'a> {
let Some(clip_node_id) = self.existing_proto_node_id(graphene_std::blending_nodes::blending::IDENTIFIER, true) else {
return;
};
let input_connector = InputConnector::node(clip_node_id, 4);
let input_connector = InputConnector::node(clip_node_id, graphene_std::blending_nodes::blending::ClipInput::INDEX);
self.set_input_with_refresh(input_connector, NodeInput::value(TaggedValue::Bool(clip), false), false);
}

Expand Down Expand Up @@ -505,10 +507,18 @@ impl<'a> ModifyInputsContext<'a> {
let Some(brush_node_id) = self.existing_network_node_id("Brush", true) else {
return;
};
self.set_input_with_refresh(InputConnector::node(brush_node_id, 1), NodeInput::value(TaggedValue::BrushStrokes(strokes), false), false);
const BRUSH_STROKES_INDEX: usize = 1;
self.set_input_with_refresh(
InputConnector::node(brush_node_id, BRUSH_STROKES_INDEX),
NodeInput::value(TaggedValue::BrushStrokes(strokes), false),
false,
);
}

pub fn resize_artboard(&mut self, location: IVec2, dimensions: IVec2) {
const ARTBOARD_LOCATION_INDEX: usize = 2;
const ARTBOARD_DIMENSIONS_INDEX: usize = 3;

let Some(artboard_node_id) = self.existing_network_node_id("Artboard", true) else {
return;
};
Expand All @@ -524,8 +534,16 @@ impl<'a> ModifyInputsContext<'a> {
dimensions.y *= -1;
location.y -= dimensions.y;
}
self.set_input_with_refresh(InputConnector::node(artboard_node_id, 2), NodeInput::value(TaggedValue::DVec2(location.into()), false), false);
self.set_input_with_refresh(InputConnector::node(artboard_node_id, 3), NodeInput::value(TaggedValue::DVec2(dimensions.into()), false), false);
self.set_input_with_refresh(
InputConnector::node(artboard_node_id, ARTBOARD_LOCATION_INDEX),
NodeInput::value(TaggedValue::DVec2(location.into()), false),
false,
);
self.set_input_with_refresh(
InputConnector::node(artboard_node_id, ARTBOARD_DIMENSIONS_INDEX),
NodeInput::value(TaggedValue::DVec2(dimensions.into()), false),
false,
);
}

/// Set the input, refresh the properties panel, and run the document graph if skip_rerender is false
Expand Down
Loading