feat(animation): interpolate more properties, and name the ones that cannot - #173
Merged
Conversation
…cannot `style.transition` promised smoothing that two properties delivered. Every other property snapped at the step's `at`, and the field's own doc comment admitted it — but nothing told the author. `validate` never looked at `style.transition` or at the diffs between `timeline` states, so a transition on `width` or `border-radius` passed clean and rendered as a jump. The missing capability was not the problem. The silence was. **Diagnosing comes first.** `check_transition_smoothing` replays the timeline in the author's order, diffs each field against the running state, and classifies every touched property into four buckets, each with its own message. A layout property explains *why* it cannot interpolate and points at `transform: translate`/`scale` as the paint-time alternative; a discrete property says the snap is expected CSS behaviour, not a rustmotion limitation — so nobody chases a bug that is not one. The check only runs when `style.transition` is actually set: nothing was promised otherwise. **Then interpolation, for what can be done honestly.** `background` (solid colours) and `border-radius` (uniform, absolute px) now animate. They are resolved onto `CssStyle` before layout, where `paint_pass` already reads them for the static case — so nothing in the frozen paint or layout passes had to change, and the interpolation maths is the existing keyframe solver, not a second one. Layout properties are deliberately left snapping. Interpolating `width` without re-running layout would put the measured box and the painted pixels out of step and blind the geometry validator — the class of bug this repository already spent a chantier repairing. Signalled, not simulated. Mixed units are refused rather than guessed: `box_builder` runs before layout, so `%` and `em` have no trustworthy base yet. The diagnostic and the runtime share one predicate for what is resolvable, so they cannot drift into disagreeing about it. Also removed: a diagnostic I had written for unknown `Animation.property` values before discovering that `schema/video.rs` already rejects them at parse time with a did-you-mean. A test pins that behaviour instead, rather than shipping dead code that looks like a feature. No shipped example changes: none of the eight declares `timeline` on a component, verified by walking the JSON rather than by grep.
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 the "generic property interpolation" gap (High/M) from the re-scored Remotion differential — the ceiling on all of
timelineandstyle.transition.The silence was the bug
style.transitionpromised smoothing that exactly two properties delivered:opacity, andcoloron text/counter. Everything else snapped at the step'sat, and the field's own doc comment admitted it. Butvalidatenever looked atstyle.transition, nor at the diffs betweentimelinestates — so a transition onwidthorborder-radiuspassed clean and rendered as a jump.Measured before any fix, sampling a 1s transition at its midpoint:
Diagnosing comes first
check_transition_smoothingreplays the timeline in the author's order, diffs each field against the running state, and classifies every touched property into four buckets — each with its own message rather than one generic complaint:The second message matters as much as the first: it stops an author chasing a bug that is not one. The check only runs when
style.transitionis actually set — nothing was promised otherwise, so nothing is warned about.Then interpolation, for what can be done honestly
background(solid colours) andborder-radius(uniform, absolute px) now animate. They are resolved ontoCssStylebefore layout, wherepaint_passalready reads them for the static case — so neither frozen pass needed changing, and the interpolation maths is the existing keyframe solver rather than a second copy of it.Layout properties are deliberately left snapping. Interpolating
widthwithout re-running layout would put the measured box and the painted pixels out of step and blind the geometry validator — the class of bug this repository already spent a chantier repairing. Signalled, not simulated.Mixed units are refused, not guessed.
box_builderruns before layout, so%andemhave no trustworthy base yet. The diagnostic and the runtime share one predicate for what counts as resolvable, so they cannot drift into disagreeing.Scope, stated plainly
Interpolable now:
opacity,color,background(solid),border-radius(uniform px).Diagnosed, not implemented:
transform(pairing two function lists of different length or order is ambiguous — guessing would be worse than saying so),box-shadow/filter/clip-path(continuous but multi-field), per-corner radius, relative units, and every layout property.Removed rather than shipped
I had written a second diagnostic for unknown
Animation.propertyvalues, then found on testing thatschema/video.rsalready rejects them at parse time with a did-you-mean. The redundant code was deleted and a test pins the existing behaviour — dead code that looks like a feature is worse than no code.Verification
cargo test --workspace: 26 targets, 1046 tests, 0 failurescargo fmt --all --checkandcargo clippy --workspace --all-targets -- -D warnings: cleantimelineon a component — verified by walking the JSON, not by grepping for the word (mega-showcaseandcomponent-showcaseboth contain"timeline"as a component type, which would have made a grep lie).