Open
Bug 1514342
Opened 5 years ago
Updated 2 years ago
ToAnimatedValue should just move/clone self if it is not animatable
Categories
(Core :: CSS Parsing and Computation, defect, P3)
Core
CSS Parsing and Computation
Tracking
()
NEW
People
(Reporter: boris, Unassigned)
References
Details
While adding ToAnimatedValue for ShapeSource, I found we didn't handle "#[animation(error)]" properly style_derive/to_animated_value.rs. For example [1]: pub enum ShapeSource<BasicShape, ReferenceBox, ImageOrUrl> { #[animation(error)] ImageOrUrl(ImageOrUrl), Shape(BasicShape, Option<ReferenceBox>), #[animation(error)] Box(ReferenceBox), #[css(function)] Path(Path), #[animation(error)] None, } If I want to derive "ToAnimatedValue", we should just move/copy the arms of "ImageOrUrl(...)" and "Box(...)" because they are not animatable. ToAnimatedZero and ComputeSquaredDistance handle this properly, but ToAnimatedValue doesn't. What I expected is something like this: impl<B, R, I> ToAnimatedValue for ShapeSource<B, R, I> { type AnimatedValue = ShapeSource<B::AnimatedValue, R::AnimatedValue, I::AnimatedValue>; #[inline] fn to_animated_value(self) -> Self { match self { ImageOrUrl(_) => ImageOrUrl(_), // Just move it. Shape(b, r) => { Shape(b.to_animated_value(), r.to_animated_value()) }, ... } } #[inline] fn from_animated_value(animated: Self::AnimatedValue) -> Self { match animated { ImageOrUrl(_) => ImageOrUrl(_), // Just move it. Shape(b, r) => { Shape(B::from_animated_value(b), R::from_animated_value(r)) }, ... } } } [1] https://searchfox.org/mozilla-central/rev/49e78df13e7a505827a3a86daae9efdf827133c6/servo/components/style/values/generics/basic_shape.rs#58-74
Reporter | ||
Updated•5 years ago
|
Summary: ToAnimatedValue should just clone self if it is not animatable → ToAnimatedValue should just move/clone self if it is not animatable
Comment 1•5 years ago
|
||
So to be clear, the behavior now is that we call (or try to?) `to_animated_value`, right?
Reporter | ||
Comment 2•5 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #1) > So to be clear, the behavior now is that we call (or try to?) > `to_animated_value`, right? Yes. There are two functions in this trait: 1. to_animated_value(self) 2. from_animated_value(animated: Self::AnimatedValue) We need this trait to convert the computed value into an intermediate value for interpolation (for some properties, e.g. color)
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•