[code-health] Standardize TransformOperation::CanBlendWith()

This replaces the existing virtual implementations of
TransformOperation::CanBlendWith() with a single one that compares
PrimitiveType() values, as defined in the spec. Years ago, CanBlendWith
was only used by BlendedBoundsForBox, and returned false on operation
types not supported by that method. As these checks are currently in
BlendedBoundsForBox itself, and CanBlendWith is used in some Blend()
implementations, eliminate the return false cases in favor of the
as-specced primitive type comparison.

As CanBlendWith is now to-spec for all operation types, replace the many
ad-hoc primitive type comparisons with calls to CanBlendWith(). Change
if/return type checks in TransformOperation::Blend() to DCHECKs as the
there is already a type check at the call site in TransformOperations,
so these should always succeed. In the case of SkewTransformOperation,
replace the loose type check (which would allow blending between skewX
and skewY which have different primitive types) with CanBlendWith. This
causes no behavioral change since a primitive type comparison already
existed in the caller.

Change-Id: Ife892b72798fe81d61ff6528deff064f41bdbfbb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2432672Reviewed-by: Kevin Ellis <kevers@chromium.org>
Reviewed-by: Ian Vollick <vollick@chromium.org>
Commit-Queue: George Steel <gtsteel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#813815}

third_party/blink/renderer/platform/transforms/interpolated_transform_operation.cc

@@ -57,8 +57,8 @@ scoped_refptr<TransformOperation> InterpolatedTransformOperation::Blend(
     const TransformOperation* from,
     double progress,
     bool blend_to_identity) {
-  if (from && !from->IsSameType(*this))
-    return this;
+  DCHECK(!from || CanBlendWith(*from));
+
   TransformOperations to_operations;
   to_operations.Operations().push_back(this);
   TransformOperations from_operations;

third_party/blink/renderer/platform/transforms/interpolated_transform_operation.h

@@ -50,10 +50,6 @@ class PLATFORM_EXPORT InterpolatedTransformOperation final
         new InterpolatedTransformOperation(from, to, starting_index, progress));
   }
 
-  bool CanBlendWith(const TransformOperation& other) const override {
-    return IsSameType(other);
-  }
-
  private:
   OperationType GetType() const override { return kInterpolated; }
 

third_party/blink/renderer/platform/transforms/matrix_3d_transform_operation.cc

