Skip to content

Simple gpu airbrush - #4469

Open
timon-schelling wants to merge 5 commits into
brush-cache-prfrom
simple-gpu-airbrush-pr
Open

Simple gpu airbrush#4469
timon-schelling wants to merge 5 commits into
brush-cache-prfrom
simple-gpu-airbrush-pr

Conversation

@timon-schelling

Copy link
Copy Markdown
Member

No description provided.

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

11 issues found across 29 files

Confidence score: 1/5

  • node-graph/nodes/brush/src/airbrush/pipeline.rs can reject every scatter pass because ScatterUniforms is bound at 24 bytes instead of the required 32, and some adapters cannot create Scatter with blended R16Float attachments — pad the uniform and use or validate a blendable density format.
  • editor/src/messages/portfolio/document/graph_operation/graph_operation_message_handler.rs panics when brush_strokes is missing from the registry, turning invalid brush operations into an editor crash — replace the expect with a logged early return.
  • editor/src/messages/portfolio/document/document_message_handler.rs removes undo/redo history whenever .gdd saving is enabled, so reopening a saved document loses its editing history — preserve history for normal document saves.
  • node-graph/nodes/brush/src/airbrush/mod.rs can render no strokes for boxed Graphic::Graphic input and can panic on mismatched stroke channel lengths; traverse the boxed form and validate strokes before indexing their samples.
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/nodes/brush/src/airbrush/pipeline.rs">

<violation number="1" location="node-graph/nodes/brush/src/airbrush/pipeline.rs:14">
P1: On adapters where `R16Float` is not blendable, creating `Scatter` fails because both density attachments request blending. Use a guaranteed blendable density format or check the adapter’s format capabilities before enabling this pipeline.</violation>

<violation number="2" location="node-graph/nodes/brush/src/airbrush/pipeline.rs:24">
P1: The scatter draw binds a 24-byte buffer for a uniform block requiring 32-byte layout size, so wgpu validation can reject every scatter pass. Pad `ScatterUniforms` to 32 bytes and update its initializer.</violation>
</file>

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

<violation number="1" location="editor/src/messages/portfolio/document/graph_operation/graph_operation_message_handler.rs:884">
P1: When the brush node registry cannot resolve `brush_strokes`, this handler panics at `.expect(...)` instead of rejecting the brush operation. Handle the missing definition with a logged early return so an invalid registry state cannot crash the editor.

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

<file name="editor/src/messages/preferences/preferences_message_handler.rs">

<violation number="1" location="editor/src/messages/preferences/preferences_message_handler.rs:96">
P2: When loading preferences disables the brush tool while Brush is active, this refresh hides Brush in the shelf but keeps Brush active. Mirror the `PreferencesMessage::BrushTool` deactivation check in the load path before refreshing the shelf.</violation>
</file>

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

<violation number="1" location="editor/src/messages/portfolio/document/node_graph/node_properties.rs:289">
P2: When the brush-strokes node is selected, its `List<Stroke>` input now falls through to the unsupported-widget placeholder because this change removes the only brush-stroke summary widget. Restore an equivalent `Stroke`-list widget, or add the new list shape to the supported property dispatch.</violation>
</file>

<file name="node-graph/nodes/brush/src/airbrush/mod.rs">

<violation number="1" location="node-graph/nodes/brush/src/airbrush/mod.rs:37">
P2: When a stroke has a channel length different from its position count, the airbrush renderer panics while indexing `Channel::Samples`. Validate `stroke.is_valid()` before constructing `StyledStroke`, or otherwise skip invalid strokes.</violation>

<violation number="2" location="node-graph/nodes/brush/src/airbrush/mod.rs:44">
P2: When the input contains the boxed `Graphic::Graphic` form, `airbrush` drops the group and renders no contained strokes. Traverse `Graphic::Graphic(item)` through a one-item list before handling the other graphic variants.</violation>
</file>

<file name="node-graph/nodes/brush/src/airbrush/kernel.rs">

