Skip to content

Spike - #4777

Draft
Keavon wants to merge 26 commits into
masterfrom
spike
Draft

Spike#4777
Keavon wants to merge 26 commits into
masterfrom
spike

Conversation

@Keavon

@Keavon Keavon commented Oct 23, 2026

Copy link
Copy Markdown
Member

No description provided.

Keavon added 25 commits October 22, 2026 23:14
@Keavon
Keavon marked this pull request as draft October 23, 2026 06:20

@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.

32 issues found and verified against the latest diff

Confidence score: 3/5

  • render_cache.rs can panic on large interactive viewports because they take the CPU-buffer path even when for_export is false. Restrict that conversion to exports and keep interactive requests out of the unreachable! arm.
  • render_cache.rs silently skips failed region readbacks, so an export can finish with transparent pixels where content should be. Propagate the readback failure instead.
  • effects.rs drops the paint input’s envelope transform in paint_overlay, so transformed paint can be placed incorrectly. Preserve the transform when converting the paint input.
  • vello_effects.wgsl passes the backdrop and paint in the wrong order for Vivid Light’s burn and dodge helpers, producing incorrect blending. Pass them in the helpers’ expected order.
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="node-graph/libraries/graphic-types/src/lib.rs">

<violation number="1" location="node-graph/libraries/graphic-types/src/lib.rs:1">
P2: Custom agent: **PR title enforcement**

“Spike” is not an imperative title verb. Replace it with an action-oriented title such as “Add a spike for …” or “Implement …”.</violation>
</file>

<file name="node-graph/libraries/wgpu-executor/src/vello_effects.wgsl">

<violation number="1" location="node-graph/libraries/wgpu-executor/src/vello_effects.wgsl:317">
P2: Vivid Light swaps the arguments to `color_burn` and `color_dodge`. Both helpers take `(backdrop, paint)`, but this call passes the backdrop in the paint slot, so the result is wrong for every input. Pass `backdrop` first: `color_burn(backdrop, 2. * paint)` and `color_dodge(backdrop, 2. * paint - 1.)`.</violation>
</file>

<file name="editor/src/messages/portfolio/document_migration.rs">

<violation number="1" location="editor/src/messages/portfolio/document_migration.rs:1815">
P2: This drops legacy 15-input Noise Pattern nodes whose first input is not a stored bool, such as a wired Clip input. The removed code migrated every 15-input node, so these stay unmigrated. Accept the 15-input shape as clip-first regardless of the value type.</violation>
</file>

<file name="node-graph/libraries/graphic-types/src/value_effects.rs">

<violation number="1" location="node-graph/libraries/graphic-types/src/value_effects.rs:56">
P2: The `ADJUSTED_GRADIENTS` lock is held across `gradient.map_ramp(...)`, which the comment above says takes thousands of evaluations. Every other caller then blocks on the global mutex, even for cache hits on unrelated gradients. Compute the resample without the lock: check the cache under the lock, drop it, resample, then re-lock to insert.</violation>
</file>

<file name="node-graph/nodes/graphic/src/graphic.rs">

<violation number="1" location="node-graph/nodes/graphic/src/graphic.rs:1041">
P2: `gradient_to_colors` adjusts only the stop colors, which is exact only for Stepped gradients. For Linear or Smooth gradients with nonlinear adjustments, the output stops misrepresent the adjusted ramp. Run the gradient through `value_effects::evaluate_gradient_effects` first, as `evaluate_gradient` and `gradient_map` do, then take `into_color_list()`. The doc comment should also mention that adjustments are baked into the output stops.</violation>
</file>

<file name="node-graph/nodes/raster/shaders/entrypoint/src/lib.rs">

<violation number="1" location="node-graph/nodes/raster/shaders/entrypoint/src/lib.rs:1">
P2: This doc comment describes a re-export that this PR removes. `lib.rs` no longer has `pub use raster_nodes::*;` and the entrypoint crate no longer depends on `raster-nodes`, so the comment is false. Remove it, or update it to describe what this crate now exports.</violation>
</file>

