Use IntermediateRGBA to store overflowed RGBA components during inter…

…polation.
servo · Apr 24, 2017 · d70e4aa · d70e4aa
1 parent e47d30f
commit d70e4aa
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 8 deletions.
diff --git a/components/style/properties/data.py b/components/style/properties/data.py
@@ -178,13 +178,11 @@ def __init__(self, style_struct, name, spec=None, animation_value_type=None, der
         # really random.
         if animation_value_type is None:
             raise TypeError("animation_value_type should be specified for (" + name + ")")
-        animation_value_types = ["none", "discrete", "ComputedValue"]
-        if animation_value_type not in animation_value_types:
-            raise TypeError("animation_value_type should be one of (" +
-                            str(animation_value_types) + ")")
         self.animation_value_type = animation_value_type
 
         self.animatable = animation_value_type != "none"
+        self.is_animatable_with_computed_value = animation_value_type == "ComputedValue" \
+            or animation_value_type == "discrete"
         if self.logical:
             # Logical properties will be animatable (i.e. the animation type is
             # discrete). For now, it is still non-animatable.

diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs
@@ -384,7 +384,11 @@ pub enum AnimationValue {
     % for prop in data.longhands:
         % if prop.animatable:
             /// ${prop.name}
-            ${prop.camel_case}(longhands::${prop.ident}::computed_value::T),
+            % if prop.is_animatable_with_computed_value:
+                ${prop.camel_case}(longhands::${prop.ident}::computed_value::T),
+            % else:
+                ${prop.camel_case}(${prop.animation_value_type}),
+            % endif
         % endif
     % endfor
 }
@@ -402,7 +406,13 @@ impl AnimationValue {
                             % if prop.boxed:
                                 Box::new(longhands::${prop.ident}::SpecifiedValue::from_computed_value(from)))
                             % else:
-                                longhands::${prop.ident}::SpecifiedValue::from_computed_value(from))
+                                longhands::${prop.ident}::SpecifiedValue::from_computed_value(
+                                % if prop.is_animatable_with_computed_value:
+                                    from
+                                % else:
+                                    &from.into()
+                                % endif
+                                ))
                             % endif
                     }
                 % endif
@@ -426,7 +436,13 @@ impl AnimationValue {
                     longhands::system_font::resolve_system_font(sf, context);
                 }
             % endif
-                Some(AnimationValue::${prop.camel_case}(val.to_computed_value(context)))
+                Some(AnimationValue::${prop.camel_case}(
+                % if prop.is_animatable_with_computed_value:
+                    val.to_computed_value(context)
+                % else:
+                    From::from(&val.to_computed_value(context))
+                % endif
+                ))
             },
             % endif
             % endfor
@@ -454,6 +470,9 @@ impl AnimationValue {
                                 inherit_struct.clone_${prop.ident}()
                             },
                         };
+                        % if not prop.is_animatable_with_computed_value:
+                            let computed = From::from(&computed);
+                        % endif
                         Some(AnimationValue::${prop.camel_case}(computed))
                     },
                     % endif
@@ -514,7 +533,12 @@ impl AnimationValue {
                 % if prop.animatable:
                     TransitionProperty::${prop.camel_case} => {
                         AnimationValue::${prop.camel_case}(
+                        % if prop.is_animatable_with_computed_value:
                             computed_values.get_${prop.style_struct.ident.strip("_")}().clone_${prop.ident}())
+                        % else:
+                            From::from(&computed_values.get_${prop.style_struct.ident.strip("_")}()
+                                                                  .clone_${prop.ident}()))
+                        % endif
                     }
                 % endif
             % endfor
@@ -2041,6 +2065,25 @@ impl<T, U> Interpolate for Either<T, U>
     }
 }
 
+impl <'a> From<<&'a IntermediateRGBA> for RGBA {
+    fn from(extended_rgba: &IntermediateRGBA) -> RGBA {
+        // RGBA::from_floats clamps each component values.
+        RGBA::from_floats(extended_rgba.red,
+                          extended_rgba.green,
+                          extended_rgba.blue,
+                          extended_rgba.alpha)
+    }
+}
+
+impl <'a> From<<&'a RGBA> for IntermediateRGBA {
+    fn from(rgba: &RGBA) -> IntermediateRGBA {
+        IntermediateRGBA::new(rgba.red_f32(),
+                              rgba.green_f32(),
+                              rgba.blue_f32(),
+                              rgba.alpha_f32())
+    }
+}
+
 #[derive(Copy, Clone, Debug, PartialEq)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 /// Unlike RGBA, each component value may exceed the range [0.0, 1.0].

diff --git a/components/style/properties/longhand/color.mako.rs b/components/style/properties/longhand/color.mako.rs
@@ -8,7 +8,8 @@
 
 <% from data import to_rust_ident %>
 
-<%helpers:longhand name="color" need_clone="True" animation_value_type="ComputedValue"
+<%helpers:longhand name="color" need_clone="True"
+                   animation_value_type="IntermediateRGBA"
                    spec="https://drafts.csswg.org/css-color/#color">
     use cssparser::RGBA;
     use std::fmt;