Replace AnimationCurve curve value inputs during preprocessing

[Gavin-Niederman]

This PR adds a branch in the preprocessor that replaces any TaggedValue::AnimationCurve(curve) value inputs in the network new eval_curve nodes.

In the future, this will be used in the expose dialog to allow exposing inputs to the timeline.

This PR is a follow-up to #4164. Until that PR is merged, this one will include its commits.

Demo

untitled.mp4

[gemini-code-assist]

Code Review

This pull request introduces the graphene-animation crate, which implements animation primitives like Keyframe, InterpolationBehavior, and AnimationCurve for the Graphene node system. It integrates these curves into the node registry, adds an eval_curve node, and updates the preprocessor to replace AnimationCurve inputs with eval_curve nodes. The feedback highlights two critical issues: first, the preprocessor's node visitor skips preprocessing inputs on nested Network nodes due to an early continue; second, evaluating bezier curves with non-finite handle coordinates can lead to panics or infinite loops, requiring sanitization of the handles.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[timon-schelling]

Also needs a rebase

[timon-schelling]

I was not that involved in the discussion about this but it seems like this fits the usecase.
personally I would changes some names but I assume the naming is common for similar keyframe based animation systems (like blender)?

[timon-schelling]

!build (Run ID 36347898299)

[github-actions]

📦 Web Build Complete for 9b903a6
https://049ea985.graphite.pages.dev

Wasm: 40.07 MB — JS: 0.46 MB — CSS: 0.09 MB — Fonts: 0.30 MB — Images: 0.09 MB — All Assets: 41.01 MB

[cubic-dev-ai]

5 issues found across 12 files

Confidence score: 2/5

• node-graph/preprocessor/src/lib.rs can rewrite an existing eval_curve node’s curve input with an incompatible Item<f64> output, leaving the graph invalid; skip existing eval_curve nodes or preserve their input.
• node-graph/libraries/animation/src/lib.rs may let NaN handle coordinates reach evaluate, which can panic; validate every knot and handle coordinate before evaluation.
• node-graph/libraries/animation/Cargo.toml leaves serde configuration incomplete: default builds lack glam serde support, while no-default-feature builds fail because serde is used unconditionally; enable glam/serde and make serde required or gate its uses.
• node-graph/interpreted-executor/src/node_registry.rs cannot resolve a List<AnimationCurve> feeding a ListDyn connector without the erasure adapter; add AnimationCurve to the list-dyn adapter.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="node-graph/preprocessor/src/lib.rs">

<violation number="1" location="node-graph/preprocessor/src/lib.rs:82">
P1: This branch also rewrites the curve constant on an existing `eval_curve` node, feeding its `Item<AnimationCurve>` input another `eval_curve` node's `Item<f64>` output. Skip existing `eval_curve` nodes or preserve inputs that require an `AnimationCurve`.</violation>
</file>

<file name="node-graph/libraries/animation/Cargo.toml">

<violation number="1" location="node-graph/libraries/animation/Cargo.toml:15">
P2: This dependency is optional, but `animation/src/lib.rs` uses `serde` unconditionally, so `cargo check -p graphene-animation --no-default-features` cannot compile. Gate the serde-only uses or make `serde` required.</violation>

<violation number="2" location="node-graph/libraries/animation/Cargo.toml:23">
P2: This feature enables serde for this crate but not for `glam`, even though the animation types serialize `glam::DVec2`; a standalone default build therefore lacks the required serde implementations. Add `glam/serde` to this feature.</violation>
</file>

<file name="node-graph/interpreted-executor/src/node_registry.rs">

<violation number="1" location="node-graph/interpreted-executor/src/node_registry.rs:375">
P2: `AnimationCurve` is now registered as a ranked list type, but its `ListDyn` erasure adapter is missing. A `List<AnimationCurve>` feeding a `ListDyn` connector therefore cannot resolve; add `AnimationCurve` to `list_dyn_rows!`.</violation>
</file>

<file name="node-graph/libraries/animation/src/lib.rs">

<violation number="1" location="node-graph/libraries/animation/src/lib.rs:135">
P2: Checking only `knot.x` lets a Bezier handle with `right_handle.x = NaN` through; `evaluate` then panics when it uses NaN as the lower bound of `left_handle.x.clamp(...)`. Validate every knot and handle coordinate before storing or deserializing keyframes, and return an error instead of panicking.