<file name="node-graph/libraries/wgpu-executor/src/vello_render.rs">

<violation number="1" location="node-graph/libraries/wgpu-executor/src/vello_render.rs:591">
P3: The doc comment says the other axis keeps 'the average' of its value, but `finite_path` never averages anything. It only replaces infinite coordinates with ±`UNBOUNDED_COORDINATE` and keeps finite ones as-is. Reword the comment to match.</violation>

<violation number="2" location="node-graph/libraries/wgpu-executor/src/vello_render.rs:747">
P2: The pixel cache ignores dimensions and alpha interpretation, so two image brushes sharing a blob can reuse an incompatible texture. Include the image metadata in `ImageKey` or key the complete image descriptor.</violation>
</file>

<file name="node-graph/nodes/raster/shaders/src/lib.rs">

<violation number="1" location="node-graph/nodes/raster/shaders/src/lib.rs:1">
P3: Gating `WGSL_SHADER` on `feature = "compile"` also removes the externally provided shader path when `compile` is off, even though `build.rs` supports that path. Consider gating on the env var instead, e.g. `#[cfg(any(feature = "compile", ...))]` or `option_env!`, so the non-compile external-path case still works.</violation>
</file>

<file name="editor/src/messages/portfolio/document/document_message.rs">

<violation number="1" location="editor/src/messages/portfolio/document/document_message.rs:314">
P3: `UpdateEffectsReach` is internal like its neighbours, but it lacks the `#[serde(skip)]` they carry, so it stays deserializable from JS input. Add `#[serde(skip)]` to match `UpdateVectorData` and `UpdateAppearanceAttributes`, and list it in the comment above them.</violation>
</file>

<file name="node-graph/libraries/raster-types/src/effect_kernels.rs">

<violation number="1" location="node-graph/libraries/raster-types/src/effect_kernels.rs:212">
P2: The inward band also covers the outside pixels. `distances_across_edge(.., true)` seeds from the outside, so those pixels have distance 0 and `edge_band` gives them coverage 1. Zero the coverage of distance-0 pixels when `inward` is set, and add an assertion that an outside pixel such as (1,10) stays 0.</violation>
</file>

<file name="node-graph/nodes/blending/src/effects.rs">

<violation number="1" location="node-graph/nodes/blending/src/effects.rs:347">
P2: `paint_overlay` drops the paint's envelope transform via `paint.into_element()`. A Transform applied to the paint input is written to the envelope's `ATTR_TRANSFORM`, and `fitted_paint` then sees no inner transform and stretches the paint across the content. Fold the envelope transform into the inner element before discarding the envelope, or pass the envelope through.</violation>

<violation number="2" location="node-graph/nodes/blending/src/effects.rs:351">
P2: Gradient and CPU-raster paints bypass effect evaluation before CPU sampling, so effects attached to a wired paint do not appear in the overlay. Evaluate supported paint variants before leaving them in the fallback branch.</violation>
</file>

<file name="node-graph/libraries/wgpu-executor/src/vello_render/tests.rs">

<violation number="1" location="node-graph/libraries/wgpu-executor/src/vello_render/tests.rs:95">
P3: This test is named `..._render_quickly`, but it only prints `duration` and never asserts on it, so a slow render still passes. Add the same `assert!(duration.as_secs_f64() < 2., ...)` check used by the astronomical tests.</violation>
</file>

<file name="node-graph/libraries/raster-types/src/raster_types.rs">

<violation number="1" location="node-graph/libraries/raster-types/src/raster_types.rs:188">
P2: `derived` holds the buffer's Mutex while `derive(self)` runs. Other derivations on the same pixels block for the whole computation, and a `derive` that calls `derived` on the same buffer deadlocks. Run `derive` outside the lock, then store its result.</violation>
</file>

<file name="node-graph/libraries/raster-types/src/parallel_rows.rs">

<violation number="1" location="node-graph/libraries/raster-types/src/parallel_rows.rs:44">
P3: The test only covers buffers whose length is an exact multiple of `width`, so the ragged final row on the threaded path is untested. Add a case where `pixels.len()` is `width * height + 1` (or a partial row) and assert that the final partial row is filled with its own index.</violation>
</file>