<violation number="1" location="node-graph/nodes/brush/src/airbrush/kernel.rs:61">
P2: If a cache bake or another operation panics while this guard is held, every later airbrush render panics at `unwrap()` because the mutex is poisoned. Handle the poisoned lock explicitly and bypass caching or recover the guard so a cache failure does not crash rendering.

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

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

<violation number="1" location="editor/src/messages/portfolio/document/document_message_handler.rs:1070">
P2: When `.gdd` saving is enabled, this drops the document's undo/redo history on every save. Reopening then bootstraps a flat registry instead of restoring the saved history; keep history enabled for normal document saves.</violation>
</file>

<file name="node-graph/nodes/brush/src/airbrush/stroke.rs">

<violation number="1" location="node-graph/nodes/brush/src/airbrush/stroke.rs:115">
P2: When pressure changes at an unchanged position after the first kept dab, this branch drops the new pressure-dependent dab. Emit a dab when the current sigma differs from `kept_last.sigma` instead of suppressing it unconditionally.</violation>
</file>

<file name="node-graph/nodes/brush/src/airbrush/region.rs">

<violation number="1" location="node-graph/nodes/brush/src/airbrush/region.rs:1">
P2: Custom agent: **PR title enforcement**

The PR title does not meet the required format: `Simple` is not an imperative leading verb, the title has only three words, and `gpu` should be `GPU`. Rename it to `Add a GPU airbrush`.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

use raster_types::Texture;
use wgpu_executor::{AsyncWgpuPipeline, Buffer, WgpuExecutor};

pub(super) const DENSITY_FORMAT: wgpu::TextureFormat = wgpu::TextureFormat::R16Float;

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: On adapters where R16Float is not blendable, creating Scatter fails because both density attachments request blending. Use a guaranteed blendable density format or check the adapter’s format capabilities before enabling this pipeline.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At node-graph/nodes/brush/src/airbrush/pipeline.rs, line 14:

<comment>On adapters where `R16Float` is not blendable, creating `Scatter` fails because both density attachments request blending. Use a guaranteed blendable density format or check the adapter’s format capabilities before enabling this pipeline.</comment>

<file context>
@@ -0,0 +1,543 @@
+use raster_types::Texture;
+use wgpu_executor::{AsyncWgpuPipeline, Buffer, WgpuExecutor};
+
+pub(super) const DENSITY_FORMAT: wgpu::TextureFormat = wgpu::TextureFormat::R16Float;
+pub(super) const COMPOSITE_FORMAT: wgpu::TextureFormat = wgpu::TextureFormat::Rgba16Float;
+
</file context>
Suggested change
pub(super) const DENSITY_FORMAT: wgpu::TextureFormat = wgpu::TextureFormat::R16Float;
pub(super) const DENSITY_FORMAT: wgpu::TextureFormat = wgpu::TextureFormat::Rgba16Float;

kernel_scale: f32,
kernel_exponent: f32,
kernel_section_scale: f32,
_pad: f32,

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: The scatter draw binds a 24-byte buffer for a uniform block requiring 32-byte layout size, so wgpu validation can reject every scatter pass. Pad ScatterUniforms to 32 bytes and update its initializer.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At node-graph/nodes/brush/src/airbrush/pipeline.rs, line 24:

<comment>The scatter draw binds a 24-byte buffer for a uniform block requiring 32-byte layout size, so wgpu validation can reject every scatter pass. Pad `ScatterUniforms` to 32 bytes and update its initializer.</comment>

<file context>
@@ -0,0 +1,543 @@
+	kernel_scale: f32,
+	kernel_exponent: f32,
+	kernel_section_scale: f32,
+	_pad: f32,
+}
+
</file context>

Comment on lines +884 to +885
let strokes_node = resolve_proto_node_type(graphene_std::brush::brush_strokes::IDENTIFIER)
.expect("Brush strokes node does not exist")

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: When the brush node registry cannot resolve brush_strokes, this handler panics at .expect(...) instead of rejecting the brush operation. Handle the missing definition with a logged early return so an invalid registry state cannot crash the editor.

