fix(text): char_* presets reach rich_text and gradient_text, plus a letter morph - #402
Merged
Merged
Conversation
The seven char_* presets were accepted by rich_text and gradient_text with no schema error, and then simply did nothing: both components painted the full, settled text from frame 0, silently ignoring props.char_animation entirely (rich_text never read it; gradient_text took it as a parameter and discarded it under an underscore). That's exactly the "declared thing that does nothing" defect this repo treats as the bug (see #339, #349, #364) — closes #363's first half. The per-preset paint logic (unit_progress/ink_paint/ apply_text_anim_preset/render_char_animation) moves out of text.rs into intrinsic.rs as shared, unmodified functions so all three text components can reuse it instead of re-implementing seven animation curves three times: - rich_text: word/char units now walk the token stream across span boundaries (stagger doesn't reset per span), each unit painting with its own span's font and color, so ink_from converges to that span's own color rather than a shared default. Pill backgrounds stay static underneath the animated glyphs. - gradient_text: the linear-gradient shader is still built once over the whole laid-out run before any per-unit split, so splitting into words/chars never restarts the ramp. ink_from has no visible effect here (a shader always wins over a flat Paint color) and is documented as a real limitation rather than a silent no-op — the other six knobs (position/scale/rotation/blur/alpha) all work. The tuning knobs from #363's second half (rotate_from, per-unit scale_jitter/baseline_jitter, animated reflow) are intentionally not added: they'd need ResolvedCharAnimation (engine/animator.rs) to carry new fields, and that file is owned by a concurrent workstream. Similarly, letter_spacing as an animatable property (#380 first half) needs a new AnimatedProperties field plus get/set wiring in the same file — also left alone rather than half-wired. Also implements #380's second half: `text.morph`, a letter-by-plus transition between `states` labels distinct from `swap`. `swap` (see text-polish.md) treats a label as one rigid block that rises and blurs; `morph` pairs identical characters between the outgoing and incoming label by longest common subsequence (left to right, same algorithm as a text diff) and slides each matched glyph from its old position to its new one, while unmatched glyphs fade (or, with `unmatched: "scramble"`, cycle through deterministic placeholder characters before settling). Exposed as a new `Text.morph` field rather than a `swap.mode`, because TextSwapConfig lives in schema/video.rs outside this change's file ownership (only the char_* preset structs and KNOWN_MOTION_PROPERTIES there); `TextMorphConfig`/ `TextMorphUnmatched` are defined directly in text.rs instead. Adds two French rule files matching the existing house style: rules/char-animation-rich-and-gradient-text.md and rules/text-morph.md. Adding `Text.morph` needed a new field on every exhaustive `Text { .. }` literal; two of those live in box_builder.rs's own test module (outside this change's owned files) and got the one-line `morph: None,` addition needed to keep the crate compiling — no logic there changed.
…tale 'text only' The preset table claimed char_* was text-only. That was true before this branch; it is the sentence a generator reads to decide not to try.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Closes #363 (the silent-ignore half). Refs #380, #388 — #380 is half-done on purpose, see below.
The fault that mattered
A
char_*preset onrich_textorgradient_textwas accepted and ignored. The same class as #339, #349 and #364, and the reason it was done before the tuning knobs in the same issue.The four shared helpers moved out of
text.rsintointrinsic.rsverbatim, and both other components now call them.rich_textwalks tokens across span boundaries, so the stagger does not restart at each span, and each unit paints with its own span's font and colour — soink_fromconverges to that span's colour rather than a shared default.gradient_texthad been discardingpropsentirely (_propsinpaint_content). The gradient shader is built over the whole laid-out run before any per-unit split, so splitting into words cannot restart the ramp — that is structural, not a flag.One knob is inert there and it is documented, not hidden:
ink_fromhas no visible effect ongradient_text, because a Skia shader always wins over a Paint's flat colour. The other six work.text.morph— and how it differs fromswaptext.states+swaptreats each label as one rigid block that rises and blurs.morphpairs identical characters by longest common subsequence, slides each matched glyph from its old position to its new one, and fades the unmatched ones — or cycles them through a deterministic placeholder sequence withunmatched: "scramble".It is a separate field rather than a
swap.modebecauseTextSwapConfiglives outside the workstream's files.Animatable
letter-spacingneeds a new field onAnimatedPropertiesinanimator.rs, which another workstream owned. The agent refused to add"letter_spacing"toKNOWN_MOTION_PROPERTIESon its own — that alone would make it validate and stay inert, reproducing the exact bug this PR is about.That was the right call, and it is why #380 stays open.
An honest note about one test
char_animation_never_restarts_the_gradient_ramp_per_unitpasses with the fix reverted. It asserts a property that also holds trivially when animation is disabled, so it is a real assertion but not a discriminating regression test. The no-restart claim rests on the code structure, not on that test. Flagged by the agent rather than counted as proof.A first version of the morph interpolation test had the same flaw — window-based ink detection that coincidentally overlapped the hard cut's own glyphs — and was caught and replaced with a comparative
assert_ne!.Verification
11 new tests.
cargo fmt --all --check,cargo clippy --workspace --all-targets -- -D warnings,cargo test --workspace(1679) all clean.Also here
SKILL.md's preset table said "Char (text only)". That was true before this branch, and it is precisely the sentence a generator reads to decide not to try. Corrected.Written comment-free, per the codebase-wide rule from #345.