fix(paint): animated border-radius, real dashed borders, and a clip-path morph - #398
Merged
Merged
Conversation
…orph (#373, #375, #384) Three "a declared thing the paint pass ignores" bugs, grouped because they all live in paint_pass.rs/animator.rs and reinforce the same lesson: a property that validates must actually move a pixel. - #373: apply_animated_props mapped translate/scale/rotate/opacity/blur/ glow/perspective/width/height from AnimatedProperties onto CssStyle, but never border_radius, even though the animator resolved it correctly and border_radius has been in KNOWN_MOTION_PROPERTIES all along. A keyframed square never rounded into a circle. One missing branch; now animator -> apply_animated_props -> paint_border is a straight line for this property too, same as the timeline/style.transition path already was. - #375: paint_border read border.style only to gate BorderStyle::None; every other variant (dashed, dotted, double) fell into the same filled-drrect path as solid. Dashed/dotted now stroke the border's centerline with a sized PathEffect::dash (3x width on/off for dashed, near-zero "on" + round cap for width-diameter dots); double draws two width/3 strokes with a width/3 gap between them, matching the CSS box model exactly. - #384: animating clip-path is new territory this codebase's vocabulary doesn't reach cleanly. The literal ask -- a `property: "clip-path"` entry inside a `keyframes` animation whose values are ClipPath-shaped objects -- needs `KeyframeValue` (schema::animation) to carry a ClipPath variant, and that type, plus the AnimationEffect exhaustiveness check in rustmotion/cli/commands/validate_schema.rs, belong to other writers in this parallel workstream. Reworked the ask instead of dropping it: a new `ClipPath::Morph { from, to, progress }` variant fixes the two endpoints once as an ordinary style value, and `progress` rides the *existing* generic scalar keyframes engine via a new `clip_path_progress` motion property -- same machinery opacity/border_radius already use, so easing, springs and loops all come for free. Interpolation is resolved geometry to resolved geometry (mirroring how clip_path_to_skia already resolves each static shape), covering polygon/inset/circle/ellipse. A kind or polygon-arity mismatch is reported on stderr naming both shapes and leaves the node unclipped for that frame, rather than snapping between two incompatible shapes -- the same "loud, not silent" precedent this file already sets for `clip-path: { kind: node-path }`. Every fix is proven by a pixel test in paint_order_tests: each was reverted, confirmed red, and restored (see PR description / handback for the exact failure messages). Two new French rules docs land under skills/rules/ (border-style.md, clip-path-morph.md) documenting the new surface; SKILL.md is deliberately left unlinked since the orchestrator owns that index across several parallel PRs.
…rrect clip-path.md The line saying clip-path never interpolates was written when that was true of every path. Keyframes reach it now through the morph variant; a timeline still snaps it.
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
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 #373. Closes #375. Refs #384, #388 — see the note on #384 before merging.
#373 — animated
border_radiusvalidated and did nothingapply_animated_propsmapped every animated scalar ontoCssStyleexceptborder_radius. One missing branch.#375 —
dashed,dottedanddoublepainted a solid ringpaint_borderreadborder.styleonly to gateBorderStyle::None; every other variant fell through to the same filled path.Now:
dashedanddottedstroke the border's centreline with a dash effect (dashed on/off = 3× width each; dotted = a near-zero "on" with a round cap, so dots of diameter = width at 2× spacing);doubledraws two width/3 strokes separated by a width/3 gap, which is the CSS box model exactly.Known limitation, documented: per-side border widths do not affect the dash cadence — it uses the max of the four uniformly.
The issue asks for
property: "clip-path"with N keyframes each holding a shape. What is here is:with the sweep driven by a scalar motion property,
clip_path_progress, through the ordinarykeyframesengine.Why it went that way. The literal form needs
KeyframeValue(untaggedNumber|Color) inschema/animation.rsto grow aClipPathvariant, andvalidate_schema.rs's exhaustiveAnimationEffectmatch to learn it. Both were outside that workstream's file ownership, and it reported the wall rather than reaching across it — which was the right call under the partition.What it buys.
easing,spring,loopand per-keyframe easing all work on clip-path morphing for free, because it is an ordinary scalar keyframe track. No new wire format.What it costs. Exactly two endpoints per morph. It covers the issue's own reproduction and the reel-4 case, and does not generalise to a chain of more than two shapes without stacking nodes.
I am leaving #384 open rather than closing it on a near-miss. The blocking files are within my reach and finishing the literal form is a decision about whether the extra vocabulary is worth it — not something to settle silently inside a merge.
Interpolation covers polygon (point-by-point, equal counts), inset (edges and radius), circle and ellipse (radii and origin). A kind mismatch or a vertex-count mismatch prints to stderr and leaves the node unclipped — no snap, no silent interpolation. That mirrors the existing precedent in the same function for
kind: node-path, rather than inventing a new failure mode.Also in here
rules/clip-path.mdsaid "Il ne s'interpole pas dans untimeline. C'est une propriété de peinture non supportée à l'animation." That is now half true — keyframes reach it, a timeline still snaps it. Corrected with a pointer to the new rule, since a half-true doc is what #370 was about.Verification
12 new tests. Each proven red by reverting. One of them was caught being vacuous on first draft: it only checked
progress: 1.0against a "to" shape covering the whole box, which passed whether or not the fix existed — fixed by also assertingprogress: 0against thefromshape.cargo fmt --all --check,cargo clippy --workspace --all-targets -- -D warnings,cargo test --workspace(1652) all clean. Comment count still 0.Written comment-free, per the codebase-wide rule from #345.