Repository navigation
Pencil: Live preview option in Pencil Tool Options - #4225
Conversation
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Pencil preview and release behavior crates/tools/src/draw2/gesture.rs, crates/engine/src/tests_draw2.rs |
GestureTool adds a default-on live_preview setting. When disabled for Pencil, it suppresses path previews during dragging and shows a thin-line overlay. Tests cover dragging, release, commit, undo, and the other tools’ preview behavior. |
Pencil options dialog and settings crates/tools/src/settings.rs, crates/ui-egui/src/dialogs/freehand.rs, crates/ui-egui/src/menus.rs |
Pencil settings include livePreview. The freehand options dialog displays and submits the checkbox when available. The tool.options command documentation describes the dialogs and their options. |
Priority:
Estimated code review effort: 2 (Simple) | ~12 minutes
Change: Feature · Severity of issue fixed: Medium
Sequence Diagram(s)
sequenceDiagram
actor User
participant FreehandOptionsDialog
participant GestureTool
participant Document
User->>FreehandOptionsDialog: Set Live preview and confirm
FreehandOptionsDialog->>GestureTool: Send livePreview through tool.setOption
User->>GestureTool: Drag Pencil
GestureTool->>User: Show path preview or thin-line overlay
User->>GestureTool: Release Pencil
GestureTool->>Document: Commit path
Possibly related issues
- Pencil: options to turn off live preview and live curve fitting while drawing #3796: The issue requests the Pencil live preview option; its separate live curve fitting request is outside this change.
Possibly related PRs
- storytold/vectorcraft#4221: Both changes affect Pencil stroke previews and release behavior in
GestureTool.
Suggested reviewers: echelon, dotcoocoo
Merge Risk: 
6cae5
-
The Pencil option appears ready to merge after normal checks; no concrete issue remains identified.
Security architecture risk: 6cae5
-
The change remains within the existing drawing and preferences flows. Disabling previews does not add privileges or bypass document-edit controls, and release, cancellation, and tool switching retain their existing transaction behavior.
Retained concerns
No architecture-level concerns identified.Security review details
-
Security Blast Radius
- inferred — The newly selectable behavior is bounded to Pencil preferences and drawing in the existing session. An input controlling this boolean can suppress intermediate previews but gains no additional document-command capability through the inspected flow.
Trust Boundaries and Controls
- observed — The dialog still restricts opening to its listed freehand tools and submits listed option fields through tool.setOption. The new value uses Pencil-specific boolean handling and the existing persistence allow-list; it does not create an alternate document-mutation path.
-
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 5 files. Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Title check The title "Pencil: Live preview option in Pencil Tool Options" matches the main change: a Live preview checkbox added to Pencil Tool Options, with the tool's preview behavior gated by the option. It i… Description check The description follows the template. The 'What and why' section explains the change, links issue #4224, and gives the reasoning for the thin-line behavior and the reason to turn Live preview off. The…Linked Issues check Issue #4224requires a Pencil-only Live preview option that is enabled by default and defers styled preview until release when disabled. The changes addlive_previewwith default-on behavior, expose…Out of Scope Changes check The changes stay within issue #4224. The gesture logic, settings, dialog, tests, and menu documentation support the Pencil option and its user-visible behavior. The Paintbrush-specific assertions veri…Full details: Title check
-
Explanation
The title "Pencil: Live preview option in Pencil Tool Options" matches the main change: a Live preview checkbox added to Pencil Tool Options, with the tool's preview behavior gated by the option. It is specific and readable. It is slightly redundant ("Pencil" appears twice), but it still conveys the primary change clearly.
Full details: Description check
-
Explanation
The description follows the template. The 'What and why' section explains the change, links issue
#4224, and gives the reasoning for the thin-line behavior and the reason to turn Live preview off. The 'How it was tested' section lists the added tests and the local test runs. The checklist is not ticked. Its items (cargo xtask ci, no panics in shipped code, clean-room, ASSETS.md) are not confirmed. The 'Every new asset has a row in ASSETS.md' item may not apply, since no new assets appear in the summary, but the author has not stated this. The description covers the required content, so the check passes. The author should still confirm the checklist items or mark them as not applicable before merge.
Full details: Linked Issues check
-
Explanation
Issue
#4224requires a Pencil-only Live preview option that is enabled by default and defers styled preview until release when disabled. The changes addlive_previewwith default-on behavior, expose it aslivePreview, suppress live path previews in the disabled mode, show the thin-line overlay, and commit the filled path on release. The dialog places the checkbox before Live curve fitting. The reported engine, tools, dialog, settings, and catalog tests cover the required behavior. Paintbrush behavior remains unchanged.
Full details: Out of Scope Changes check
-
Explanation
The changes stay within issue
#4224. The gesture logic, settings, dialog, tests, and menu documentation support the Pencil option and its user-visible behavior. The Paintbrush-specific assertions verify that unrelated tool behavior remains unchanged. No unrelated product behavior is identified.
-
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
A Pencil stroke always previewed as the path it draws, with its full appearance: with a fill set, the fill flipped between the shape and its inside-out as the stroke crossed itself or curved back, and hid the line being drawn. Pencil Tool Options get a Live preview checkbox, above Live curve fitting, on by default and kept with the tool's other options (`livePreview`, settings.rs). With it off, the stroke previews nothing while it is drawn: a thin line follows the pen (the drag overlay the Knife and Blob Brush already show), and the release, or a switch of tool mid-stroke, draws the path with its fill and stroke, as one undo step, as before. Escape still cancels the stroke. With it on nothing changes. The Paintbrush has no such option and keeps previewing. Public reference: Draw and edit paths using the Pencil tool, https://helpx.adobe.com/illustrator/using/tool-techniques/pencil-tool.html (lists Live preview and Live curve fitting among the Pencil's options). The checkbox label is translated in all 24 catalogs. Tests: pencil_with_live_preview_off_shows_a_thin_line_until_the_release (tools), pencil_with_live_preview_off_draws_the_filled_path_on_release (engine), and the freehand dialog test shows the checkbox for the Pencil only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
6cae56f to
21e9c8f
Compare
|
Merged, thanks @ncic! On by default, so nothing changes until someone turns it off. |
Fixes #4224 (split from #3796, whose other half, Live curve fitting, landed in #4182).
Pencil Tool Options get a Live preview checkbox, next to Live curve fitting, as the public help page lists them. It is on by default, and on, nothing changes.
With it off, the stroke previews nothing while it is drawn. A thin line follows the pen instead, the same drag line the Knife and the Blob Brush already show, and the release draws the path with its fill and stroke, as one undo step, as before. Escape still cancels the stroke. The help page only says that Live preview shows the stroke's styles while it is drawn; showing a thin line while it is off is my choice, so that the line being drawn stays visible.
Live preview is useful, but not all the time. Its natural side effects get in the way while drawing: a filled stroke flips inside out as it crosses itself or curves back, fills and strokes overlap the line being drawn, and a redraw of a selected path changes the shape under the pen before it is lifted. That's why many people, me included, draw with it off most of the time, in this line mode, to see the stroke clearly without distractions.
Off, the line shows exactly where the pen went. With Live preview on, the path is refitted at every move while it is drawn, so it wobbles and shows small bumps that aren't in the hand's movement; for accurate drawing that is the most distracting part, and the thin line doesn't have it.
What changes
livePreview, Pencil only, kept with the tool's other options insettings.rs). The Paintbrush has no such option and keeps previewing.Tests
pencil_with_live_preview_off_shows_a_thin_line_until_the_release(tools): no previews and a thin line while drawing with it off, previews and no line with it on, the release draws the path, Escape cancels, the Paintbrush keeps previewing.pencil_with_live_preview_off_draws_the_filled_path_on_release(engine): nothing is added while drawing, the release draws the filled path as one undo step.Locally the tools tests, the engine Pencil tests and the dialog, catalog and settings tests pass; GitHub's checks here run the rest.
Worked together with Claude Code Opus 5.5 High ❤️
Summary by CodeRabbit