(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 editor/src/messages/portfolio/document/graph_operation/graph_operation_message_handler.rs, line 884:

<comment>When the brush node registry cannot resolve `brush_strokes`, this handler panics at `.expect(...)` instead of rejecting the brush operation. Handle the missing definition with a logged early return so an invalid registry state cannot crash the editor.

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

<file context>
@@ -854,6 +880,20 @@ fn import_usvg_node_inner(
 }
 
+fn insert_brush_strokes_chain(network_interface: &mut NodeNetworkInterface, layer: LayerNodeIdentifier, strokes_node_id: NodeId, color: Color, diameter: f64, hardness: f64, flow: f64) {
+	let strokes_node = resolve_proto_node_type(graphene_std::brush::brush_strokes::IDENTIFIER)
+		.expect("Brush strokes node does not exist")
+		.node_template_input_override([
</file context>
Suggested change
let strokes_node = resolve_proto_node_type(graphene_std::brush::brush_strokes::IDENTIFIER)
.expect("Brush strokes node does not exist")
let Some(strokes_node_definition) = resolve_proto_node_type(graphene_std::brush::brush_strokes::IDENTIFIER) else {
log::error!("Brush strokes node does not exist");
return;
};
let strokes_node = strokes_node_definition

zoom_with_scroll: self.zoom_with_scroll,
});
responses.add(FrontendMessage::UpdateUIScale { scale: self.ui_scale });
responses.add(ToolMessage::RefreshToolShelf);

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: When loading preferences disables the brush tool while Brush is active, this refresh hides Brush in the shelf but keeps Brush active. Mirror the PreferencesMessage::BrushTool deactivation check in the load path before refreshing the shelf.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At editor/src/messages/preferences/preferences_message_handler.rs, line 96:

<comment>When loading preferences disables the brush tool while Brush is active, this refresh hides Brush in the shelf but keeps Brush active. Mirror the `PreferencesMessage::BrushTool` deactivation check in the load path before refreshing the shelf.</comment>

<file context>
@@ -93,6 +93,7 @@ impl MessageHandler<PreferencesMessage, PreferencesMessageContext<'_>> for Prefe
 					zoom_with_scroll: self.zoom_with_scroll,
 				});
 				responses.add(FrontendMessage::UpdateUIScale { scale: self.ui_scale });