(Based on your team's feedback about avoiding panics in application code.)</violation>
</file>

<sub>Tip: cubic used a learning from your PR history. Let your coding agent read cubic learnings directly with the cubic MCP.

Re-trigger cubic</sub>

[cubic-dev-ai]

P1: This branch also rewrites the curve constant on an existing eval_curve node, feeding its Item<AnimationCurve> input another eval_curve node's Item<f64> output. Skip existing eval_curve nodes or preserve inputs that require an AnimationCurve.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At node-graph/preprocessor/src/lib.rs, line 82:

<comment>This branch also rewrites the curve constant on an existing `eval_curve` node, feeding its `Item<AnimationCurve>` input another `eval_curve` node's `Item<f64>` output. Skip existing `eval_curve` nodes or preserve inputs that require an `AnimationCurve`.</comment>

<file context>
@@ -41,48 +41,66 @@ impl Preprocessor {
+							id
+						})
+					}
+					TaggedValue::AnimationCurve(curve) => {
+						let id = NodeId::new();
+						let curve_node = DocumentNode {
</file context>

[cubic-dev-ai]

P2: This dependency is optional, but animation/src/lib.rs uses serde unconditionally, so cargo check -p graphene-animation --no-default-features cannot compile. Gate the serde-only uses or make serde required.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At node-graph/libraries/animation/Cargo.toml, line 15:

<comment>This dependency is optional, but `animation/src/lib.rs` uses `serde` unconditionally, so `cargo check -p graphene-animation --no-default-features` cannot compile. Gate the serde-only uses or make `serde` required.</comment>

<file context>
@@ -0,0 +1,26 @@
+publish.workspace = true
+
+[dependencies]
+serde = { workspace = true, optional = true }
+kurbo = { workspace = true }
+glam = { workspace = true }
</file context>

[cubic-dev-ai]

P2: This feature enables serde for this crate but not for glam, even though the animation types serialize glam::DVec2; a standalone default build therefore lacks the required serde implementations. Add glam/serde to this feature.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At node-graph/libraries/animation/Cargo.toml, line 23:

<comment>This feature enables serde for this crate but not for `glam`, even though the animation types serialize `glam::DVec2`; a standalone default build therefore lacks the required serde implementations. Add `glam/serde` to this feature.</comment>

<file context>
@@ -0,0 +1,26 @@
+
+[features]
+default = ["serde"]
+serde = ["dep:serde"]
+
+[lints]
</file context>

Suggested change

	serde = ["dep:serde"]
	serde = ["dep:serde", "glam/serde"]

[cubic-dev-ai]

P2: AnimationCurve is now registered as a ranked list type, but its ListDyn erasure adapter is missing. A List<AnimationCurve> feeding a ListDyn connector therefore cannot resolve; add AnimationCurve to list_dyn_rows!.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At node-graph/interpreted-executor/src/node_registry.rs, line 375:

<comment>`AnimationCurve` is now registered as a ranked list type, but its `ListDyn` erasure adapter is missing. A `List<AnimationCurve>` feeding a `ListDyn` connector therefore cannot resolve; add `AnimationCurve` to `list_dyn_rows!`.</comment>

<file context>
@@ -371,6 +372,7 @@ fn node_registry() -> HashMap<ProtoNodeIdentifier, HashMap<NodeIOTypes, NodeCons
 				InterpolationDistribution,
 				RowsOrColumns,
 				Resource,
+				AnimationCurve,
 			)
 		};
</file context>

[cubic-dev-ai]

P2: Checking only knot.x lets a Bezier handle with right_handle.x = NaN through; evaluate then panics when it uses NaN as the lower bound of left_handle.x.clamp(...). Validate every knot and handle coordinate before storing or deserializing keyframes, and return an error instead of panicking.

(Based on your team's feedback about avoiding panics in application code.)

View Feedback

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At node-graph/libraries/animation/src/lib.rs, line 135:

<comment>Checking only `knot.x` lets a Bezier handle with `right_handle.x = NaN` through; `evaluate` then panics when it uses NaN as the lower bound of `left_handle.x.clamp(...)`. Validate every knot and handle coordinate before storing or deserializing keyframes, and return an error instead of panicking.

(Based on your team's feedback about avoiding panics in application code.) </comment>

<file context>
@@ -0,0 +1,267 @@
+	///
+	/// This method panics if a keyframe with a non-finite x-coordinate is given.
+	pub fn insert_keyframe(&mut self, keyframe: Keyframe) -> usize {
+		assert!(keyframe.knot.x.is_finite(), "Keyframes must have a finite x-coordinate");
+
+		match self.keyframes.binary_search_by(|kf| kf.knot.x.partial_cmp(&keyframe.knot.x).unwrap_or(std::cmp::Ordering::Equal)) {
</file context>

[timon-schelling]

LGTM, Thanks for the great work. Merging soon.