You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Brushes: art along a smooth curve keeps smooth edges - #7321
Art and pattern brushes bend their art along the path through Track::frame (crates/brush/src/track.rs), and scatter brushes place and turn their copies with it. The frame put each point on a chord of the flattened path and blended the tangent linearly between vertices. On a smooth curve the bent art's edges followed those chords, so the curve fitted to the result turned at its smooth anchors. With a straight-sided Stretch art brush on a half circle, the edges turned by 0.8° to 6.2° at those anchors (radius 2,000 pt to 30 pt).
On a chord that comes from flattening a curve, the frame follows the cubic through both vertices with their tangents, so position and tangent are continuous at every vertex. A vertex tangent between two such chords is the tangent of the circle through the vertex and its neighbors. It is held within an angle of about 6 × the flattening tolerance ÷ the chord's length of each chord, so beside a long chord (for example a straight segment drawn as a curve, next to a tight bend) the frame stays within about twice the tolerance of the chord. A curved chord whose end tangents turn further than that keeps to the chord, as before.
Art on a line of the path stays on the line, and a line keeps its own direction where it meets a curve.
A calligraphic nib that follows the pen samples between vertices on the same frame, so its outline keeps its distance from the curve (samples in crates/brush/src/calli.rs).
How it was tested
New tests, each shown failing without the part of the change it covers:
art_brush_edges_stay_smooth_along_a_curve (crates/brush/src/tests.rs): a Stretch art brush on half circles of radius 30, 100, 300 and 2,000 pt; no edge turns by 0.5° or more at a smooth anchor.
pen_driven_nib_outline_follows_the_curve_between_vertices (crates/brush/src/tests.rs): a pressure-driven round nib on a half circle; the outline stays within 0.05 pt of the nib's reach.
a_straight_curve_element_beside_a_tight_bend_stays_on_its_line (track.rs): a 200 pt straight cubic joined to a quarter circle of radius 5, either way round; the frame stays within 0.1 pt of the line, and its tangent's component across the line stays under 0.01.
a_long_curve_flattened_coarser_is_still_followed (track.rs): a quarter circle of radius 5,000,000 pt, which flattening gives a larger tolerance; the frame stays within 0.05 pt of the curve.
a_curve_of_no_length_leaves_a_line_a_line (track.rs): a curve of no length between two lines leaves both lines.
New tests that pin the lines and seams: a_polylines_frame_stays_on_its_lines, a_line_into_a_smooth_curve_keeps_its_direction, a_closed_curves_frame_is_smooth_across_its_seam and an_implicit_closing_line_is_not_a_curve (track.rs).
Focused checks on x86-64 Linux. The branch is one commit on main at 4558196e16 and merges cleanly with main at 28bd9c9292. It shares crates/brush/src/calli.rs and crates/brush/src/tests.rs with open #1858 and crates/brush/src/tests.rs with open #7103. The commands run were:
No panics in shipped code: no unwrap(), expect(), panic!, unreachable!, todo! or unimplemented! outside tests, and no unchecked indexing on data from files, parameters or
messages. Errors are returned as Result and shown to the user
(why and how).
Clean-room: nothing copied from Adobe products or GPL/AGPL code (AGENTS.md).
Every new asset has a row in ASSETS.md.
Summary by CodeRabbit
Bug Fixes
Brush outlines now follow curved strokes more smoothly, improving nib placement and reducing sharp turns along curved paths.
Art and pattern brushes bent their art along the chords of the flattened
path, with the tangent blended linearly between vertices, and scatter
brushes placed and turned their copies on the same chords. On a smooth
curve the bent art's edges followed the chords, and the curve fitted to
the result turned by up to a few degrees at its smooth anchors.
- On a chord that comes from flattening a curve, the frame now follows
the cubic through both vertices with their tangents, so position and
tangent meet at every vertex. A vertex tangent between two such chords
is the tangent of the circle through the vertex and its neighbors, and
it stays within a few times the flattening tolerance of each chord, so
the frame stays near the curve next to a long chord. A curved chord
whose end tangents turn further than that keeps to the chord.
- Art on a line of the path stays on the line, and a line keeps its own
direction where it meets a curve.
- A calligraphic nib that follows the pen samples between vertices on
the same frame, so its outline keeps its distance from the curve.
No actionable comments were generated in the recent review.
Walkthrough
Track framing now retains curve provenance and interpolates frames along suitable curved segments. Calligraphic sampling uses those frames for curved segments. New tests check track framing and brush outlines along curves.
Changes: Curve-aware brush track framing
Layer / File(s)
Summary
Build curve-aware track data crates/brush/src/track.rs
Track retains curve metadata from flattened paths. It uses curve provenance and segment lengths to compute tangent limits and endpoint tangents.
Frame curved segments and use them in sampling crates/brush/src/track.rs, crates/brush/src/calli.rs, crates/brush/src/tests.rs
frame_in interpolates positions and tangents on suitable curved segments, while line segments and unsuitable curve chords remain on their chords. Calligraphic sampling uses interpolated frames on curved segments. Tests check line and curve framing, joins, closed-track seams, and brush outlines.
This change makes art, pattern, scatter and pen-driven nib brushes follow flattened curves smoothly while keeping straight-line behavior. Review found no concrete regression, and the new tests cover the changed behavior. Merge risk is minimal, pending the author's own CI run.
Pre-merge checks: 4
Check name
Status
Explanation
Title check
The title "Brushes: art along a smooth curve keeps smooth edges" matches the main change. The PR changes the path frame (Track::frame_in and curve-following interpolation) so art brush edges stay smoo…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Description check
The description follows the repository template: it has a 'What and why' section explaining the frame change and its motivation, a 'How it was tested' section listing new tests and commands run, and a…
Full details: Title check
Explanation
The title "Brushes: art along a smooth curve keeps smooth edges" matches the main change. The PR changes the path frame (Track::frame_in and curve-following interpolation) so art brush edges stay smooth at vertices on curves. The new art-brush smoothness test and the nib sampling change are also covered by the title's focus on smooth curves.
Full details: Description check
Explanation
The description follows the repository template: it has a 'What and why' section explaining the frame change and its motivation, a 'How it was tested' section listing new tests and commands run, and a completed checklist. Some items are unverified: the author says the CI run passed on a specific commit, and a few checklist boxes are marked without independent confirmation. The objectives note that cargo xtask ci is not confirmed passing, which conflicts with the checklist. The description is detailed and specific, so the check passes.
Finishing Touches
Generate unit tests (beta)
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Reviewing files that changed from the base of the PR and between 28bd9c9 and 90be8fb.
Files selected for processing: 3
crates/brush/src/calli.rs
crates/brush/src/tests.rs
crates/brush/src/track.rs
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What and why
Art and pattern brushes bend their art along the path through
Track::frame(crates/brush/src/track.rs), and scatter brushes place and turn their copies with it. The frame put each point on a chord of the flattened path and blended the tangent linearly between vertices. On a smooth curve the bent art's edges followed those chords, so the curve fitted to the result turned at its smooth anchors. With a straight-sided Stretch art brush on a half circle, the edges turned by 0.8° to 6.2° at those anchors (radius 2,000 pt to 30 pt).samplesincrates/brush/src/calli.rs).How it was tested
New tests, each shown failing without the part of the change it covers:
art_brush_edges_stay_smooth_along_a_curve(crates/brush/src/tests.rs): a Stretch art brush on half circles of radius 30, 100, 300 and 2,000 pt; no edge turns by 0.5° or more at a smooth anchor.pen_driven_nib_outline_follows_the_curve_between_vertices(crates/brush/src/tests.rs): a pressure-driven round nib on a half circle; the outline stays within 0.05 pt of the nib's reach.a_straight_curve_element_beside_a_tight_bend_stays_on_its_line(track.rs): a 200 pt straight cubic joined to a quarter circle of radius 5, either way round; the frame stays within 0.1 pt of the line, and its tangent's component across the line stays under 0.01.a_long_curve_flattened_coarser_is_still_followed(track.rs): a quarter circle of radius 5,000,000 pt, which flattening gives a larger tolerance; the frame stays within 0.05 pt of the curve.a_curve_of_no_length_leaves_a_line_a_line(track.rs): a curve of no length between two lines leaves both lines.New tests that pin the lines and seams:
a_polylines_frame_stays_on_its_lines,a_line_into_a_smooth_curve_keeps_its_direction,a_closed_curves_frame_is_smooth_across_its_seamandan_implicit_closing_line_is_not_a_curve(track.rs).Focused checks on x86-64 Linux. The branch is one commit on
mainat4558196e16and merges cleanly withmainat28bd9c9292. It sharescrates/brush/src/calli.rsandcrates/brush/src/tests.rswith open #1858 andcrates/brush/src/tests.rswith open #7103. The commands run were:cargo fmt --all -- --checkcargo check --workspace --testscargo clippy -p vectorcraft-brush --all-targets -- -D warningscargo test -p vectorcraft-brush: 84 passed, 0 failed.cargo test -p vectorcraft-render -p vectorcraft-svg -p vectorcraft-pdf -p vectorcraft-eps -p vectorcraft-engine -p vectorcraft-tools brush: 277 passed, 0 failed.cargo xtask ciruns in this pull request's CI.The art, pattern and scatter paths share the frame; the measurements above are for art brushes and the pen-driven nib.
cargo xtask cipassed in this pull request's CI run on90be8fba18: https://github.com/storytold/vectorcraft/actions/runs/43525329385/job/132229541081Checklist
cargo xtask cipasses (fmt, clippy, tests, assets, brands, layers, wasm).unwrap(),expect(),panic!,unreachable!,todo!orunimplemented!outside tests, and no unchecked indexing on data from files, parameters ormessages. Errors are returned as
Resultand shown to the user(why and how).
ASSETS.md.Summary by CodeRabbit