Skip to content

Add an Autoscale option to the Brush tool - #4485

Open
timon-schelling wants to merge 1 commit into
masterfrom
brush-tool-autoscale
Open

Add an Autoscale option to the Brush tool#4485
timon-schelling wants to merge 1 commit into
masterfrom
brush-tool-autoscale

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.

2 issues found across 1 file

Confidence score: 3/5

  • In editor/src/messages/tool/tool_messages/brush_tool.rs, toggling Autoscale can reinterpret an unchanged brush diameter in the wrong unit, causing the brush size to change unexpectedly; convert options.diameter using the current viewport zoom when switching modes.
  • In editor/src/messages/tool/tool_messages/brush_tool.rs, non-representable zoom values can make round-tripped diameters fail exact equality, creating a new brush group instead of matching the existing stroke; use a scale-aware floating-point comparison.
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="editor/src/messages/tool/tool_messages/brush_tool.rs">

<violation number="1" location="editor/src/messages/tool/tool_messages/brush_tool.rs:221">
P2: When the user toggles Autoscale with a brush layer selected, this branch reinterprets the unchanged diameter in the wrong unit. Convert `options.diameter` by the current viewport zoom when changing modes so the existing document-space diameter remains unchanged.</violation>

<violation number="2" location="editor/src/messages/tool/tool_messages/brush_tool.rs:387">
P2: When autoscale selection sync round-trips a diameter through a non-representable zoom, exact equality rejects the matching stroke and creates a new brush group. 比较 the diameters with a scale-aware floating-point tolerance.</violation>
</file>

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

Re-trigger cubic

let value = |index: usize| node.inputs.get(index).and_then(|input| input.as_value());
matches!(value(STROKES_COLOR_INPUT), Some(TaggedValue::Color(color)) if *color == options.active_color())
&& matches!(value(STROKES_DIAMETER_INPUT), Some(TaggedValue::F64(diameter)) if *diameter == options.diameter)
&& matches!(value(STROKES_DIAMETER_INPUT), Some(TaggedValue::F64(diameter)) if *diameter == options.stroke_diameter(document))

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 autoscale selection sync round-trips a diameter through a non-representable zoom, exact equality rejects the matching stroke and creates a new brush group. 比较 the diameters with a scale-aware floating-point tolerance.

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

<comment>When autoscale selection sync round-trips a diameter through a non-representable zoom, exact equality rejects the matching stroke and creates a new brush group. 比较 the diameters with a scale-aware floating-point tolerance.</comment>

<file context>
@@ -355,7 +384,7 @@ impl BrushToolData {
 		let value = |index: usize| node.inputs.get(index).and_then(|input| input.as_value());
 		matches!(value(STROKES_COLOR_INPUT), Some(TaggedValue::Color(color)) if *color == options.active_color())
-			&& matches!(value(STROKES_DIAMETER_INPUT), Some(TaggedValue::F64(diameter)) if *diameter == options.diameter)
+			&& matches!(value(STROKES_DIAMETER_INPUT), Some(TaggedValue::F64(diameter)) if *diameter == options.stroke_diameter(document))
 			&& matches!(value(STROKES_HARDNESS_INPUT), Some(TaggedValue::F64(hardness)) if *hardness == options.hardness)
 			&& matches!(value(STROKES_FLOW_INPUT), Some(TaggedValue::F64(flow)) if *flow == options.flow)
</file context>
Suggested change
&& matches!(value(STROKES_DIAMETER_INPUT), Some(TaggedValue::F64(diameter)) if *diameter == options.stroke_diameter(document))
&& matches!(value(STROKES_DIAMETER_INPUT), Some(TaggedValue::F64(diameter)) if (*diameter - options.stroke_diameter(document)).abs() <= f64::EPSILON * diameter.abs().max(options.stroke_diameter(document).abs()).max(1.))

responses.add(ToolMessage::SelectWorkingColor { color, primary: true });
}
}
BrushToolMessageOptionsUpdate::Autoscale(autoscale) => self.options.autoscale = autoscale,

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 user toggles Autoscale with a brush layer selected, this branch reinterprets the unchanged diameter in the wrong unit. Convert options.diameter by the current viewport zoom when changing modes so the existing document-space diameter remains unchanged.

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

<comment>When the user toggles Autoscale with a brush layer selected, this branch reinterprets the unchanged diameter in the wrong unit. Convert `options.diameter` by the current viewport zoom when changing modes so the existing document-space diameter remains unchanged.</comment>

<file context>
@@ -190,6 +218,7 @@ impl<'a> MessageHandler<ToolMessage, &mut ToolActionMessageContext<'a>> for Brus
 					responses.add(ToolMessage::SelectWorkingColor { color, primary: true });
 				}
 			}
+			BrushToolMessageOptionsUpdate::Autoscale(autoscale) => self.options.autoscale = autoscale,
 			BrushToolMessageOptionsUpdate::WorkingColorsChanged => {
 				self.options.color.fill_choice = Some(solid(context.global_tool_data.primary_color));
</file context>
Suggested change
BrushToolMessageOptionsUpdate::Autoscale(autoscale) => self.options.autoscale = autoscale,
BrushToolMessageOptionsUpdate::Autoscale(autoscale) => {
if self.options.autoscale != autoscale {
let zoom = viewport_zoom(context.document);
self.options.diameter = if autoscale { self.options.diameter * zoom } else { self.options.diameter / zoom };
}
self.options.autoscale = autoscale;
}

注册 for free to join this conversation on GitHub. Already have an account? 登录 to comment

标签

None yet

项目

None yet

Development

Successfully merging this pull request may close these issues.

1 participant