Fix matrix3d transform under page zoom

third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp

 /*
  * Copyright (C) 2004 Zack Rusin <zack@kde.org>
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012 Apple Inc.
  * All rights reserved.
  * Copyright (C) 2007 Alexey Proskuryakov <ap@webkit.org>
  * Copyright (C) 2007 Nicholas Shanks <webkit@nickshanks.com>
  * Copyright (C) 2011 Sencha, Inc. All rights reserved.
  * Copyright (C) 2015 Google Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
@@ skipping 72 matching lines @@
 }
 
 inline static CSSValue* zoomAdjustedPixelValueOrAuto(
     const Length& length,
     const ComputedStyle& style) {
   if (length.isAuto())
     return CSSIdentifierValue::create(CSSValueAuto);
   return zoomAdjustedPixelValue(length.value(), style);
 }
 
-inline static CSSPrimitiveValue* zoomAdjustedNumberValue(
-    double value,
-    const ComputedStyle& style) {
-  return CSSPrimitiveValue::create(value / style.effectiveZoom(),
-                                   CSSPrimitiveValue::UnitType::Number);
-}
-
 static CSSValue* zoomAdjustedPixelValueForLength(const Length& length,
                                                  const ComputedStyle& style) {
   if (length.isFixed())
     return zoomAdjustedPixelValue(length.value(), style);
   return CSSValue::create(length, style.effectiveZoom());
 }
 
 static CSSValue* pixelValueForUnzoomedLength(
     const UnzoomedLength& unzoomedLength,
     const ComputedStyle& style) {
@@ skipping 1239 matching lines @@
   CSSValueList& list = *valuesForBorderRadiusCorner(radius, style);
   if (list.item(0).equals(list.item(1)))
     return list.item(0);
   return list;
 }
 
 static CSSFunctionValue* valueForMatrixTransform(
     const TransformationMatrix& transform,
     const ComputedStyle& style) {
   CSSFunctionValue* transformValue = nullptr;
+  double zoomFactor = style.effectiveZoom();

[pdr., 2016/11/08 07:04:08]

I wonder if we could simplify this by moving it up into computedTransform?

Something like:

computedTransform(...) {
  ...
  TransformationMatrix matrix;
  ...
  if (style.effectiveZoom() != 1) {
    if (affine) {
      ...
    } else {
      ...
    }
  }
  then return the list
}

[Franklin Ta, 2016/11/08 07:52:18]

There are a lot of other functions in this file in the same style (valueForBlah)
that could be inlined but aren't. I think we should just stick with it? Let me
know if you feel strongly about this.

[Franklin Ta, 2016/11/08 16:47:02]

Terminology note: I just noticed you guys define TransformationMatrix::isAffine
as the affine form for 2d points but in my description when I say affine I meant
the form for 3d points (i.e., the bottom row has to be [0, 0, 0, 1]). Hope that
wasn't confusing.

   if (transform.isAffine()) {
     transformValue = CSSFunctionValue::create(CSSValueMatrix);
 
     transformValue->append(*CSSPrimitiveValue::create(
         transform.a(), CSSPrimitiveValue::UnitType::Number));
     transformValue->append(*CSSPrimitiveValue::create(
         transform.b(), CSSPrimitiveValue::UnitType::Number));
     transformValue->append(*CSSPrimitiveValue::create(
         transform.c(), CSSPrimitiveValue::UnitType::Number));
     transformValue->append(*CSSPrimitiveValue::create(
         transform.d(), CSSPrimitiveValue::UnitType::Number));
-    transformValue->append(*zoomAdjustedNumberValue(transform.e(), style));
-    transformValue->append(*zoomAdjustedNumberValue(transform.f(), style));
+    transformValue->append(*CSSPrimitiveValue::create(
+        transform.e() / zoomFactor, CSSPrimitiveValue::UnitType::Number));
+    transformValue->append(*CSSPrimitiveValue::create(
+        transform.f() / zoomFactor, CSSPrimitiveValue::UnitType::Number));
   } else {
     transformValue = CSSFunctionValue::create(CSSValueMatrix3d);
 
     transformValue->append(*CSSPrimitiveValue::create(
         transform.m11(), CSSPrimitiveValue::UnitType::Number));
     transformValue->append(*CSSPrimitiveValue::create(
         transform.m12(), CSSPrimitiveValue::UnitType::Number));
     transformValue->append(*CSSPrimitiveValue::create(
         transform.m13(), CSSPrimitiveValue::UnitType::Number));
     transformValue->append(*CSSPrimitiveValue::create(
-        transform.m14(), CSSPrimitiveValue::UnitType::Number));
+        transform.m14() * zoomFactor, CSSPrimitiveValue::UnitType::Number));
 
     transformValue->append(*CSSPrimitiveValue::create(
         transform.m21(), CSSPrimitiveValue::UnitType::Number));
     transformValue->append(*CSSPrimitiveValue::create(
         transform.m22(), CSSPrimitiveValue::UnitType::Number));
     transformValue->append(*CSSPrimitiveValue::create(
         transform.m23(), CSSPrimitiveValue::UnitType::Number));
     transformValue->append(*CSSPrimitiveValue::create(
-        transform.m24(), CSSPrimitiveValue::UnitType::Number));
+        transform.m24() * zoomFactor, CSSPrimitiveValue::UnitType::Number));
 
     transformValue->append(*CSSPrimitiveValue::create(
         transform.m31(), CSSPrimitiveValue::UnitType::Number));
     transformValue->append(*CSSPrimitiveValue::create(
         transform.m32(), CSSPrimitiveValue::UnitType::Number));
     transformValue->append(*CSSPrimitiveValue::create(
         transform.m33(), CSSPrimitiveValue::UnitType::Number));
     transformValue->append(*CSSPrimitiveValue::create(
-        transform.m34(), CSSPrimitiveValue::UnitType::Number));
+        transform.m34() * zoomFactor, CSSPrimitiveValue::UnitType::Number));
 
-    transformValue->append(*zoomAdjustedNumberValue(transform.m41(), style));
-    transformValue->append(*zoomAdjustedNumberValue(transform.m42(), style));
-    transformValue->append(*zoomAdjustedNumberValue(transform.m43(), style));
+    transformValue->append(*CSSPrimitiveValue::create(
+        transform.m41() / zoomFactor, CSSPrimitiveValue::UnitType::Number));
+    transformValue->append(*CSSPrimitiveValue::create(
+        transform.m42() / zoomFactor, CSSPrimitiveValue::UnitType::Number));
+    transformValue->append(*CSSPrimitiveValue::create(
+        transform.m43() / zoomFactor, CSSPrimitiveValue::UnitType::Number));

[alancutter (OOO until 2018), 2016/11/09 02:35:59]

I'd push the Matrix3DTransformOperation::zoom() logic up into
TransformationMatrix and then you can just call transform.zoom(1 / zoomFactor)
here instead of duplicating how zooming works in the reverse direction.

[Franklin Ta, 2016/11/09 07:12:06]

This is a great idea and tidies up everything! Done.

     transformValue->append(*CSSPrimitiveValue::create(
         transform.m44(), CSSPrimitiveValue::UnitType::Number));
   }
 
   return transformValue;
 }
 
 static CSSValue* computedTransform(const LayoutObject* layoutObject,
                                    const ComputedStyle& style) {
   if (!layoutObject || !style.hasTransform())
@@ skipping 2204 matching lines @@
     case CSSPropertyAll:
       return nullptr;
     default:
       break;
   }
   ASSERT_NOT_REACHED();
   return nullptr;
 }
 
 }  // namespace blink