fix(paint): let a self-painting component see its animated background - #415
Merged
Merged
Conversation
Six components paint their own background and read it from their own style: badge, callout, caption, kbd, stat, tooltip. The animated value never reached them. resolve_transition_css_overrides writes the interpolated colour onto the node's css.background, and the cascade only carries inheritable properties onto the clone a component is painted from -- background is not one. So a badge with a timeline step changing style.background painted its declared colour for the whole scene, and the generic background step underneath painted the animated one where the component's own opaque fill covered it. with_resolved_background writes the node's resolved background onto a clone of the six. It returns None when the resolved value already equals the component's own, so a component whose background is not animated is not cloned at all -- the serde round-trip is not paid per frame for the common case. stat is the reason this is not folded into with_cascaded_style: that function is about inheritance and classifies stat as non-typographic, so it returns None for it and the fix would have missed a sixth of the components it is for. Verified on a badge with a 1.0s linear transition: red at 0.2s, (127, 0, 127) at 1.0s, blue at 1.8s. Before, all three were red.
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 #324.
Reproduced
A
badgewith atimelinestep changingstyle.background, over a 1.0 s lineartransition:
#FF0000#FF0000#FF0000#7F007F#FF0000#0000FFCause
Six components paint their own background and read it from their own style:
badge,callout,caption,kbd,stat,tooltip.resolve_transition_css_overrideswrites the interpolated colour onto the node'scss.background. The cascade only carries inheritable properties onto theclone a component is painted from, and
backgroundis not one — so the componentkept painting its declared colour, over the animated one the generic background
step had just painted underneath it.
The fix
with_resolved_backgroundwrites the node's resolved background onto a clone ofthose six. It returns
Nonewhen the resolved value already equals thecomponent's own, so a component whose background is not animated is not cloned
at all — the serde round-trip is not paid per frame for the common case.
Why not fold it into
with_cascaded_stylestat. That function is about inheritance and classifiesstatasnon-typographic, so it returns
Nonefor it — folding the fix in there would havemissed a sixth of the components it is for. The two concerns are kept apart, and
a_stat_follows_it_too_although_the_cascade_never_clones_itis the test that pinsthat specific path.
Tests
a_badge_follows_an_animated_background_instead_of_repainting_its_owna_stat_follows_it_too_although_the_cascade_never_clones_itwith_cascaded_styledoes not reacha_component_with_no_background_animation_is_untouchedReverting the dispatch change fails the first two with
got (255, 0, 0)— thedeclared colour, at the halfway point.
Gate
cargo fmt --all --checkclean ·cargo clippy --workspace --all-targets --features rustmotion/studio -D warningsclean ·cargo test --workspace1787 passed.