feat(editor): add adjustable pen smoothing - #98
Conversation
c82c724 to
a15a795
Compare
a15a795 to
d48ecfd
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Alt+wheel has two behavioral regressions involving rectangle selection feedback and pen-default adjustment while another layer remains selected.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds adjustable, undoable post-stroke smoothing to the freehand pen.
Changes:
- Implements bounded RDP/Chaikin smoothing levels.
- Adds editor controls, persistence, transforms, and rendering support.
- Adds documentation and end-to-end smoke coverage.
File summaries
| File | Description |
|---|---|
src/stroke-smoothing.hpp |
Declares smoothing levels and API. |
src/stroke-smoothing.cpp |
Implements bounded stroke smoothing. |
src/editor.hpp |
Stores the active smoothing level. |
src/editor.cpp |
Integrates smoothing with editing and history. |
src/capture.hpp |
Adds raw geometry and smoothing metadata. |
src/capture.cpp |
Persists and renders smoothed strokes. |
tests/stroke-smoothing-smoke.hpp |
Declares the smoke test. |
tests/stroke-smoothing-smoke.cpp |
Tests smoothing and editor integration. |
tests/editor-smoke.cpp |
Registers the new smoke test. |
README.md |
Documents smoothing controls. |
CMakeLists.txt |
Adds implementation and tests to builds. |
Review details
- Files reviewed: 11/11 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| } | ||
| if (annotation.kind != Annotation::Kind::Spotlight) | ||
| if (annotation.kind == Annotation::Kind::Spotlight) { | ||
| beginSelectionAdjust(); |
There was a problem hiding this comment.
🟡 Changes recommended
Synchronous smoothing can stall the UI, and loaded smoothing levels are not range-validated.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
src/editor.cpp:5114
- This smoothing pass runs synchronously inside
mouseReleaseEvent. The adversarial path can leave all 2,048 samples for the iterative RDP pass, which still has quadratic worst-case work, so releasing a zig-zag stroke can stall the visible overlay contrary todocs/threading.md:5-8. Please move the pass to aQtConcurrent/QFutureWatcherflow or use an algorithm with a frame-bounded worst case before committing the stroke.
- Files reviewed: 11/11 changed files
- Comments generated: 1
- Review effort level: Balanced
Freehand strokes now use adjustable release-time smoothing, with a medium
3/6level by default. The live stroke follows the raw pointer input;0/6preserves that exact path, while higher levels clean up the finished path without moving its endpoints.Alt+wheelchanges smoothing for the selected stroke, or sets the level for the next stroke when nothing is selected. The setting remains undoable and is preserved in the operation log.Tested with
make check.