Skip to content

Auto create Read x node when inserting map node into chain (#4792) - #4812

Closed
Ankit-cs wants to merge 1 commit into
GraphiteEditor:masterfrom
Ankit-cs:auto-create-read-node-for-map
Closed

Ankit-cs wants to merge 1 commit into
GraphiteEditor:masterfrom
Ankit-cs:auto-create-read-node-for-map

Conversation

@Ankit-cs

Copy link
Copy Markdown
Contributor

Closes #4792

Description

When dragging or splicing a Map node into a wire/chain (such as after a Stroke node), the Map node's primary input 0 (content) receives the upstream type (e.g. Vector), while its secondary input 1 (mapped) defaults to expecting String[]. This caused a type mismatch error when dropping the node onto a wire.

Changes

  • In NodeNetworkInterface::insert_node_between and insert_node_before_input in layout.rs:
    • Added auto_create_read_node_for_map to detect when a Map node is inserted onto a wire.
    • If the Map node's mapped input is unconnected, it queries the upstream output type feeding input 0 (content).
    • Maps the incoming type (Vector, Graphic, Raster, Color, Gradient, Number, String) to the corresponding Read X node (read_vector, read_graphic, etc.).
    • Instantiates and positions the Read X node directly below the chain (offset by IVec2::new(-NODE_CHAIN_WIDTH, 2) relative to Map).
    • Automatically connects the Read X node's output to the Map node's mapped input.

Tests

  • Added inserting_map_node_into_chain_auto_creates_read_node in characterization_tests.rs to verify that inserting a Map node into a chain carrying vector data automatically creates and wires the Read Vector node at the expected relative coordinates.

@TrueDoctor

TrueDoctor commented Oct 25, 2026 •

Copy link
Copy Markdown
Member

!build (Run ID 40217970365)

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

2 issues found across 2 files

Confidence score: 3/5

  • In editor/src/messages/portfolio/document/utility_types/network_interface/layout.rs, inserting Map after GPU raster data can select Read Raster, whose output is Raster<CPU>, and wire an incompatible function. Exclude GPU raster data from this selection.
  • In editor/src/messages/portfolio/document/utility_types/network_interface/layout.rs, the auto-created Read node stays at the Map’s old position when the Map is moved to the feeder. Create the Read node after the Map reaches its final position.

You’re at about 92% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="editor/src/messages/portfolio/document/utility_types/network_interface/layout.rs">

<violation number="1" location="editor/src/messages/portfolio/document/utility_types/network_interface/layout.rs:1187">
P2: `Read Raster` returns `Raster<CPU>`, but this branch also selects it for `Raster<GPU>` content. Inserting Map after GPU raster data therefore wires an incompatible mapped function; do not select this node for GPU data unless a GPU-compatible reader exists.</violation>

<violation number="2" location="editor/src/messages/portfolio/document/utility_types/network_interface/layout.rs:1200">
P2: `insert_node_before_input` creates the Read node before moving an absolute Map to the feeder's position, so the Read node remains at the Map's old location. Run the auto-creation after the Map's final positioning and branch shift, or move the created Read node with the Map.</violation>
</file>

Reply to a comment to ask cubic a question or push back. It learns from your replies.

Re-trigger cubic

};

let mut read_template = read_definition.default_node_template();
let map_position = self.position(node_id, network_path).unwrap_or_default();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: insert_node_before_input creates the Read node before moving an absolute Map to the feeder's position, so the Read node remains at the Map's old location. Run the auto-creation after the Map's final positioning and branch shift, or move the created Read node with the Map.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At editor/src/messages/portfolio/document/utility_types/network_interface/layout.rs, line 1200:

<comment>`insert_node_before_input` creates the Read node before moving an absolute Map to the feeder's position, so the Read node remains at the Map's old location. Run the auto-creation after the Map's final positioning and branch shift, or move the created Read node with the Map.</comment>

<file context>
@@ -1154,8 +1154,59 @@ impl NodeNetworkInterface {
+		};
+
+		let mut read_template = read_definition.default_node_template();
+		let map_position = self.position(node_id, network_path).unwrap_or_default();
+		let read_position = map_position + IVec2::new(-NODE_CHAIN_WIDTH, 2);
+		read_template.node_type_metadata = NodeTypePersistentMetadata::node(read_position);
</file context>

let read_proto_id = match element_name.as_deref() {
Some("Vector") => graphene_std::context::read_vector::IDENTIFIER,
Some("Graphic") => graphene_std::context::read_graphic::IDENTIFIER,
Some("Raster" | "Raster<CPU>" | "Raster<GPU>") => graphene_std::context::read_raster::IDENTIFIER,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: Read Raster returns Raster<CPU>, but this branch also selects it for Raster<GPU> content. Inserting Map after GPU raster data therefore wires an incompatible mapped function; do not select this node for GPU data unless a GPU-compatible reader exists.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At editor/src/messages/portfolio/document/utility_types/network_interface/layout.rs, line 1187:

<comment>`Read Raster` returns `Raster<CPU>`, but this branch also selects it for `Raster<GPU>` content. Inserting Map after GPU raster data therefore wires an incompatible mapped function; do not select this node for GPU data unless a GPU-compatible reader exists.</comment>

<file context>
@@ -1154,8 +1154,59 @@ impl NodeNetworkInterface {
+		let read_proto_id = match element_name.as_deref() {
+			Some("Vector") => graphene_std::context::read_vector::IDENTIFIER,
+			Some("Graphic") => graphene_std::context::read_graphic::IDENTIFIER,
+			Some("Raster" | "Raster<CPU>" | "Raster<GPU>") => graphene_std::context::read_raster::IDENTIFIER,
+			Some("Color") => graphene_std::context::read_color::IDENTIFIER,
+			Some("Gradient") => graphene_std::context::read_gradient::IDENTIFIER,
</file context>
Suggested change
Some("Raster" | "Raster<CPU>" | "Raster<GPU>") => graphene_std::context::read_raster::IDENTIFIER,
Some("Raster" | "Raster<CPU>") => graphene_std::context::read_raster::IDENTIFIER,

@Keavon

Keavon commented Oct 25, 2026

Copy link
Copy Markdown
Member

@Keavon Keavon closed this Oct 25, 2026
@github-actions

Copy link
Copy Markdown
📦 Web Build Complete for 37abf34
https://defd5464.graphite.pages.dev

Wasm: 41.85 MB — JS: 0.73 MB — CSS: 0.13 MB — Fonts: 0.69 MB — Images: 0.09 MB — All Assets: 43.49 MB

@Ankit-cs

Ankit-cs commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor Author

I want to try again once @Keavon

@TrueDoctor

Copy link
Copy Markdown
Member

Feel free to do so @Ankit-cs just open a new pr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Auto create Read x node when inserting map node into chain

3 participants