Avoid visually transitioning on length CSS properties when zoom changes

third_party/WebKit/Source/core/css/resolver/AnimatedStyleBuilder.cpp

 /*
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 84 matching lines @@
   if (value->isLength())
     return BorderImageLength(toAnimatableLength(value)->getLength(
         state.style()->effectiveZoom(), ValueRangeNonNegative));
   if (value->isDouble())
     return BorderImageLength(
         clampTo<double>(toAnimatableDouble(value)->toDouble(), 0));
   ASSERT(toAnimatableUnknown(value)->toCSSValueID() == CSSValueAuto);
   return Length(Auto);
 }
 
-template <typename T>
-T animatableValueClampTo(const AnimatableValue* value,
-                         T min = defaultMinimumForClamp<T>(),
-                         T max = defaultMaximumForClamp<T>()) {
-  static_assert(std::is_integral<T>::value,
-                "should use integral type T when rounding values");
-  return clampTo<T>(
-      roundForImpreciseConversion<T>(toAnimatableDouble(value)->toDouble()),
-      min, max);
+double animatableValueToPixels(const AnimatableValue* value,
+                               const StyleResolverState& state) {
+  return toAnimatableLength(value)
+      ->getLength(state.style()->effectiveZoom(), ValueRangeAll)
+      .pixels();
 }
 
 template <typename T>
-T animatableLineWidthClamp(const AnimatableValue* value) {
-  double doubleValue = toAnimatableDouble(value)->toDouble();
+T roundedClampTo(double value,

[suzyh_UTC10 (ex-contributor), 2016/12/12 04:30:13]

Is there any chance that the changes to function names and arguments (which is
kind of a refactoring) can be separated from the actual change to avoid
transitions? The function name/argument changes are driven by the behaviour
change, but doing both in a single CL means it's not obvious which parts of the
patch actually produce the behaviour change.

[alancutter (OOO until 2018), 2016/12/13 02:36:34]

Done.

+                 T min = defaultMinimumForClamp<T>(),
+                 T max = defaultMaximumForClamp<T>()) {
+  static_assert(std::is_integral<T>::value,
+                "should use integral type T when rounding values");
+  return clampTo<T>(roundForImpreciseConversion<T>(value), min, max);
+}
+
+template <typename T>
+T animatableLineWidthClamp(const AnimatableValue* value,
+                           const StyleResolverState& state) {
+  double lineWidth = animatableValueToPixels(value, state);
   // This matches StyleBuilderConverter::convertLineWidth().
-  return (doubleValue > 0 && doubleValue < 1)
-             ? 1
-             : animatableValueClampTo<T>(value);
+  return (lineWidth > 0 && lineWidth < 1) ? 1 : roundedClampTo<T>(lineWidth);
 }
 
 LengthBox animatableValueToLengthBox(const AnimatableValue* value,
                                      const StyleResolverState& state,
                                      ValueRange range = ValueRangeAll) {
   const AnimatableLengthBox* animatableLengthBox = toAnimatableLengthBox(value);
   return LengthBox(
       animatableValueToLength(animatableLengthBox->top(), state, range),
       animatableValueToLength(animatableLengthBox->right(), state, range),
       animatableValueToLength(animatableLengthBox->bottom(), state, range),
@@ skipping 23 matching lines @@
 
 TransformOrigin animatableValueToTransformOrigin(
     const AnimatableValue* value,
     const StyleResolverState& state,
     ValueRange range = ValueRangeAll) {
   const AnimatableLengthPoint3D* animatableLengthPoint3D =
       toAnimatableLengthPoint3D(value);
   return TransformOrigin(
       animatableValueToLength(animatableLengthPoint3D->x(), state),
       animatableValueToLength(animatableLengthPoint3D->y(), state),
-      clampTo<float>(
-          toAnimatableDouble(animatableLengthPoint3D->z())->toDouble()));
+      animatableValueToPixels(animatableLengthPoint3D->z(), state));
 }
 
 LengthSize animatableValueToLengthSize(const AnimatableValue* value,
                                        const StyleResolverState& state,
                                        ValueRange range) {
   const AnimatableLengthSize* animatableLengthSize =
       toAnimatableLengthSize(value);
   return LengthSize(
       animatableValueToLength(animatableLengthSize->width(), state, range),
       animatableValueToLength(animatableLengthSize->height(), state, range));
@@ skipping 167 matching lines @@
       return;
     case CSSPropertyBorderBottomLeftRadius:
       style->setBorderBottomLeftRadius(
           animatableValueToLengthSize(value, state, ValueRangeNonNegative));
       return;
     case CSSPropertyBorderBottomRightRadius:
       style->setBorderBottomRightRadius(
           animatableValueToLengthSize(value, state, ValueRangeNonNegative));
       return;
     case CSSPropertyBorderBottomWidth:
-      style->setBorderBottomWidth(animatableLineWidthClamp<unsigned>(value));
+      style->setBorderBottomWidth(
+          animatableLineWidthClamp<unsigned>(value, state));
       return;
     case CSSPropertyBorderImageOutset:
       style->setBorderImageOutset(
           animatableValueToBorderImageLengthBox(value, state));
       return;
     case CSSPropertyBorderImageSlice:
       style->setBorderImageSlices(
           animatableValueToLengthBox(toAnimatableLengthBoxAndBool(value)->box(),
                                      state, ValueRangeNonNegative));
       style->setBorderImageSlicesFill(
           toAnimatableLengthBoxAndBool(value)->flag());
       return;
     case CSSPropertyBorderImageSource:
       style->setBorderImageSource(
           state.styleImage(property, *toAnimatableImage(value)->toCSSValue()));
       return;
     case CSSPropertyBorderImageWidth:
       style->setBorderImageWidth(
           animatableValueToBorderImageLengthBox(value, state));
       return;
     case CSSPropertyBorderLeftColor:
       style->setBorderLeftColor(toAnimatableColor(value)->getColor());
       style->setVisitedLinkBorderLeftColor(
           toAnimatableColor(value)->visitedLinkColor());
       return;
     case CSSPropertyBorderLeftWidth:
-      style->setBorderLeftWidth(animatableLineWidthClamp<unsigned>(value));
+      style->setBorderLeftWidth(
+          animatableLineWidthClamp<unsigned>(value, state));
       return;
     case CSSPropertyBorderRightColor:
       style->setBorderRightColor(toAnimatableColor(value)->getColor());
       style->setVisitedLinkBorderRightColor(
           toAnimatableColor(value)->visitedLinkColor());
       return;
     case CSSPropertyBorderRightWidth:
-      style->setBorderRightWidth(animatableLineWidthClamp<unsigned>(value));
+      style->setBorderRightWidth(
+          animatableLineWidthClamp<unsigned>(value, state));
       return;
     case CSSPropertyBorderTopColor:
       style->setBorderTopColor(toAnimatableColor(value)->getColor());
       style->setVisitedLinkBorderTopColor(
           toAnimatableColor(value)->visitedLinkColor());
       return;
     case CSSPropertyBorderTopLeftRadius:
       style->setBorderTopLeftRadius(
           animatableValueToLengthSize(value, state, ValueRangeNonNegative));
       return;
     case CSSPropertyBorderTopRightRadius:
       style->setBorderTopRightRadius(
           animatableValueToLengthSize(value, state, ValueRangeNonNegative));
       return;
     case CSSPropertyBorderTopWidth:
-      style->setBorderTopWidth(animatableLineWidthClamp<unsigned>(value));
+      style->setBorderTopWidth(
+          animatableLineWidthClamp<unsigned>(value, state));
       return;
     case CSSPropertyBottom:
       style->setBottom(animatableValueToLength(value, state));
       return;
     case CSSPropertyBoxShadow:
       style->setBoxShadow(toAnimatableShadow(value)->getShadowList());
       return;
     case CSSPropertyClip:
       style->setClip(animatableValueToLengthBox(value, state));
       return;
@@ skipping 64 matching lines @@
       else
         style->setLineHeight(Length(
             clampTo<float>(toAnimatableDouble(value)->toDouble(), 0), Percent));
       return;
     case CSSPropertyListStyleImage:
       style->setListStyleImage(
           state.styleImage(property, *toAnimatableImage(value)->toCSSValue()));
       return;
     case CSSPropertyLetterSpacing:
       style->setLetterSpacing(
-          clampTo<float>(toAnimatableDouble(value)->toDouble()));
+          clampTo<float>(animatableValueToPixels(value, state)));
       return;
     case CSSPropertyMarginBottom:
       style->setMarginBottom(animatableValueToLength(value, state));
       return;
     case CSSPropertyMarginLeft:
       style->setMarginLeft(animatableValueToLength(value, state));
       return;
     case CSSPropertyMarginRight:
       style->setMarginRight(animatableValueToLength(value, state));
       return;
@@ skipping 27 matching lines @@
     case CSSPropertyOrphans:
       style->setOrphans(
           clampTo<short>(round(toAnimatableDouble(value)->toDouble()), 1));
       return;
     case CSSPropertyOutlineColor:
       style->setOutlineColor(toAnimatableColor(value)->getColor());
       style->setVisitedLinkOutlineColor(
           toAnimatableColor(value)->visitedLinkColor());
       return;
     case CSSPropertyOutlineOffset:
-      style->setOutlineOffset(animatableValueClampTo<int>(value));
+      style->setOutlineOffset(
+          roundedClampTo<int>(animatableValueToPixels(value, state)));
       return;
     case CSSPropertyOutlineWidth:
-      style->setOutlineWidth(animatableLineWidthClamp<unsigned short>(value));
+      style->setOutlineWidth(
+          animatableLineWidthClamp<unsigned short>(value, state));
       return;
     case CSSPropertyPaddingBottom:
       style->setPaddingBottom(
           animatableValueToLength(value, state, ValueRangeNonNegative));
       return;
     case CSSPropertyPaddingLeft:
       style->setPaddingLeft(
           animatableValueToLength(value, state, ValueRangeNonNegative));
       return;
     case CSSPropertyPaddingRight:
@@ skipping 52 matching lines @@
     case CSSPropertyTextIndent:
       style->setTextIndent(animatableValueToLength(value, state));
       return;
     case CSSPropertyTextShadow:
       style->setTextShadow(toAnimatableShadow(value)->getShadowList());
       return;
     case CSSPropertyTop:
       style->setTop(animatableValueToLength(value, state));
       return;
     case CSSPropertyWebkitBorderHorizontalSpacing:
-      style->setHorizontalBorderSpacing(
-          animatableValueClampTo<unsigned short>(value));
+      style->setHorizontalBorderSpacing(roundedClampTo<unsigned short>(
+          animatableValueToPixels(value, state)));
       return;
     case CSSPropertyWebkitBorderVerticalSpacing:
-      style->setVerticalBorderSpacing(
-          animatableValueClampTo<unsigned short>(value));
+      style->setVerticalBorderSpacing(roundedClampTo<unsigned short>(
+          animatableValueToPixels(value, state)));
       return;
     case CSSPropertyClipPath:
       style->setClipPath(
           toAnimatableClipPathOperation(value)->getClipPathOperation());
       return;
     case CSSPropertyColumnCount:
       style->setColumnCount(clampTo<unsigned short>(
           round(toAnimatableDouble(value)->toDouble()), 1));
       return;
     case CSSPropertyColumnGap:
-      style->setColumnGap(clampTo(toAnimatableDouble(value)->toDouble(), 0));
+      style->setColumnGap(clampTo(animatableValueToPixels(value, state), 0));
       return;
     case CSSPropertyColumnRuleColor:
       style->setColumnRuleColor(toAnimatableColor(value)->getColor());
       style->setVisitedLinkColumnRuleColor(
           toAnimatableColor(value)->visitedLinkColor());
       return;
     case CSSPropertyColumnWidth:
-      style->setColumnWidth(clampTo(toAnimatableDouble(value)->toDouble(),
+      style->setColumnWidth(clampTo(animatableValueToPixels(value, state),
                                     std::numeric_limits<float>::epsilon()));
       return;
     case CSSPropertyColumnRuleWidth:
       style->setColumnRuleWidth(
-          animatableLineWidthClamp<unsigned short>(value));
+          animatableLineWidthClamp<unsigned short>(value, state));
       return;
     case CSSPropertyFilter:
       style->setFilter(toAnimatableFilterOperations(value)->operations());
       return;
     case CSSPropertyBackdropFilter:
       style->setBackdropFilter(
           toAnimatableFilterOperations(value)->operations());
       return;
     case CSSPropertyWebkitMaskBoxImageOutset:
       style->setMaskBoxImageOutset(
@@ skipping 25 matching lines @@
     case CSSPropertyWebkitMaskPositionY:
       setOnFillLayers<CSSPropertyWebkitMaskPositionY>(style->accessMaskLayers(),
                                                       value, state);
       return;
     case CSSPropertyWebkitMaskSize:
       setOnFillLayers<CSSPropertyWebkitMaskSize>(style->accessMaskLayers(),
                                                  value, state);
       return;
     case CSSPropertyPerspective:
       style->setPerspective(
-          value->isDouble()
-              ? clampTo<float>(toAnimatableDouble(value)->toDouble())
+          value->isLength()
+              ? clampTo<float>(animatableValueToPixels(value, state))
               : 0);
       return;
     case CSSPropertyPerspectiveOrigin:
       style->setPerspectiveOrigin(animatableValueToLengthPoint(value, state));
       return;
     case CSSPropertyShapeOutside:
       style->setShapeOutside(toAnimatableShapeValue(value)->getShapeValue());
       return;
     case CSSPropertyShapeMargin:
       style->setShapeMargin(
@@ skipping 73 matching lines @@
     case CSSPropertyWebkitPerspectiveOriginY:
       style->setPerspectiveOriginY(animatableValueToLength(value, state));
       return;
     case CSSPropertyWebkitTransformOriginX:
       style->setTransformOriginX(animatableValueToLength(value, state));
       return;
     case CSSPropertyWebkitTransformOriginY:
       style->setTransformOriginY(animatableValueToLength(value, state));
       return;
     case CSSPropertyWebkitTransformOriginZ:
-      style->setTransformOriginZ(toAnimatableDouble(value)->toDouble());
+      style->setTransformOriginZ(animatableValueToPixels(value, state));
       return;
     case CSSPropertyWidows:
       style->setWidows(
           clampTo<short>(round(toAnimatableDouble(value)->toDouble()), 1));
       return;
     case CSSPropertyWidth:
       style->setWidth(
           animatableValueToLength(value, state, ValueRangeNonNegative));
       return;
     case CSSPropertyWordSpacing:
       style->setWordSpacing(
-          clampTo<float>(toAnimatableDouble(value)->toDouble()));
+          clampTo<float>(animatableValueToPixels(value, state)));
       return;
     case CSSPropertyVerticalAlign:
       style->setVerticalAlignLength(animatableValueToLength(value, state));
       return;
     case CSSPropertyVisibility:
       style->setVisibility(toAnimatableVisibility(value)->visibility());
       return;
     case CSSPropertyZIndex:
       style->setZIndex(
           clampTo<int>(round(toAnimatableDouble(value)->toDouble())));
@@ skipping 24 matching lines @@
       style->setRy(
           animatableValueToLength(value, state, ValueRangeNonNegative));
       return;
 
     default:
       ASSERT_NOT_REACHED();
   }
 }
 
 }  // namespace blink