<file name="node-graph/nodes/gstd/src/render_cache.rs">

<violation number="1" location="node-graph/nodes/gstd/src/render_cache.rs:359">
P1: Large Vello viewports return a CPU `Buffer` regardless of `for_export`. Interactive requests set `for_export: false`, so `render_background` can hit its `unreachable!` arm; restrict this conversion to exports and use a texture-safe fallback for oversized interactive views.</violation>

<violation number="2" location="node-graph/nodes/gstd/src/render_cache.rs:622">
P2: A failed region readback is logged and skipped, so the export returns `Poll::Final` with transparent pixels where that region should be. Propagate the failure instead, e.g. record it in `error` and return `Poll::Fallback`, or return `Poll::Error`, so the caller sees an incomplete result.</violation>
</file>

<file name="node-graph/libraries/graphic-types/src/color_lookup_table.rs">

<violation number="1" location="node-graph/libraries/graphic-types/src/color_lookup_table.rs:17">
P2: With a fixed cap of 8 and LRU eviction, a document with 9 or more distinct LUT files re-parses every file on every evaluation, since each entry is evicted before it is read again. The per-node cache this replaces had no shared limit and did not thrash. Either size the cache from the number of Color Lookup nodes, or keep the parsed table on the node again.</violation>

<violation number="2" location="node-graph/libraries/graphic-types/src/color_lookup_table.rs:23">
P3: The static cache keeps up to 8 parsed tables alive for the process lifetime, even after the nodes that read them are deleted. Large 3D tables can be tens of MB each. The old per-node cache released them with the node. Consider clearing entries that no live node references, or accept the retention and note it in the doc comment.</violation>

<violation number="3" location="node-graph/libraries/graphic-types/src/color_lookup_table.rs:28">
P2: `parse_cached` holds the process-wide `PARSED_FILES` lock while `Lut::parse` runs. A slow parse of one file blocks cache hits for every other Color Lookup adjustment. Parse outside the lock, then re-lock only to push the result.</violation>
</file>

<file name="node-graph/libraries/vector-types/src/gradient.rs">

<violation number="1" location="node-graph/libraries/vector-types/src/gradient.rs:1485">
P2: `map_ramp` samples the ramp through `evaluator.evaluate`, which applies the spread before interpolating. For non-cyclic ramps with stops outside 0..1, the subdivision points that fall outside 0..1 get clamped or transparent colors instead of the true ramp color, and those points can reach the visible ramp. Sample the unspread linear ramp for out-of-range positions, or clamp the subdivision range to 0..1 for non-cyclic ramps.</violation>
</file>

<file name="editor/src/messages/tool/tool_messages/gradient_tool.rs">

<violation number="1" location="editor/src/messages/tool/tool_messages/gradient_tool.rs:425">
P3: Use the generated `StartCurvatureInput::INDEX` and `EndCurvatureInput::INDEX` instead of the literal `1` and `2`. The inspector and `gradient_form_set` already do this, so a reordered parameter would otherwise make this read silently classify the wrong input.</violation>
</file>

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

<violation number="1" location="editor/src/messages/portfolio/document/graph_operation/utility_types.rs:774">
P3: Use the existing `GradientForm::dials()` helper instead of re-matching the two dial constants. Then drop the now-unused `LINEAR_DIALS, RADIAL_DIALS` import.</violation>

<violation number="2" location="editor/src/messages/portfolio/document/graph_operation/utility_types.rs:778">
P3: The first curvature write should skip the rerender, since the second write queues the graph run. Passing `true` here matches the single-run pattern used elsewhere in this file.</violation>
</file>

<file name="node-graph/libraries/graphic-types/src/adjustment.rs">

<violation number="1" location="node-graph/libraries/graphic-types/src/adjustment.rs:240">
P2: `Posterize` levels is read back without a lower bound, so a stored `0` or `1` reaches the kernel and divides by `levels - 1 = 0`, producing NaN colors. Clamp it to the node's minimum of 2 after the visit.</violation>
</file>

