Animation: Implement DoubleStyleInterpolation in StringKeyframe

Source/core/animation/StringKeyframe.cpp

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "config.h"
 #include "core/animation/StringKeyframe.h"
 
 #include "core/animation/ColorStyleInterpolation.h"
 #include "core/animation/DefaultStyleInterpolation.h"
 #include "core/animation/DeferredLegacyStyleInterpolation.h"
+#include "core/animation/DoubleStyleInterpolation.h"
 #include "core/animation/LegacyStyleInterpolation.h"
+#include "core/animation/LengthBoxStyleInterpolation.h"
 #include "core/animation/LengthStyleInterpolation.h"
 #include "core/animation/VisibilityStyleInterpolation.h"
 #include "core/animation/css/CSSAnimations.h"
 #include "core/css/CSSPropertyMetadata.h"
 #include "core/css/resolver/StyleResolver.h"
 #include "core/rendering/style/RenderStyle.h"
 
 namespace blink {
 
 StringKeyframe::StringKeyframe(const StringKeyframe& copyFrom)
@@ skipping 50 matching lines @@
 {
     CSSValue* fromCSSValue = m_value.get();
     CSSValue* toCSSValue = toStringPropertySpecificKeyframe(end)->value();
     ValueRange range = ValueRangeAll;
 
     if (!CSSPropertyMetadata::isAnimatableProperty(property))
         return DefaultStyleInterpolation::create(fromCSSValue, toCSSValue, property);
 
     switch (property) {
     case CSSPropertyBorderBottomWidth:
     case CSSPropertyBorderLeftWidth:

[Eric Willigers, 2014/12/22 07:35:38]

     case CSSPropertyBorderBottomWidth:
     case CSSPropertyBorderLeftWidth:
     case CSSPropertyBorderRightWidth:
     case CSSPropertyBorderTopWidth:
     case CSSPropertyFontSize:
should all use non-negative length (like CSSPropertyHeight)

[jadeg, 2015/01/05 23:15:20]

Done.

     case CSSPropertyBorderRightWidth:
     case CSSPropertyBorderTopWidth:
+    case CSSPropertyFillOpacity:
+    case CSSPropertyFlexGrow:
+    case CSSPropertyFlexShrink:
+    case CSSPropertyFloodOpacity:
     case CSSPropertyFontSize:
+    case CSSPropertyLetterSpacing:

[Eric Willigers, 2014/12/22 07:35:38]

letter spacer and word spacing and outline offset here are incorrect - they are
(optionally negative) lengths, like CSSPropertyLeft.

[jadeg, 2015/01/05 23:15:20]

Done.

+    case CSSPropertyOrphans:

[Eric Willigers, 2014/12/22 07:35:38]

The following are always integers:   orphans webkit-column-count widows z-index

[jadeg, 2015/01/05 23:15:20]

Done.

+    case CSSPropertyOutlineOffset:
+    case CSSPropertyOutlineWidth:
+    case CSSPropertyPerspective:
+    case CSSPropertyShapeImageThreshold:

[Eric Willigers, 2014/12/22 07:35:38]

shape-image-threshold is clamped 0..1, just like opacity

[jadeg, 2015/01/05 23:15:20]

Have changed this, but just to note that this clamping was not picked up in the
layout tests.

+    case CSSPropertyStopOpacity:
+    case CSSPropertyStrokeMiterlimit:
+    case CSSPropertyStrokeOpacity:
+    case CSSPropertyWebkitBorderHorizontalSpacing:
+    case CSSPropertyWebkitBorderVerticalSpacing:
+    case CSSPropertyWebkitColumnCount:
+    case CSSPropertyWebkitColumnGap:
+    case CSSPropertyWebkitColumnRuleWidth:
+    case CSSPropertyWebkitColumnWidth:

[Eric Willigers, 2014/12/22 07:35:38]

OutlineWidth and Perspective and WebkitBorderHorizontalSpacing and
WebkitBorderVerticalSpacing and WebkitColumnGap and WebkitColumnWidth are
non-negative lengths, like CSSPropertyWidth
They are not double.

[jadeg, 2015/01/05 23:15:20]

Done.

+    case CSSPropertyWidows:
+    case CSSPropertyWordSpacing:
+    case CSSPropertyZIndex:
+    case CSSPropertyZoom:
+        if (DoubleStyleInterpolation::canCreateFrom(*fromCSSValue) && DoubleStyleInterpolation::canCreateFrom(*toCSSValue))

[Eric Willigers, 2014/12/22 07:35:38]

Note that Chrome does not permit negative values for the majority of these
properties. The exceptions are
FillOpacity 
FloodOpacity 
StopOpacity
StrokeOpacity
  (and even these are later clamped to 0..1)
ZIndex.

[jadeg, 2015/01/05 23:15:20]

Just to check, I have found no clamping rules for 
case CSSPropertyFlexGrow:
case CSSPropertyFlexShrink:
case CSSPropertyWebkitColumnRuleWidth:
Not sure if this is correct or just a hole in the layout tests

[jadeg, 2015/01/05 23:15:20]

Done.

+            return DoubleStyleInterpolation::create(*fromCSSValue, *toCSSValue, property, NoClamp);
+        break;
+    case CSSPropertyOpacity:
+        if (DoubleStyleInterpolation::canCreateFrom(*fromCSSValue) && DoubleStyleInterpolation::canCreateFrom(*toCSSValue))
+            return DoubleStyleInterpolation::create(*fromCSSValue, *toCSSValue, property, ClampOpacity);
+        break;
+
+    case CSSPropertyLineHeight:
+        if (DoubleStyleInterpolation::canCreateFrom(*fromCSSValue) && DoubleStyleInterpolation::canCreateFrom(*toCSSValue))
+            return DoubleStyleInterpolation::create(*fromCSSValue, *toCSSValue, property, NoClamp);

[Eric Willigers, 2014/12/22 05:02:41]

clamp to non-negative values

[jadeg, 2015/01/05 23:15:20]

Done.

+        if (LengthStyleInterpolation::canCreateFrom(*fromCSSValue) && LengthStyleInterpolation::canCreateFrom(*toCSSValue))
+            return LengthStyleInterpolation::create(*fromCSSValue, *toCSSValue, property, range);

[Eric Willigers, 2014/12/22 05:02:41]

ValueRangeNonNegative 

[jadeg, 2015/01/05 23:15:20]

Done.

+        break;
+
     case CSSPropertyHeight:
-    case CSSPropertyLineHeight:
     case CSSPropertyMaxHeight:
     case CSSPropertyMaxWidth:
     case CSSPropertyMinHeight:
     case CSSPropertyMinWidth:
     case CSSPropertyMotionPosition:
-    case CSSPropertyOutlineWidth:
     case CSSPropertyPaddingBottom:
     case CSSPropertyPaddingLeft:
     case CSSPropertyPaddingRight:
     case CSSPropertyPaddingTop:
-    case CSSPropertyPerspective:
     case CSSPropertyShapeMargin:
     case CSSPropertyWidth:
         range = ValueRangeNonNegative;
         // Fall through
     case CSSPropertyBottom:
+    case CSSPropertyFlexBasis:

[Eric Willigers, 2014/12/22 05:02:41]

Move CSSPropertyFlexBasis up to use ValueRangeNonNegative (like
CSSPropertyHeight), since Negative values are invalid.

Spec: https://developer.mozilla.org/en-US/docs/Web/CSS/flex-basis
Tested in http://jsfiddle.net/ericwilligers/bL2rzfv7/



(CSSPropertyTextIndent case below is ok - text indent may be negative)

[jadeg, 2015/01/05 23:15:20]

Done.

     case CSSPropertyLeft:
-    case CSSPropertyLetterSpacing:
     case CSSPropertyMarginBottom:
     case CSSPropertyMarginLeft:
     case CSSPropertyMarginRight:
     case CSSPropertyMarginTop:
-    case CSSPropertyOutlineOffset:
     case CSSPropertyRight:
+    case CSSPropertyTextIndent:
     case CSSPropertyTop:
     case CSSPropertyVerticalAlign:
-    case CSSPropertyWordSpacing:

[Eric Willigers, 2014/12/22 05:02:41]

Removing
  case CSSPropertyLetterSpacing:
  case CSSPropertyOutlineOffset:
  case CSSPropertyWordSpacing:
is incorrect:  letter-spacing and outline-offset and word-spacing may be any
length (including negative).

Tested in http://jsfiddle.net/ericwilligers/jkppdmoL/

[jadeg, 2015/01/05 23:15:20]

Done.

         if (LengthStyleInterpolation::canCreateFrom(*fromCSSValue) && LengthStyleInterpolation::canCreateFrom(*toCSSValue))
             return LengthStyleInterpolation::create(*fromCSSValue, *toCSSValue, property, range);
+        break;
+
+    case CSSPropertyBorderImageSlice:

[Eric Willigers, 2014/12/19 00:16:13]

Let's move the two LengthBoxStyleInterpolation cases to a separate CL

[jadeg, 2015/01/05 23:15:20]

Done.

+    case CSSPropertyClip:
+        if (LengthBoxStyleInterpolation::canCreateFrom(*fromCSSValue) && LengthBoxStyleInterpolation::canCreateFrom(*toCSSValue))
+            return LengthBoxStyleInterpolation::create(*fromCSSValue, *toCSSValue, property);
+        break;
+
     case CSSPropertyVisibility:
         if (VisibilityStyleInterpolation::canCreateFrom(*fromCSSValue) && VisibilityStyleInterpolation::canCreateFrom(*toCSSValue) && (VisibilityStyleInterpolation::isVisible(*fromCSSValue) || VisibilityStyleInterpolation::isVisible(*toCSSValue))) {
             return VisibilityStyleInterpolation::create(*fromCSSValue, *toCSSValue, property);
         }
         break;
+

[samli, 2014/12/22 03:41:15]

Remove new line

[jadeg, 2015/01/05 23:15:20]

Done.

     case CSSPropertyFill:
     case CSSPropertyStroke:
+

[samli, 2014/12/22 03:41:15]

ditto

[jadeg, 2015/01/05 23:15:20]

Done.

     case CSSPropertyBackgroundColor:
     case CSSPropertyBorderBottomColor:
     case CSSPropertyBorderLeftColor:
     case CSSPropertyBorderRightColor:
     case CSSPropertyBorderTopColor:
     case CSSPropertyColor:
     case CSSPropertyFloodColor:
     case CSSPropertyLightingColor:
     case CSSPropertyOutlineColor:
     case CSSPropertyStopColor:
@@ skipping 34 matching lines @@
 }
 
 void StringKeyframe::PropertySpecificKeyframe::trace(Visitor* visitor)
 {
     visitor->trace(m_value);
     visitor->trace(m_animatableValueCache);
     Keyframe::PropertySpecificKeyframe::trace(visitor);
 }
 
 }