Rework 'Split Vec2' and 'Split Channels' as multi-output nodes and merge 'Position/Tangent on Path' into 'Evaluate Path' - #4645
Conversation
There was a problem hiding this comment.
3 issues found across 16 files
Confidence score: 3/5
- In
document_migration.rs, old Split Channels wires will shift onto the hidden whole-image output and the wrong channel fields. Set the shift to1. - In
adjustments.rs, migrating old Extract Channel nodes to the Raster-only node leaves graphs extracting from colors or gradients with incompatible types. Preserve those implementations or migrate those graphs to a compatible node. - In
math/src/lib.rs, a Split Vec2 → Combine round trip drops input metadata such as transforms. Preserve the attributes when rebuilding the vector.
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_migration.rs">
<violation number="1" location="editor/src/messages/portfolio/document_migration.rs:1383">
P1: Legacy Split Channels has no primary output, but the replacement adds a hidden whole-image output before its channel fields. Set this shift to `1`; otherwise old red/green/blue/alpha wires target the hidden image/red/green/blue outputs respectively.</violation>
</file>
<file name="node-graph/nodes/math/src/lib.rs">
<violation number="1" location="node-graph/nodes/math/src/lib.rs:1774">
P2: `Split Vec2` copies the input attributes to both components, but `combine_vec2` rebuilds the vector with `Item::new_from_element`, discarding them. A Split → Combine round trip therefore loses metadata such as transforms; preserve the shared component attributes in `combine_vec2`.</violation>
</file>
<file name="node-graph/nodes/raster/src/adjustments.rs">
<violation number="1" location="node-graph/nodes/raster/src/adjustments.rs:158">
P2: This removes the prior `Color` and `Gradient` implementations, but migration redirects every old Extract Channel node to this Raster-only node. Existing graphs extracting channels from colors or gradients become type-incompatible; preserve those implementations or migrate those cases separately.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| let split_wrapper_replacements = [ | ||
| ("Split Vec2", graphene_std::math_nodes::split_vec_2::IDENTIFIER, 0), | ||
| ("Split Vector2", graphene_std::math_nodes::split_vec_2::IDENTIFIER, 1), | ||
| ("Split Channels", graphene_std::raster_nodes::adjustments::split_channels::IDENTIFIER, 0), |
There was a problem hiding this comment.
P1: Legacy Split Channels has no primary output, but the replacement adds a hidden whole-image output before its channel fields. Set this shift to 1; otherwise old red/green/blue/alpha wires target the hidden image/red/green/blue outputs respectively.
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_migration.rs, line 1383:
<comment>Legacy Split Channels has no primary output, but the replacement adds a hidden whole-image output before its channel fields. Set this shift to `1`; otherwise old red/green/blue/alpha wires target the hidden image/red/green/blue outputs respectively.</comment>
<file context>
@@ -1385,6 +1374,182 @@ pub fn document_migration_upgrades(document: &mut DocumentMessageHandler, reset_
+ let split_wrapper_replacements = [
+ ("Split Vec2", graphene_std::math_nodes::split_vec_2::IDENTIFIER, 0),
+ ("Split Vector2", graphene_std::math_nodes::split_vec_2::IDENTIFIER, 1),
+ ("Split Channels", graphene_std::raster_nodes::adjustments::split_channels::IDENTIFIER, 0),
+ ];
+ for (old_reference, new_identifier, output_shift) in split_wrapper_replacements {
</file context>
| ("Split Channels", graphene_std::raster_nodes::adjustments::split_channels::IDENTIFIER, 0), | |
| ("Split Channels", graphene_std::raster_nodes::adjustments::split_channels::IDENTIFIER, 1), |
| let (vec2, attributes) = vec2.into_parts(); | ||
|
|
||
| Vec2Components { | ||
| x: Item::from_parts(vec2.x, attributes.clone()), |
There was a problem hiding this comment.
P2: Split Vec2 copies the input attributes to both components, but combine_vec2 rebuilds the vector with Item::new_from_element, discarding them. A Split → Combine round trip therefore loses metadata such as transforms; preserve the shared component attributes in combine_vec2.
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 node-graph/nodes/math/src/lib.rs, line 1774:
<comment>`Split Vec2` copies the input attributes to both components, but `combine_vec2` rebuilds the vector with `Item::new_from_element`, discarding them. A Split → Combine round trip therefore loses metadata such as transforms; preserve the shared component attributes in `combine_vec2`.</comment>
<file context>
@@ -1754,6 +1754,28 @@ fn combine_vec2(
+ let (vec2, attributes) = vec2.into_parts();
+
+ Vec2Components {
+ x: Item::from_parts(vec2.x, attributes.clone()),
+ y: Item::from_parts(vec2.y, attributes),
+ }
</file context>
| /// Separates an image into its red, green, blue, and alpha channels, each provided as a grayscale image. | ||
| #[cfg(feature = "std")] | ||
| #[node_macro::node(name("Split Channels"), category("Raster: Channels"), destructure_output)] | ||
| fn split_channels(_: impl Ctx, image: Item<Raster<CPU>>) -> ImageChannels { |
There was a problem hiding this comment.
P2: This removes the prior Color and Gradient implementations, but migration redirects every old Extract Channel node to this Raster-only node. Existing graphs extracting channels from colors or gradients become type-incompatible; preserve those implementations or migrate those cases separately.
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 node-graph/nodes/raster/src/adjustments.rs, line 158:
<comment>This removes the prior `Color` and `Gradient` implementations, but migration redirects every old Extract Channel node to this Raster-only node. Existing graphs extracting channels from colors or gradients become type-incompatible; preserve those implementations or migrate those cases separately.</comment>
<file context>
@@ -138,27 +138,47 @@ fn gamma_correction<T: Adjust<Color>>(
+/// Separates an image into its red, green, blue, and alpha channels, each provided as a grayscale image.
+#[cfg(feature = "std")]
+#[node_macro::node(name("Split Channels"), category("Raster: Channels"), destructure_output)]
+fn split_channels(_: impl Ctx, image: Item<Raster<CPU>>) -> ImageChannels {
+ let (image, attributes) = image.into_parts();
+ let (width, height) = (image.width, image.height);
</file context>
…rge 'Position/Tangent on Path' into 'Evaluate Path'
bfd2515 to
e94f081
Compare
No description provided.