@@ -93,8 +93,7 @@ scoped_refptr<TransformOperation> Matrix3DTransformOperation::Blend(
     const TransformOperation* from,
     double progress,
     bool blend_to_identity) {
-  if (from && !from->IsSameType(*this))
-    return this;
+  DCHECK(!from || CanBlendWith(*from));
 
   // Convert the TransformOperations into matrices. Fail the blend operation
   // if either of the matrices is non-invertible.

third_party/blink/renderer/platform/transforms/matrix_3d_transform_operation.h

@@ -41,10 +41,6 @@ class PLATFORM_EXPORT Matrix3DTransformOperation final
 
   TransformationMatrix Matrix() const { return matrix_; }
 
-  bool CanBlendWith(const TransformOperation& other) const override {
-    return false;
-  }
-
   static bool IsMatchingOperationType(OperationType type) {
     return type == kMatrix3D;
   }

third_party/blink/renderer/platform/transforms/matrix_transform_operation.cc

@@ -64,8 +64,7 @@ scoped_refptr<TransformOperation> MatrixTransformOperation::Blend(
     const TransformOperation* from,
     double progress,
     bool blend_to_identity) {
-  if (from && !from->IsSameType(*this))
-    return this;
+  DCHECK(!from || CanBlendWith(*from));
 
   // convert the TransformOperations into matrices
   TransformationMatrix from_t;

third_party/blink/renderer/platform/transforms/matrix_transform_operation.h

@@ -52,10 +52,6 @@ class PLATFORM_EXPORT MatrixTransformOperation final
     return TransformationMatrix(a_, b_, c_, d_, e_, f_);
   }
 
-  bool CanBlendWith(const TransformOperation& other) const override {
-    return false;
-  }
-
   static bool IsMatchingOperationType(OperationType type) {
     return type == kMatrix;
   }

third_party/blink/renderer/platform/transforms/perspective_transform_operation.h

@@ -40,10 +40,6 @@ class PLATFORM_EXPORT PerspectiveTransformOperation final
 
   double Perspective() const { return p_; }
 
-  bool CanBlendWith(const TransformOperation& other) const override {
-    return IsSameType(other);
-  }
-
   static bool IsMatchingOperationType(OperationType type) {
     return type == kPerspective;
   }

third_party/blink/renderer/platform/transforms/rotate_transform_operation.cc

@@ -71,8 +71,7 @@ scoped_refptr<TransformOperation> RotateTransformOperation::Blend(
     const TransformOperation* from,
     double progress,
     bool blend_to_identity) {
-  if (from && !IsMatchingOperationType(from->GetType()))
-    return this;
+  DCHECK(!from || CanBlendWith(*from));
 
   if (blend_to_identity)
     return RotateTransformOperation::Create(
@@ -96,11 +95,6 @@ scoped_refptr<TransformOperation> RotateTransformOperation::Blend(
       Rotation::Slerp(from_rotate.rotation_, rotation_, progress), type);
 }
 
-bool RotateTransformOperation::CanBlendWith(
-    const TransformOperation& other) const {
-  return other.IsSameType(*this);
-}
-
 RotateAroundOriginTransformOperation::RotateAroundOriginTransformOperation(
     double angle,
     double origin_x,
@@ -134,8 +128,8 @@ scoped_refptr<TransformOperation> RotateAroundOriginTransformOperation::Blend(
     const TransformOperation* from,
     double progress,
     bool blend_to_identity) {
-  if (from && !from->IsSameType(*this))
-    return this;
+  DCHECK(!from || CanBlendWith(*from));
+
   if (blend_to_identity) {
     return RotateAroundOriginTransformOperation::Create(
         Angle() * (1 - progress), origin_x_, origin_y_);

third_party/blink/renderer/platform/transforms/rotate_transform_operation.h

@@ -70,9 +70,8 @@ class PLATFORM_EXPORT RotateTransformOperation : public TransformOperation {
                             double& result_angle_a,
                             double& result_angle_b);
 
-  bool CanBlendWith(const TransformOperation& other) const override;
   OperationType GetType() const override { return type_; }
-  OperationType PrimitiveType() const final { return kRotate3D; }
+  OperationType PrimitiveType() const override { return kRotate3D; }
 
   void Apply(TransformationMatrix& transform,
              const FloatSize& /*borderBoxSize*/) const override {
@@ -130,6 +129,7 @@ class PLATFORM_EXPORT RotateAroundOriginTransformOperation final
   static bool IsMatchingOperationType(OperationType type) {
     return type == kRotateAroundOrigin;
   }
+  OperationType PrimitiveType() const override { return kRotateAroundOrigin; }
 
  private:
   RotateAroundOriginTransformOperation(double angle,

third_party/blink/renderer/platform/transforms/scale_transform_operation.cc

@@ -63,8 +63,7 @@ scoped_refptr<TransformOperation> ScaleTransformOperation::Blend(
     const TransformOperation* from,
     double progress,
     bool blend_to_identity) {
-  if (from && !from->CanBlendWith(*this))
-    return this;
+  DCHECK(!from || CanBlendWith(*from));
 
   if (blend_to_identity)
     return ScaleTransformOperation::Create(
@@ -83,11 +82,4 @@ scoped_refptr<TransformOperation> ScaleTransformOperation::Blend(
       blink::Blend(from_z, z_, progress), is_3d ? kScale3D : kScale);
 }
 
-bool ScaleTransformOperation::CanBlendWith(
-    const TransformOperation& other) const {
-  return other.GetType() == kScaleX || other.GetType() == kScaleY ||
-         other.GetType() == kScaleZ || other.GetType() == kScale3D ||
-         other.GetType() == kScale;
-}
-
 }  // namespace blink

third_party/blink/renderer/platform/transforms/scale_transform_operation.h

@@ -54,8 +54,6 @@ class PLATFORM_EXPORT ScaleTransformOperation final
   double Y() const { return y_; }
   double Z() const { return z_; }
 
-  bool CanBlendWith(const TransformOperation& other) const override;
-
   void Apply(TransformationMatrix& transform, const FloatSize&) const override {
     transform.Scale3d(x_, y_, z_);
   }

third_party/blink/renderer/platform/transforms/skew_transform_operation.cc

@@ -37,8 +37,7 @@ scoped_refptr<TransformOperation> SkewTransformOperation::Blend(
     const TransformOperation* from,
     double progress,
     bool blend_to_identity) {
-  if (from && !from->CanBlendWith(*this))
-    return this;
+  DCHECK(!from || CanBlendWith(*from));
 
   if (blend_to_identity)
     return SkewTransformOperation::Create(blink::Blend(angle_x_, 0.0, progress),
@@ -54,10 +53,4 @@ scoped_refptr<TransformOperation> SkewTransformOperation::Blend(
       blink::Blend(from_angle_y, angle_y_, progress), type_);
 }
 
-bool SkewTransformOperation::CanBlendWith(
-    const TransformOperation& other) const {
-  return other.GetType() == kSkew || other.GetType() == kSkewX ||
-         other.GetType() == kSkewY;
-}
-
 }  // namespace blink

third_party/blink/renderer/platform/transforms/skew_transform_operation.h

@@ -42,8 +42,6 @@ class PLATFORM_EXPORT SkewTransformOperation final : public TransformOperation {
   double AngleX() const { return angle_x_; }
   double AngleY() const { return angle_y_; }
 
-  bool CanBlendWith(const TransformOperation& other) const override;
-
   static bool IsMatchingOperationType(OperationType type) {
     return type == kSkewX || type == kSkewY || type == kSkew;
   }

third_party/blink/renderer/platform/transforms/transform_operation.h

@@ -92,7 +92,9 @@ class PLATFORM_EXPORT TransformOperation
   bool IsSameType(const TransformOperation& other) const {
     return other.GetType() == GetType();
   }
-  virtual bool CanBlendWith(const TransformOperation& other) const = 0;
+  bool CanBlendWith(const TransformOperation& other) const {
+    return PrimitiveType() == other.PrimitiveType();
+  }
 
   virtual bool PreservesAxisAlignment() const { return false; }
 

third_party/blink/renderer/platform/transforms/transform_operations.cc

@@ -98,10 +98,9 @@ wtf_size_t TransformOperations::MatchingPrefixLength(
   wtf_size_t num_operations =
       std::min(Operations().size(), other.Operations().size());
   for (wtf_size_t i = 0; i < num_operations; ++i) {
-    if (Operations()[i]->PrimitiveType() !=
-        other.Operations()[i]->PrimitiveType()) {
-      // Remaining operations in each operations list require matrix/matrix3d
-      // interpolation.
+    if (!Operations()[i]->CanBlendWith(*other.Operations()[i])) {
+      // Remaining operations in each operations list require merging for
+      // matrix/matrix3d interpolation.
       return i;
     }
   }

third_party/blink/renderer/platform/transforms/transform_operations_test.cc

@@ -571,6 +571,75 @@ TEST(TransformOperationsTest, PerspectiveOpsTest) {
   EXPECT_TRUE(ops.HasNonTrivial3DComponent());
 }
 
+TEST(TransformOperationsTest, CanBlendWithSkewTest) {
+  TransformOperations ops_x, ops_y, ops_skew, ops_skew2;
+  ops_x.Operations().push_back(
+      SkewTransformOperation::Create(45, 0, TransformOperation::kSkewX));
+  ops_y.Operations().push_back(
+      SkewTransformOperation::Create(0, 45, TransformOperation::kSkewY));
+  ops_skew.Operations().push_back(
+      SkewTransformOperation::Create(45, 0, TransformOperation::kSkew));
+  ops_skew2.Operations().push_back(
+      SkewTransformOperation::Create(0, 45, TransformOperation::kSkew));
+
+  EXPECT_TRUE(ops_x.Operations()[0]->CanBlendWith(*ops_x.Operations()[0]));
+  EXPECT_TRUE(ops_y.Operations()[0]->CanBlendWith(*ops_y.Operations()[0]));
+
+  EXPECT_FALSE(ops_x.Operations()[0]->CanBlendWith(*ops_y.Operations()[0]));
+  EXPECT_FALSE(ops_x.Operations()[0]->CanBlendWith(*ops_skew.Operations()[0]));
+  EXPECT_FALSE(ops_y.Operations()[0]->CanBlendWith(*ops_skew.Operations()[0]));
+
+  EXPECT_TRUE(
+      ops_skew.Operations()[0]->CanBlendWith(*ops_skew2.Operations()[0]));
+
+  ASSERT_TRUE(IsA<SkewTransformOperation>(
+      *ops_skew.Blend(ops_skew2, 0.5).Operations()[0]));
+  ASSERT_TRUE(IsA<Matrix3DTransformOperation>(
+      *ops_x.Blend(ops_y, 0.5).Operations()[0]));
+}
+
+TEST(TransformOperationsTest, CanBlendWithMatrixTest) {
+  TransformOperations ops_a, ops_b;
+  ops_a.Operations().push_back(
+      MatrixTransformOperation::Create(1, 0, 0, 1, 0, 0));
+  ops_a.Operations().push_back(
+      RotateTransformOperation::Create(0, TransformOperation::kRotate));
+  ops_b.Operations().push_back(
+      MatrixTransformOperation::Create(2, 0, 0, 2, 0, 0));
+  ops_b.Operations().push_back(
+      RotateTransformOperation::Create(360, TransformOperation::kRotate));
+
+  EXPECT_TRUE(ops_a.Operations()[0]->CanBlendWith(*ops_b.Operations()[0]));
+
+  TransformOperations ops_blended = ops_a.Blend(ops_b, 0.5);
+  ASSERT_EQ(ops_blended.Operations().size(), 2u);
+  ASSERT_TRUE(IsA<MatrixTransformOperation>(*ops_blended.Operations()[0]));
+  ASSERT_TRUE(IsA<RotateTransformOperation>(*ops_blended.Operations()[1]));
+  EXPECT_EQ(To<RotateTransformOperation>(*ops_blended.Operations()[1]).Angle(),
+            180.0);
+}
+
+TEST(TransformOperationsTest, CanBlendWithMatrix3DTest) {
+  TransformOperations ops_a, ops_b;
+  ops_a.Operations().push_back(Matrix3DTransformOperation::Create(
+      TransformationMatrix(1, 0, 0, 1, 0, 0)));
+  ops_a.Operations().push_back(
+      RotateTransformOperation::Create(0, TransformOperation::kRotate));
+  ops_b.Operations().push_back(Matrix3DTransformOperation::Create(
+      TransformationMatrix(2, 0, 0, 2, 0, 0)));
+  ops_b.Operations().push_back(
+      RotateTransformOperation::Create(360, TransformOperation::kRotate));
+
+  EXPECT_TRUE(ops_a.Operations()[0]->CanBlendWith(*ops_b.Operations()[0]));
+
+  TransformOperations ops_blended = ops_a.Blend(ops_b, 0.5);
+  ASSERT_EQ(ops_blended.Operations().size(), 2u);
+  ASSERT_TRUE(IsA<Matrix3DTransformOperation>(*ops_blended.Operations()[0]));
+  ASSERT_TRUE(IsA<RotateTransformOperation>(*ops_blended.Operations()[1]));
+  EXPECT_EQ(To<RotateTransformOperation>(*ops_blended.Operations()[1]).Angle(),
+            180.0);
+}
+
 TEST(TransformOperationsTest, InterpolatedTransformBlendIdentityTest) {
   // When interpolating transform lists of differing lengths, the length of the
   // shorter list behaves as if it is padded with identity transforms.

third_party/blink/renderer/platform/transforms/translate_transform_operation.cc

@@ -74,8 +74,7 @@ scoped_refptr<TransformOperation> TranslateTransformOperation::Blend(
     const TransformOperation* from,
     double progress,
     bool blend_to_identity) {
-  if (from && !from->CanBlendWith(*this))
-    return this;
+  DCHECK(!from || CanBlendWith(*from));
 
   const Length zero_length = Length::Fixed(0);
   if (blend_to_identity) {
@@ -97,13 +96,6 @@ scoped_refptr<TransformOperation> TranslateTransformOperation::Blend(
       blink::Blend(from_z, z_, progress), is_3d ? kTranslate3D : kTranslate);
 }
 
-bool TranslateTransformOperation::CanBlendWith(
-    const TransformOperation& other) const {
-  return other.GetType() == kTranslate || other.GetType() == kTranslateX ||
-         other.GetType() == kTranslateY || other.GetType() == kTranslateZ ||
-         other.GetType() == kTranslate3D;
-}
-
 scoped_refptr<TranslateTransformOperation>
 TranslateTransformOperation::ZoomTranslate(double factor) {
   return Create(x_.Zoom(factor), y_.Zoom(factor), z_ * factor, type_);

third_party/blink/renderer/platform/transforms/translate_transform_operation.h

@@ -52,7 +52,6 @@ class PLATFORM_EXPORT TranslateTransformOperation final
     return *this == static_cast<const TransformOperation&>(other);
   }
 
-  bool CanBlendWith(const TransformOperation& other) const override;
   bool DependsOnBoxSize() const override {
     return x_.IsPercentOrCalc() || y_.IsPercentOrCalc();
   }