+				responses.add(ToolMessage::RefreshToolShelf);
 			}
 			PreferencesMessage::ResetToDefaults => {
</file context>
Suggested change
responses.add(ToolMessage::RefreshToolShelf);
if !self.brush_tool && tool_message_handler.tool_state.tool_data.active_tool_type == ToolType::Brush {
responses.add(ToolMessage::ActivateToolSelect);
}
responses.add(ToolMessage::RefreshToolShelf);

Some(x) if id_is::<DAffine2>(x) => transform_widget(default_info, &mut extra_widgets),
Some(x) if id_is::<Color>(x) => color_widget(default_info, ColorInput::default().allow_none(false)),
Some(x) if id_is::<Gradient>(x) => color_widget(default_info, ColorInput::default().allow_none(false)),
Some(x) if id_is::<BrushTrace>(x) => brush_strokes_widget(default_info).into(),

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: When the brush-strokes node is selected, its List<Stroke> input now falls through to the unsupported-widget placeholder because this change removes the only brush-stroke summary widget. Restore an equivalent Stroke-list widget, or add the new list shape to the supported property dispatch.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At editor/src/messages/portfolio/document/node_graph/node_properties.rs, line 289:

<comment>When the brush-strokes node is selected, its `List<Stroke>` input now falls through to the unsupported-widget placeholder because this change removes the only brush-stroke summary widget. Restore an equivalent `Stroke`-list widget, or add the new list shape to the supported property dispatch.</comment>

<file context>
@@ -286,7 +285,6 @@ pub(crate) fn property_from_type(
 						Some(x) if id_is::<Gradient>(x) => color_widget(default_info, ColorInput::default().allow_none(false)),
-						Some(x) if id_is::<BrushTrace>(x) => brush_strokes_widget(default_info).into(),
 						// ============
 						// STRUCT TYPES
 						// ============
@@ -460,33 +458,6 @@ pub fn vector_modification_widget(parameter_widgets_info: ParameterWidgetsInfo)
</file context>

let sharpest = (EDGE_WIDTH_FACTOR * sigma_texels / (2. * MIN_EDGE_TEXELS)).max(1.);
let exponent = (SOFTEST * (HARDEST / SOFTEST).powf(stroke.hardness.clamp(0., 1.))).min(sharpest);
let key = (exponent.ln() * KEY_STEPS_PER_LN).round() as i32;
let mut entries = self.entries.lock().unwrap();

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: If a cache bake or another operation panics while this guard is held, every later airbrush render panics at unwrap() because the mutex is poisoned. Handle the poisoned lock explicitly and bypass caching or recover the guard so a cache failure does not crash rendering.

(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/nodes/brush/src/airbrush/kernel.rs, line 61:

<comment>If a cache bake or another operation panics while this guard is held, every later airbrush render panics at `unwrap()` because the mutex is poisoned. Handle the poisoned lock explicitly and bypass caching or recover the guard so a cache failure does not crash rendering.

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

<file context>
@@ -0,0 +1,163 @@
+		let sharpest = (EDGE_WIDTH_FACTOR * sigma_texels / (2. * MIN_EDGE_TEXELS)).max(1.);
+		let exponent = (SOFTEST * (HARDEST / SOFTEST).powf(stroke.hardness.clamp(0., 1.))).min(sharpest);
+		let key = (exponent.ln() * KEY_STEPS_PER_LN).round() as i32;
+		let mut entries = self.entries.lock().unwrap();
+		if let Some(index) = entries.iter().position(|(cached, _)| *cached == key) {
+			if let Some(texture) = entries[index].1.texture.upgrade() {
</file context>
Suggested change
let mut entries = self.entries.lock().unwrap();
let Ok(mut entries) = self.entries.lock() else {
return bake(executor, (key as f64 / KEY_STEPS_PER_LN).exp());
};

document_format::ExportFormat::Xz,
document_format::ExportOptions::default(),
document_format::ExportOptions {
include_history: 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.

P2: When .gdd saving is enabled, this drops the document's undo/redo history on every save. Reopening then bootstraps a flat registry instead of restoring the saved history; keep history enabled for normal document saves.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At editor/src/messages/portfolio/document/document_message_handler.rs, line 1070:

<comment>When `.gdd` saving is enabled, this drops the document's undo/redo history on every save. Reopening then bootstraps a flat registry instead of restoring the saved history; keep history enabled for normal document saves.</comment>

<file context>
@@ -1066,7 +1066,10 @@ impl MessageHandler<DocumentMessage, DocumentMessageContext<'_>> for DocumentMes
 								document_format::ExportFormat::Xz,
-								document_format::ExportOptions::default(),
+								document_format::ExportOptions {
+									include_history: false,
+									..Default::default()
+								},
</file context>
Suggested change
include_history: false,
include_history: true,

let hardness = item.attribute_cloned_or(ATTR_HARDNESS, crate::DEFAULT_HARDNESS / 100.);
let flow = item.attribute_cloned_or(ATTR_FLOW, crate::DEFAULT_FLOW / 100.);
match item.into_element() {
Graphic::StrokeList(list) => strokes.extend(list.into_iter().map(Item::into_element).filter(|stroke| !stroke.is_empty()).map(|stroke| stroke::StyledStroke {

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: When a stroke has a channel length different from its position count, the airbrush renderer panics while indexing Channel::Samples. Validate stroke.is_valid() before constructing StyledStroke, or otherwise skip invalid strokes.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At node-graph/nodes/brush/src/airbrush/mod.rs, line 37:

<comment>When a stroke has a channel length different from its position count, the airbrush renderer panics while indexing `Channel::Samples`. Validate `stroke.is_valid()` before constructing `StyledStroke`, or otherwise skip invalid strokes.</comment>

<file context>
@@ -0,0 +1,69 @@
+		let hardness = item.attribute_cloned_or(ATTR_HARDNESS, crate::DEFAULT_HARDNESS / 100.);
+		let flow = item.attribute_cloned_or(ATTR_FLOW, crate::DEFAULT_FLOW / 100.);
+		match item.into_element() {
+			Graphic::StrokeList(list) => strokes.extend(list.into_iter().map(Item::into_element).filter(|stroke| !stroke.is_empty()).map(|stroke| stroke::StyledStroke {
+				color,
+				diameter,
</file context>

let kept_last = self.kept_last?;
let dab = dab(&stroke.stroke.sample(stroke.stroke.len() - 1), stroke);
if dab.position == kept_last.position {
return (self.kept == 1).then_some((kept_last, kept_last));

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: When pressure changes at an unchanged position after the first kept dab, this branch drops the new pressure-dependent dab. Emit a dab when the current sigma differs from kept_last.sigma instead of suppressing it unconditionally.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At node-graph/nodes/brush/src/airbrush/stroke.rs, line 115:

<comment>When pressure changes at an unchanged position after the first kept dab, this branch drops the new pressure-dependent dab. Emit a dab when the current sigma differs from `kept_last.sigma` instead of suppressing it unconditionally.</comment>

<file context>
@@ -0,0 +1,222 @@
+		let kept_last = self.kept_last?;
+		let dab = dab(&stroke.stroke.sample(stroke.stroke.len() - 1), stroke);
+		if dab.position == kept_last.position {
+			return (self.kept == 1).then_some((kept_last, kept_last));
+		}
+		Some((kept_last, dab))
</file context>

@@ -0,0 +1,67 @@
use core_types::math::bbox::AxisAlignedBbox;

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

The PR title does not meet the required format: Simple is not an imperative leading verb, the title has only three words, and gpu should be GPU. Rename it to Add a GPU airbrush.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At node-graph/nodes/brush/src/airbrush/region.rs:

<comment>The PR title does not meet the required format: `Simple` is not an imperative leading verb, the title has only three words, and `gpu` should be `GPU`. Rename it to `Add a GPU airbrush`.</comment>

<file context>
@@ -0,0 +1,67 @@
+use core_types::math::bbox::AxisAlignedBbox;
+use core_types::transform::Footprint;
+use glam::{DAffine2, DVec2, UVec2};
+
+const MAX_RESOLUTION: u32 = 8192;
+
+const CROP_STEP: u32 = 256;
+
+#[derive(Clone, Copy, PartialEq)]
</file context>

@timon-schelling

timon-schelling commented Aug 21, 2026

Copy link
Copy Markdown
Member Author

!build desktop (Run ID 32505605578)

@github-actions

Copy link
Copy Markdown
📦 Mac Build Complete for 11aec5d
Download binary

@github-actions

Copy link
Copy Markdown
📦 Windows Build Complete for 11aec5d
Download binary

@github-actions

github-actions Bot commented Aug 21, 2026

Copy link
Copy Markdown
📦 Linux Build Complete for 11aec5d
Download binary
Download Flatpak

@timon-schelling

timon-schelling commented Aug 21, 2026

Copy link
Copy Markdown
Member Author

!build desktop (Run ID 32536948337)

@github-actions

Copy link
Copy Markdown
📦 Mac Build Complete for 6016bce
Download binary

@github-actions

github-actions Bot commented Aug 22, 2026

Copy link
Copy Markdown
📦 Linux Build Complete for 6016bce
Download binary
Download Flatpak

@github-actions

Copy link
Copy Markdown
📦 Windows Build Complete for 6016bce
Download binary

@timon-schelling

timon-schelling commented Aug 22, 2026

Copy link
Copy Markdown
Member Author

!build desktop (Run ID 32567085875)

@github-actions

Copy link
Copy Markdown
📦 Mac Build Complete for e8d0146
Download binary

@github-actions

github-actions Bot commented Aug 22, 2026

Copy link
Copy Markdown
📦 Linux Build Complete for e8d0146
Download binary
Download Flatpak

@github-actions

Copy link
Copy Markdown
📦 Windows Build Complete for e8d0146
Download binary

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