<file name="frontend/wrapper/src/native_communication.rs">

<violation number="1" location="frontend/wrapper/src/native_communication.rs:140">
P3: This test checks only the encode side, so the name overstates what it covers. The native-side decode in `receive_native_message` has no test. Move the decode mapping into a helper that the test can call, or rename the test to describe encoding only.</violation>
</file>

<file name="node-graph/libraries/graphic-types/src/generator.rs">

<violation number="1" location="node-graph/libraries/graphic-types/src/generator.rs:254">
P2: The noise node accepts a 64-bit seed but this cast silently aliases every seed differing by a multiple of 2³². Preserve the full input when deriving the library seed, or constrain the node to the library's 32-bit seed range.</violation>
</file>

<file name="frontend/wrapper/src/helpers.rs">

<violation number="1" location="frontend/wrapper/src/helpers.rs:96">
P2: Native HMR/remounts can leave image placeholders blank: the frontend clears `window.imageCanvases`, but the native renderer still omits images already marked as sent, so this function has no pixels to recreate them. Clear the sent-image cache or force a full image resend when the native wrapper is reinitialized.</violation>
</file>

<file name="editor/src/node_graph_executor.rs">

<violation number="1" location="editor/src/node_graph_executor.rs:328">
P2: Export bounds now depend on cached `layer_effects_reach`, but the export path does not apply the current render output's effects metadata before calculating bounds. Exporting before a preview metadata update can clip effect overflow; compute bounds from the export output or update the document metadata first.</violation>
</file>

<file name="node-graph/libraries/graphic-types/src/adjustment_tables.rs">

<violation number="1" location="node-graph/libraries/graphic-types/src/adjustment_tables.rs:90">
P2: Per-channel runs containing alpha-changing adjustments are uploaded as curve passes, but the GPU curve shader discards zero-alpha pixels before evaluating the alpha curve. `MakeOpaque` and alpha curves therefore remain transparent on the GPU, diverging from the CPU behavior; handle alpha-changing passes without the zero-alpha early return or route them through a compatible CPU path.</violation>
</file>

<file name="node-graph/libraries/wgpu-executor/src/vello_effects.rs">

<violation number="1" location="node-graph/libraries/wgpu-executor/src/vello_effects.rs:202">
P2: This collapses non-uniform or sheared transforms to a geometric-mean scale, so blur and shadow extents are wrong per axis and can be clipped. Preserve axis-dependent effect scaling and use a conservative transformed reach for the offscreen region.</violation>
</file>

Tip: Get a Slack DM when cubic finishes reviewing your PRs. Connect Slack

Re-trigger cubic

let max_region_size = max_texture_size.min(wgpu_executor::MAX_LAYER_RENDER_SIZE);

