From 9b07ec97634aa3725f9520b78f83b913f70ed949 Mon Sep 17 00:00:00 2001 From: Keavon Chambers Date: Mon, 27 Apr 2026 03:47:42 -0700 Subject: [PATCH] Migrate 'Sample Polylines' from a subgraph to a proto node --- .../node_graph/document_node_definitions.rs | 135 ------------------ .../messages/portfolio/document_migration.rs | 37 ++++- node-graph/nodes/gcore/src/animation.rs | 7 - .../nodes/gcore/src/context_modification.rs | 1 - node-graph/nodes/vector/src/vector_nodes.rs | 85 ++++------- 5 files changed, 60 insertions(+), 205 deletions(-) diff --git a/editor/src/messages/portfolio/document/node_graph/document_node_definitions.rs b/editor/src/messages/portfolio/document/node_graph/document_node_definitions.rs index ddf61f7a0c..793ba02910 100644 --- a/editor/src/messages/portfolio/document/node_graph/document_node_definitions.rs +++ b/editor/src/messages/portfolio/document/node_graph/document_node_definitions.rs @@ -1938,141 +1938,6 @@ fn document_node_definitions() -> HashMap), 0)], - implementation: DocumentNodeImplementation::ProtoNode(vector::subpath_segment_lengths::IDENTIFIER), - call_argument: generic!(T), - ..Default::default() - }, - DocumentNode { - inputs: vec![ - NodeInput::import(concrete!(Table), 0), - NodeInput::import(concrete!(vector::misc::PointSpacingType), 1), - NodeInput::import(concrete!(f64), 2), - NodeInput::import(concrete!(u32), 3), - NodeInput::import(concrete!(f64), 4), - NodeInput::import(concrete!(f64), 5), - NodeInput::import(concrete!(bool), 6), - NodeInput::node(NodeId(0), 0), - ], - implementation: DocumentNodeImplementation::ProtoNode(vector::sample_polyline::IDENTIFIER), - call_argument: generic!(T), - ..Default::default() - }, - DocumentNode { - inputs: vec![NodeInput::node(NodeId(1), 0)], - implementation: DocumentNodeImplementation::ProtoNode(memo::memo::IDENTIFIER), - call_argument: generic!(T), - ..Default::default() - }, - ] - .into_iter() - .enumerate() - .map(|(id, node)| (NodeId(id as u64), node)) - .collect(), - ..Default::default() - }), - inputs: vec![ - NodeInput::value(TaggedValue::Vector(Default::default()), true), - NodeInput::value(TaggedValue::PointSpacingType(Default::default()), false), - NodeInput::value(TaggedValue::F64(100.), false), - NodeInput::value(TaggedValue::U32(100), false), - NodeInput::value(TaggedValue::F64(0.), false), - NodeInput::value(TaggedValue::F64(0.), false), - NodeInput::value(TaggedValue::Bool(false), false), - ], - ..Default::default() - }, - persistent_node_metadata: DocumentNodePersistentMetadata { - network_metadata: Some(NodeNetworkMetadata { - persistent_metadata: NodeNetworkPersistentMetadata { - node_metadata: [ - DocumentNodeMetadata { - persistent_metadata: DocumentNodePersistentMetadata { - node_type_metadata: NodeTypePersistentMetadata::node(IVec2::new(0, 7)), - ..Default::default() - }, - ..Default::default() - }, - DocumentNodeMetadata { - persistent_metadata: DocumentNodePersistentMetadata { - node_type_metadata: NodeTypePersistentMetadata::node(IVec2::new(7, 0)), - ..Default::default() - }, - ..Default::default() - }, - DocumentNodeMetadata { - persistent_metadata: DocumentNodePersistentMetadata { - node_type_metadata: NodeTypePersistentMetadata::node(IVec2::new(14, 0)), - ..Default::default() - }, - ..Default::default() - }, - ] - .into_iter() - .enumerate() - .map(|(id, node)| (NodeId(id as u64), node)) - .collect(), - ..Default::default() - }, - ..Default::default() - }), - input_metadata: vec![ - ("Content", "The shape to be resampled and converted into a polyline.").into(), - ("Spacing", node_properties::SAMPLE_POLYLINE_DESCRIPTION_SPACING).into(), - InputMetadata::with_name_description_override( - "Separation", - node_properties::SAMPLE_POLYLINE_DESCRIPTION_SEPARATION, - WidgetOverride::Number(NumberInputSettings { - min: Some(0.), - unit: Some(" px".to_string()), - ..Default::default() - }), - ), - InputMetadata::with_name_description_override( - "Quantity", - node_properties::SAMPLE_POLYLINE_DESCRIPTION_QUANTITY, - WidgetOverride::Number(NumberInputSettings { - min: Some(2.), - is_integer: true, - ..Default::default() - }), - ), - InputMetadata::with_name_description_override( - "Start Offset", - node_properties::SAMPLE_POLYLINE_DESCRIPTION_START_OFFSET, - WidgetOverride::Number(NumberInputSettings { - min: Some(0.), - unit: Some(" px".to_string()), - ..Default::default() - }), - ), - InputMetadata::with_name_description_override( - "Stop Offset", - node_properties::SAMPLE_POLYLINE_DESCRIPTION_STOP_OFFSET, - WidgetOverride::Number(NumberInputSettings { - min: Some(0.), - unit: Some(" px".to_string()), - ..Default::default() - }), - ), - ("Adaptive Spacing", node_properties::SAMPLE_POLYLINE_DESCRIPTION_ADAPTIVE_SPACING).into(), - ], - output_names: vec!["Vector".to_string()], - ..Default::default() - }, - }, - description: Cow::Borrowed("Convert vector geometry into a polyline composed of evenly spaced points."), - properties: Some("sample_polyline_properties"), - }, DocumentNodeDefinition { identifier: "Scatter Points", category: "Vector: Modifier", diff --git a/editor/src/messages/portfolio/document_migration.rs b/editor/src/messages/portfolio/document_migration.rs index 629af6fcb8..5092f0e968 100644 --- a/editor/src/messages/portfolio/document_migration.rs +++ b/editor/src/messages/portfolio/document_migration.rs @@ -94,6 +94,9 @@ const NODE_REPLACEMENTS: &[NodeReplacement<'static>] = &[ "graphene_core::transform::FreezeRealTimeNode", "graphene_core::transform_nodes::BoundlessFootprintNode", "graphene_core::transform_nodes::FreezeRealTimeNode", + // `subpath_segment_lengths` was inlined into the `sample_polyline` proto; old "Sample Polyline" subnetworks pass through unchanged. + "graphene_core::vector::SubpathSegmentLengthsNode", + "core_types::vector::SubpathSegmentLengthsNode", ], }, NodeReplacement { @@ -946,10 +949,6 @@ const NODE_REPLACEMENTS: &[NodeReplacement<'static>] = &[ node: graphene_std::vector::stroke::IDENTIFIER, aliases: &["graphene_core::vector::StrokeNode"], }, - NodeReplacement { - node: graphene_std::vector::subpath_segment_lengths::IDENTIFIER, - aliases: &["graphene_core::vector::SubpathSegmentLengthsNode"], - }, NodeReplacement { node: graphene_std::vector::tangent_on_path::IDENTIFIER, aliases: &["graphene_core::vector::TangentOnPathNode"], @@ -1075,6 +1074,10 @@ pub fn document_migration_upgrades(document: &mut DocumentMessageHandler, reset_ } fn migrate_node(node_id: &NodeId, node: &DocumentNode, network_path: &[NodeId], document: &mut DocumentMessageHandler, reset_node_definitions_on_open: bool) -> Option<()> { + // Must run before the reset block below: a node referencing a removed catalog entry would otherwise abort + // `migrate_node` via the `?` on `resolve_document_node_type`, preventing subsequent migration blocks from running. + migrate_removed_catalog_definitions(node_id, node, network_path, document); + if reset_node_definitions_on_open && let Some(reference) = document.network_interface.reference(node_id, network_path) { let node_definition = resolve_document_node_type(&reference)?; document.network_interface.replace_implementation(node_id, network_path, &mut node_definition.default_node_template()); @@ -2026,6 +2029,32 @@ fn migrate_node(node_id: &NodeId, node: &DocumentNode, network_path: &[NodeId], Some(()) } +/// Migrates document nodes whose catalog definitions have been removed. +/// +/// This is called from `migrate_node` BEFORE its standard reset/migration logic, since that logic aborts when it can't +/// resolve the node's `reference` against the current catalog. Each block here detects a specific decommissioned +/// definition by its old reference name, swaps it to a still-supported implementation, and preserves the user's inputs. +/// After this runs, the node's reference resolves cleanly so the rest of `migrate_node` proceeds normally. +fn migrate_removed_catalog_definitions(node_id: &NodeId, node: &DocumentNode, network_path: &[NodeId], document: &mut DocumentMessageHandler) -> Option<()> { + // Collapse the legacy "Sample Polyline" wrapper network into the standalone `sample_polyline` proto. + // The proto now computes per-bezpath segment lengths inline, so the wrapper's separate `subpath_segment_lengths` + // and `Memo` nodes are no longer needed. The 7 user-facing inputs are positionally identical between the + // old wrapper and the new proto. + if let Some(DefinitionIdentifier::Network(name)) = document.network_interface.reference(node_id, network_path) + && name == "Sample Polyline" + && node.inputs.len() == 7 + { + let mut node_template = resolve_proto_node_type(graphene_std::vector::sample_polyline::IDENTIFIER)?.default_node_template(); + document.network_interface.replace_implementation(node_id, network_path, &mut node_template); + let old_inputs = document.network_interface.replace_inputs(node_id, network_path, &mut node_template)?; + for (index, input) in old_inputs.iter().take(7).enumerate() { + document.network_interface.set_input(&InputConnector::node(*node_id, index), input.clone(), network_path); + } + } + + Some(()) +} + #[cfg(test)] mod tests { use super::*; diff --git a/node-graph/nodes/gcore/src/animation.rs b/node-graph/nodes/gcore/src/animation.rs index 27bcc1b723..5b96eedb5f 100644 --- a/node-graph/nodes/gcore/src/animation.rs +++ b/node-graph/nodes/gcore/src/animation.rs @@ -1,6 +1,5 @@ use core_types::table::Table; use core_types::transform::Footprint; -use core_types::uuid::NodeId; use core_types::{CacheHash, CloneVarArgs, Color, Context, Ctx, ExtractAll, ExtractAnimationTime, ExtractPointerPosition, ExtractRealTime, OwnedContextImpl}; use glam::{DAffine2, DVec2}; use graphic_types::vector_types::GradientStops; @@ -74,10 +73,7 @@ async fn quantize_real_time( Context -> DAffine2, Context -> Footprint, Context -> DVec2, - Context -> Vec, - Context -> Vec, Context -> Vec, - Context -> Vec, Context -> Vec, Context -> Table, Context -> Table, @@ -117,10 +113,7 @@ async fn quantize_animation_time( Context -> DAffine2, Context -> Footprint, Context -> DVec2, - Context -> Vec, - Context -> Vec, Context -> Vec, - Context -> Vec, Context -> Vec, Context -> Table, Context -> Table, diff --git a/node-graph/nodes/gcore/src/context_modification.rs b/node-graph/nodes/gcore/src/context_modification.rs index 04dd91851f..0bd736f269 100644 --- a/node-graph/nodes/gcore/src/context_modification.rs +++ b/node-graph/nodes/gcore/src/context_modification.rs @@ -30,7 +30,6 @@ async fn context_modification( Context -> Option, Context -> Vec, Context -> Vec, - Context -> Vec, Context -> Vec, Context -> Table, Context -> Table, diff --git a/node-graph/nodes/vector/src/vector_nodes.rs b/node-graph/nodes/vector/src/vector_nodes.rs index 4263ef42c9..8229273374 100644 --- a/node-graph/nodes/vector/src/vector_nodes.rs +++ b/node-graph/nodes/vector/src/vector_nodes.rs @@ -1326,18 +1326,34 @@ pub async fn flatten_path(_: impl Ctx, #[implem } /// Convert vector geometry into a polyline composed of evenly spaced points. -#[node_macro::node(category(""), path(core_types::vector))] +#[node_macro::node(category("Vector: Modifier"), path(core_types::vector), properties("sample_polyline_properties"))] async fn sample_polyline( _: impl Ctx, content: Table, spacing: PointSpacingType, - #[unit(" px")] separation: f64, + #[default(100.)] + #[hard_min(0.)] + #[unit(" px")] + separation: f64, + #[default(100)] + #[hard_min(2)] quantity: u32, - #[unit(" px")] start_offset: f64, - #[unit(" px")] stop_offset: f64, + #[hard_min(0.)] + #[unit(" px")] + start_offset: f64, + #[hard_min(0.)] + #[unit(" px")] + stop_offset: f64, adaptive_spacing: bool, - subpath_segment_lengths: Vec, ) -> Table { + let pathseg_perimeter = |segment: PathSeg| { + if is_linear(segment) { + Line::new(segment.start(), segment.end()).perimeter(DEFAULT_ACCURACY) + } else { + segment.perimeter(DEFAULT_ACCURACY) + } + }; + content .into_iter() .map(|mut row| { @@ -1351,27 +1367,14 @@ async fn sample_polyline( // Transfer the stroke transform from the input vector content to the result. result.style.set_stroke_transform(row.attribute_cloned_or_default("transform")); - // Using `stroke_bezpath_iter` so that the `subpath_segment_lengths` is aligned to the segments of each bezpath. - // So we can index into `subpath_segment_lengths` to get the length of the segments. - // NOTE: `subpath_segment_lengths` has precalulated lengths with transformation applied. - let bezpaths = row.element().stroke_bezpath_iter(); - - // Keeps track of the index of the first segment of the next bezpath in order to get lengths of all segments. - let mut next_segment_index = 0; - - for local_bezpath in bezpaths { + for local_bezpath in row.element().stroke_bezpath_iter() { // Apply the transform to compute sample locations in world space (for correct distance-based spacing) let mut world_bezpath = local_bezpath.clone(); let transform_attribute: DAffine2 = row.attribute_cloned_or_default("transform"); world_bezpath.apply_affine(Affine::new(transform_attribute.to_cols_array())); - let segment_count = world_bezpath.segments().count(); - - // For the current bezpath we get its segment's length by calculating the start index and end index. - let current_bezpath_segments_length = &subpath_segment_lengths[next_segment_index..next_segment_index + segment_count]; - - // Increment the segment index by the number of segments in the current bezpath to calculate the next bezpath segment's length. - next_segment_index += segment_count; + // Per-segment perimeter lengths (transform-baked) for distance-based spacing + let segment_lengths: Vec = world_bezpath.segments().map(pathseg_perimeter).collect(); let amount = match spacing { PointSpacingType::Separation => separation, @@ -1380,9 +1383,7 @@ async fn sample_polyline( // Compute sample locations using world-space distances, then evaluate positions on the untransformed bezpath. // This avoids needing to invert the transform (which fails when the transform is singular, e.g. zero scale). - let Some((locations, was_closed)) = - bezpath_algorithms::compute_sample_locations(&world_bezpath, spacing, amount, start_offset, stop_offset, adaptive_spacing, current_bezpath_segments_length) - else { + let Some((locations, was_closed)) = bezpath_algorithms::compute_sample_locations(&world_bezpath, spacing, amount, start_offset, stop_offset, adaptive_spacing, &segment_lengths) else { continue; }; @@ -1827,32 +1828,6 @@ async fn poisson_disk_points( .collect() } -#[node_macro::node(category(""), path(core_types::vector))] -async fn subpath_segment_lengths(_: impl Ctx, content: Table) -> Vec { - let pathseg_perimeter = |segment: PathSeg| { - if is_linear(segment) { - Line::new(segment.start(), segment.end()).perimeter(DEFAULT_ACCURACY) - } else { - segment.perimeter(DEFAULT_ACCURACY) - } - }; - - content - .into_iter() - .flat_map(|vector| { - let transform: DAffine2 = vector.attribute_cloned_or_default("transform"); - vector - .element() - .stroke_bezpath_iter() - .flat_map(|mut bezpath| { - bezpath.apply_affine(Affine::new(transform.to_cols_array())); - bezpath.segments().map(pathseg_perimeter).collect::>() - }) - .collect::>() - }) - .collect() -} - #[node_macro::node(name("Spline"), category("Vector: Modifier"), path(core_types::vector))] async fn spline(_: impl Ctx, content: Table) -> Table { content @@ -3126,7 +3101,7 @@ mod test { #[tokio::test] async fn sample_polyline() { let path = BezPath::from_vec(vec![PathEl::MoveTo(Point::ZERO), PathEl::CurveTo(Point::ZERO, Point::new(100., 0.), Point::new(100., 0.))]); - let sample_polyline = super::sample_polyline(Footprint::default(), vector_node_from_bezpath(path), PointSpacingType::Separation, 30., 0, 0., 0., false, vec![100.]).await; + let sample_polyline = super::sample_polyline(Footprint::default(), vector_node_from_bezpath(path), PointSpacingType::Separation, 30., 0, 0., 0., false).await; let sample_polyline = sample_polyline.element(0).unwrap(); assert_eq!(sample_polyline.point_domain.positions().len(), 4); for (pos, expected) in sample_polyline.point_domain.positions().iter().zip([DVec2::X * 0., DVec2::X * 30., DVec2::X * 60., DVec2::X * 90.]) { @@ -3136,7 +3111,7 @@ mod test { #[tokio::test] async fn sample_polyline_adaptive_spacing() { let path = BezPath::from_vec(vec![PathEl::MoveTo(Point::ZERO), PathEl::CurveTo(Point::ZERO, Point::new(100., 0.), Point::new(100., 0.))]); - let sample_polyline = super::sample_polyline(Footprint::default(), vector_node_from_bezpath(path), PointSpacingType::Separation, 18., 0, 45., 10., true, vec![100.]).await; + let sample_polyline = super::sample_polyline(Footprint::default(), vector_node_from_bezpath(path), PointSpacingType::Separation, 18., 0, 45., 10., true).await; let sample_polyline = sample_polyline.element(0).unwrap(); assert_eq!(sample_polyline.point_domain.positions().len(), 4); for (pos, expected) in sample_polyline.point_domain.positions().iter().zip([DVec2::X * 45., DVec2::X * 60., DVec2::X * 75., DVec2::X * 90.]) { @@ -3163,12 +3138,6 @@ mod test { } } #[tokio::test] - async fn segment_lengths() { - let bezpath = BezPath::from_vec(vec![PathEl::MoveTo(Point::ZERO), PathEl::CurveTo(Point::ZERO, Point::new(100., 0.), Point::new(100., 0.))]); - let lengths = subpath_segment_lengths(Footprint::default(), vector_node_from_bezpath(bezpath)).await; - assert_eq!(lengths, vec![100.]); - } - #[tokio::test] async fn path_length() { let bezpath = Rect::new(100., 100., 201., 201.).to_path(DEFAULT_ACCURACY); let transform = DAffine2::from_scale(DVec2::new(2., 2.));