// Only an export asks for an output too large for one texture, which is assembled on the CPU instead
if physical_resolution.x > max_texture_size || physical_resolution.y > max_texture_size {

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.

P1: Large Vello viewports return a CPU Buffer regardless of for_export. Interactive requests set for_export: false, so render_background can hit its unreachable! arm; restrict this conversion to exports and use a texture-safe fallback for oversized interactive views.

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/gstd/src/render_cache.rs, line 359:

<comment>Large Vello viewports return a CPU `Buffer` regardless of `for_export`. Interactive requests set `for_export: false`, so `render_background` can hit its `unreachable!` arm; restrict this conversion to exports and use a texture-safe fallback for oversized interactive views.</comment>

<file context>
@@ -356,10 +351,25 @@ fn render_output_cache(
+	let max_region_size = max_texture_size.min(wgpu_executor::MAX_LAYER_RENDER_SIZE);
+
+	// Only an export asks for an output too large for one texture, which is assembled on the CPU instead
+	if physical_resolution.x > max_texture_size || physical_resolution.y > max_texture_size {
+		let output = render_regions_to_buffer(
+			&viewport_bounds_device,
</file context>

@@ -1,16 +1,30 @@
pub mod adjustment;

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: Custom agent: PR title enforcement

“Spike” is not an imperative title verb. Replace it with an action-oriented title such as “Add a spike for …” or “Implement …”.

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/libraries/graphic-types/src/lib.rs, line 1:

<comment>“Spike” is not an imperative title verb. Replace it with an action-oriented title such as “Add a spike for …” or “Implement …”.</comment>

<file context>
@@ -1,16 +1,30 @@
+pub mod adjustment;
+pub mod adjustment_kernels;
+pub mod adjustment_tables;
</file context>

result = hard_light(backdrop, paint);
}
case 14u: {
result = select(color_dodge(2. * paint - 1., backdrop), color_burn(2. * paint, backdrop), paint <= 0.5);

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: Vivid Light swaps the arguments to color_burn and color_dodge. Both helpers take (backdrop, paint), but this call passes the backdrop in the paint slot, so the result is wrong for every input. Pass backdrop first: color_burn(backdrop, 2. * paint) and color_dodge(backdrop, 2. * paint - 1.).

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/libraries/wgpu-executor/src/vello_effects.wgsl, line 317:

<comment>Vivid Light swaps the arguments to `color_burn` and `color_dodge`. Both helpers take `(backdrop, paint)`, but this call passes the backdrop in the paint slot, so the result is wrong for every input. Pass `backdrop` first: `color_burn(backdrop, 2. * paint)` and `color_dodge(backdrop, 2. * paint - 1.)`.</comment>

<file context>
@@ -0,0 +1,664 @@
+			result = hard_light(backdrop, paint);
+		}
+		case 14u: {
+			result = select(color_dodge(2. * paint - 1., backdrop), color_burn(2. * paint, backdrop), paint <= 0.5);
+		}
+		case 15u: {
</file context>
Suggested change
result = select(color_dodge(2. * paint - 1., backdrop), color_burn(2. * paint, backdrop), paint <= 0.5);
result = select(color_dodge(backdrop, 2. * paint - 1.), color_burn(backdrop, 2. * paint), paint <= 0.5);

let clipped = |clip_index: usize| !matches!(node.inputs.get(clip_index).and_then(|input| input.as_value()), Some(TaggedValue::Bool(false)));

if reference == DefinitionIdentifier::ProtoNode(graphene_std::raster_nodes::std_nodes::noise_pattern::IDENTIFIER) {
let starts_with_clip = matches!(node.inputs.first().and_then(|input| input.as_value()), Some(TaggedValue::Bool(_)));

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: This drops legacy 15-input Noise Pattern nodes whose first input is not a stored bool, such as a wired Clip input. The removed code migrated every 15-input node, so these stay unmigrated. Accept the 15-input shape as clip-first regardless of the value type.

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 1815:

<comment>This drops legacy 15-input Noise Pattern nodes whose first input is not a stored bool, such as a wired Clip input. The removed code migrated every 15-input node, so these stay unmigrated. Accept the 15-input shape as clip-first regardless of the value type.</comment>

<file context>
@@ -1717,6 +1800,52 @@ pub fn document_migration_upgrades(document: &mut DocumentMessageHandler, reset_
+			let clipped = |clip_index: usize| !matches!(node.inputs.get(clip_index).and_then(|input| input.as_value()), Some(TaggedValue::Bool(false)));
+
+			if reference == DefinitionIdentifier::ProtoNode(graphene_std::raster_nodes::std_nodes::noise_pattern::IDENTIFIER) {
+				let starts_with_clip = matches!(node.inputs.first().and_then(|input| input.as_value()), Some(TaggedValue::Bool(_)));
+				let clip_index = match node.inputs.len() {
+					16 => 1,
</file context>

let key = hasher.finish();

// A lock poisoned by a panic elsewhere still guards a usable cache
let mut cached = ADJUSTED_GRADIENTS.lock().unwrap_or_else(PoisonError::into_inner);

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: The ADJUSTED_GRADIENTS lock is held across gradient.map_ramp(...), which the comment above says takes thousands of evaluations. Every other caller then blocks on the global mutex, even for cache hits on unrelated gradients. Compute the resample without the lock: check the cache under the lock, drop it, resample, then re-lock to insert.

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/libraries/graphic-types/src/value_effects.rs, line 56:

<comment>The `ADJUSTED_GRADIENTS` lock is held across `gradient.map_ramp(...)`, which the comment above says takes thousands of evaluations. Every other caller then blocks on the global mutex, even for cache hits on unrelated gradients. Compute the resample without the lock: check the cache under the lock, drop it, resample, then re-lock to insert.</comment>

<file context>
@@ -0,0 +1,115 @@
+	let key = hasher.finish();
+
+	// A lock poisoned by a panic elsewhere still guards a usable cache
+	let mut cached = ADJUSTED_GRADIENTS.lock().unwrap_or_else(PoisonError::into_inner);
+	if let Some(index) = cached.iter().position(|(cached_key, ..)| *cached_key == key) {
+		let entry = cached.remove(index);
</file context>

return result.clone();
}
/// The files parsed most recently, least recent first, shared by every adjustment that reads a lookup table.
static PARSED_FILES: Mutex<Vec<(ResourceHash, ParsedLut)>> = Mutex::new(Vec::new());

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.

P3: The static cache keeps up to 8 parsed tables alive for the process lifetime, even after the nodes that read them are deleted. Large 3D tables can be tens of MB each. The old per-node cache released them with the node. Consider clearing entries that no live node references, or accept the retention and note it in the doc comment.

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/libraries/graphic-types/src/color_lookup_table.rs, line 23:

<comment>The static cache keeps up to 8 parsed tables alive for the process lifetime, even after the nodes that read them are deleted. Large 3D tables can be tens of MB each. The old per-node cache released them with the node. Consider clearing entries that no live node references, or accept the retention and note it in the doc comment.</comment>

<file context>
@@ -6,37 +6,40 @@ mod format_cube;
-			return result.clone();
-		}
+/// The files parsed most recently, least recent first, shared by every adjustment that reads a lookup table.
+static PARSED_FILES: Mutex<Vec<(ResourceHash, ParsedLut)>> = Mutex::new(Vec::new());
+
+/// [`Lut::parse`] for a resource, reusing the result while recent adjustments keep reading a file with the same content hash.
</file context>

Some(&TaggedValue::Number(value)) => value,
_ => 0.,
};
let is_radial = [curvature(1), curvature(2)] == graphene_std::renderer::dial_gradient::RADIAL_DIALS;

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.

P3: Use the generated StartCurvatureInput::INDEX and EndCurvatureInput::INDEX instead of the literal 1 and 2. The inspector and gradient_form_set already do this, so a reordered parameter would otherwise make this read silently classify the wrong input.

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/tool/tool_messages/gradient_tool.rs, line 425:

<comment>Use the generated `StartCurvatureInput::INDEX` and `EndCurvatureInput::INDEX` instead of the literal `1` and `2`. The inspector and `gradient_form_set` already do this, so a reordered parameter would otherwise make this read silently classify the wrong input.</comment>

<file context>
@@ -416,11 +416,14 @@ fn read_gradient_chain_state(layer: LayerNodeIdentifier, network_interface: &Nod
+				Some(&TaggedValue::Number(value)) => value,
+				_ => 0.,
+			};
+			let is_radial = [curvature(1), curvature(2)] == graphene_std::renderer::dial_gradient::RADIAL_DIALS;
+			gradient_form = Some(if is_radial { GradientForm::Radial } else { GradientForm::Linear });
 		}
</file context>
Suggested change
let is_radial = [curvature(1), curvature(2)] == graphene_std::renderer::dial_gradient::RADIAL_DIALS;
use graphene_std::math_nodes::gradient_curvature::{EndCurvatureInput, StartCurvatureInput};
let is_radial = [curvature(StartCurvatureInput::INDEX), curvature(EndCurvatureInput::INDEX)] == graphene_std::renderer::dial_gradient::RADIAL_DIALS;

GradientForm::Linear => LINEAR_DIALS,
GradientForm::Radial => RADIAL_DIALS,
};
self.set_input_with_refresh(InputConnector::node(node_id, StartCurvatureInput), NodeInput::value(TaggedValue::Number(start), false), false);

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.

P3: The first curvature write should skip the rerender, since the second write queues the graph run. Passing true here matches the single-run pattern used elsewhere in this file.

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/graph_operation/utility_types.rs, line 778:

<comment>The first curvature write should skip the rerender, since the second write queues the graph run. Passing `true` here matches the single-run pattern used elsewhere in this file.</comment>

<file context>
@@ -757,20 +757,26 @@ impl<'a> ModifyInputsContext<'a> {
+			GradientForm::Linear => LINEAR_DIALS,
+			GradientForm::Radial => RADIAL_DIALS,
+		};
+		self.set_input_with_refresh(InputConnector::node(node_id, StartCurvatureInput), NodeInput::value(TaggedValue::Number(start), false), false);
+		self.set_input_with_refresh(InputConnector::node(node_id, EndCurvatureInput), NodeInput::value(TaggedValue::Number(end), false), false);
 	}
</file context>
Suggested change
self.set_input_with_refresh(InputConnector::node(node_id, StartCurvatureInput), NodeInput::value(TaggedValue::Number(start), false), false);
self.set_input_with_refresh(InputConnector::node(node_id, StartCurvatureInput), NodeInput::value(TaggedValue::Number(start), false), true);


let input_connector = InputConnector::node(node_id, graphene_std::math_nodes::gradient_form::GradientFormInput);
self.set_input_with_refresh(input_connector, NodeInput::value(TaggedValue::GradientForm(gradient_form), false), false);
let [start, end] = match gradient_form {

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.

P3: Use the existing GradientForm::dials() helper instead of re-matching the two dial constants. Then drop the now-unused LINEAR_DIALS, RADIAL_DIALS import.

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/graph_operation/utility_types.rs, line 774:

<comment>Use the existing `GradientForm::dials()` helper instead of re-matching the two dial constants. Then drop the now-unused `LINEAR_DIALS, RADIAL_DIALS` import.</comment>

<file context>
@@ -757,20 +757,26 @@ impl<'a> ModifyInputsContext<'a> {
 
-		let input_connector = InputConnector::node(node_id, graphene_std::math_nodes::gradient_form::GradientFormInput);
-		self.set_input_with_refresh(input_connector, NodeInput::value(TaggedValue::GradientForm(gradient_form), false), false);
+		let [start, end] = match gradient_form {
+			GradientForm::Linear => LINEAR_DIALS,
+			GradientForm::Radial => RADIAL_DIALS,
</file context>

#[test]
fn rasterized_image_copy_matches_editor_shape() {
let editor_image = RasterizedImage {
fn image_data_crosses_to_the_native_frontend_intact() {

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.

P3: This test checks only the encode side, so the name overstates what it covers. The native-side decode in receive_native_message has no test. Move the decode mapping into a helper that the test can call, or rename the test to describe encoding only.

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 frontend/wrapper/src/native_communication.rs, line 140:

<comment>This test checks only the encode side, so the name overstates what it covers. The native-side decode in `receive_native_message` has no test. Move the decode mapping into a helper that the test can call, or rename the test to describe encoding only.</comment>

<file context>
@@ -97,37 +130,32 @@ pub fn send_message_to_cef(message: String) {
 	#[test]
-	fn rasterized_image_copy_matches_editor_shape() {
-		let editor_image = RasterizedImage {
+	fn image_data_crosses_to_the_native_frontend_intact() {
+		let image = RasterizedImage {
 			id: 7,
</file context>
Suggested change
fn image_data_crosses_to_the_native_frontend_intact() {
fn encode_frontend_messages_base64_encodes_image_pixels() {

This branch was successfully deployed

1 active deployment
graphite-dev (Preview) — 6172e584 Deployed Oct 23, 2026 by github-actions[bot]
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.

1 participant