Open-code SVGAnimatedProperty::CreateAnimatedValue()

This function is only called in one place - when creating the underlying value for a SMIL animation - and all implementations do the same thing. Moving this out of the property interface will make the animation code less spread out. In the future the SVG OM types may not be used for SMIL animations. Use a switch on the property type in the animation code instead of this virtual type-dispatching function. Add a CreateUnderlyingValueForAnimation() to the SVGAnimateElement hierarchy to hold this operation. Bug: 1017723 Change-Id: Ibaa00953183c774519760f2a3950870bbcdd8b75 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4111947 Auto-Submit: Fredrik Söderquist <fs@opera.com> Reviewed-by: Philip Rogers <pdr@chromium.org> Commit-Queue: Philip Rogers <pdr@chromium.org> Cr-Commit-Position: refs/heads/main@{#1084705}
chromium · Dec 17, 2022 · a485217 · a485217
1 parent e5686c8
commit a485217
Show file tree

Hide file tree

Showing 11 changed files with 119 additions and 25 deletions.
diff --git a/third_party/blink/renderer/core/svg/properties/svg_animated_property.h b/third_party/blink/renderer/core/svg/properties/svg_animated_property.h
@@ -54,7 +54,6 @@ class SVGAnimatedPropertyBase : public GarbageCollectedMixin {
   virtual const SVGPropertyBase& BaseValueBase() const = 0;
   virtual bool IsAnimating() const = 0;
 
-  virtual SVGPropertyBase* CreateAnimatedValue() = 0;
   virtual void SetAnimatedValue(SVGPropertyBase*) = 0;
   virtual void AnimationEnded() = 0;
 
@@ -142,10 +141,6 @@ class SVGAnimatedPropertyCommon : public SVGAnimatedPropertyBase {
     return parse_status;
   }
 
-  SVGPropertyBase* CreateAnimatedValue() override {
-    return base_value_->Clone();
-  }
-
   void SetAnimatedValue(SVGPropertyBase* value) override {
     DCHECK_EQ(value->GetType(), Property::ClassType());
     current_value_ = static_cast<Property*>(value);

diff --git a/third_party/blink/renderer/core/svg/svg_animate_element.cc b/third_party/blink/renderer/core/svg/svg_animate_element.cc
@@ -32,10 +32,20 @@
 #include "third_party/blink/renderer/core/svg/animation/smil_animation_value.h"
 #include "third_party/blink/renderer/core/svg/properties/svg_animated_property.h"
 #include "third_party/blink/renderer/core/svg/properties/svg_property.h"
+#include "third_party/blink/renderer/core/svg/svg_angle.h"
 #include "third_party/blink/renderer/core/svg/svg_animated_color.h"
+#include "third_party/blink/renderer/core/svg/svg_boolean.h"
+#include "third_party/blink/renderer/core/svg/svg_integer.h"
+#include "third_party/blink/renderer/core/svg/svg_integer_optional_integer.h"
 #include "third_party/blink/renderer/core/svg/svg_length.h"
 #include "third_party/blink/renderer/core/svg/svg_length_list.h"
 #include "third_party/blink/renderer/core/svg/svg_number.h"
+#include "third_party/blink/renderer/core/svg/svg_number_list.h"
+#include "third_party/blink/renderer/core/svg/svg_number_optional_number.h"
+#include "third_party/blink/renderer/core/svg/svg_path.h"
+#include "third_party/blink/renderer/core/svg/svg_point_list.h"
+#include "third_party/blink/renderer/core/svg/svg_preserve_aspect_ratio.h"
+#include "third_party/blink/renderer/core/svg/svg_rect.h"
 #include "third_party/blink/renderer/core/svg/svg_string.h"
 #include "third_party/blink/renderer/core/xlink_names.h"
 #include "third_party/blink/renderer/platform/heap/garbage_collected.h"
@@ -233,6 +243,58 @@ SVGPropertyBase* SVGAnimateElement::CreatePropertyForAttributeAnimation(
   return target_property_->BaseValueBase().CloneForAnimation(value);
 }
 
+SVGPropertyBase* SVGAnimateElement::CreateUnderlyingValueForAttributeAnimation()
+    const {
+  // SVG DOM animVal animation code-path.
+  DCHECK_NE(type_, kAnimatedTransformList);
+  DCHECK(target_property_);
+  const SVGPropertyBase& base_value = target_property_->BaseValueBase();
+  switch (base_value.GetType()) {
+    case kAnimatedAngle:
+      return To<SVGAngle>(base_value).Clone();
+    case kAnimatedBoolean:
+      return To<SVGBoolean>(base_value).Clone();
+    case kAnimatedEnumeration:
+      return To<SVGEnumeration>(base_value).Clone();
+    case kAnimatedInteger:
+      return To<SVGInteger>(base_value).Clone();
+    case kAnimatedIntegerOptionalInteger:
+      return To<SVGIntegerOptionalInteger>(base_value).Clone();
+    case kAnimatedLength:
+      return To<SVGLength>(base_value).Clone();
+    case kAnimatedLengthList:
+      return To<SVGLengthList>(base_value).Clone();
+    case kAnimatedNumber:
+      return To<SVGNumber>(base_value).Clone();
+    case kAnimatedNumberList:
+      return To<SVGNumberList>(base_value).Clone();
+    case kAnimatedNumberOptionalNumber:
+      return To<SVGNumberOptionalNumber>(base_value).Clone();
+    case kAnimatedPath:
+      return To<SVGPath>(base_value).Clone();
+    case kAnimatedPoints:
+      return To<SVGPointList>(base_value).Clone();
+    case kAnimatedPreserveAspectRatio:
+      return To<SVGPreserveAspectRatio>(base_value).Clone();
+    case kAnimatedRect:
+      return To<SVGRect>(base_value).Clone();
+    case kAnimatedString:
+      return To<SVGString>(base_value).Clone();
+
+    // The following are either not animated or are not animated as
+    // attributeType=XML. <animateTransform> handles the transform-list case.
+    case kAnimatedUnknown:
+    case kAnimatedColor:
+    case kAnimatedPoint:
+    case kAnimatedStringList:
+    case kAnimatedTransform:
+    case kAnimatedTransformList:
+    default:
+      NOTREACHED();
+      return nullptr;
+  }
+}
+
 SVGPropertyBase* SVGAnimateElement::CreatePropertyForCSSAnimation(
     const String& value) const {
   // CSS properties animation code-path.
@@ -416,24 +478,26 @@ bool SVGAnimateElement::CalculateFromAndByValues(const String& from_string,
   return true;
 }
 
-SMILAnimationValue SVGAnimateElement::CreateAnimationValue() const {
+SVGPropertyBase* SVGAnimateElement::CreateUnderlyingValueForAnimation() const {
   DCHECK(targetElement());
-  SMILAnimationValue animation_value;
   if (IsAnimatingSVGDom()) {
     // SVG DOM animVal animation code-path.
-    animation_value.property_value = target_property_->CreateAnimatedValue();
-    DCHECK_EQ(animation_value.property_value->GetType(), type_);
-  } else {
-    DCHECK(IsAnimatingCSSProperty());
-    // Presentation attributes that have an SVG DOM representation should use
-    // the "SVG DOM" code-path (above.)
-    DCHECK(SVGElement::IsAnimatableCSSProperty(AttributeName()));
-
-    // CSS properties animation code-path.
-    String base_value =
-        ComputeCSSPropertyValue(targetElement(), css_property_id_);
-    animation_value.property_value = CreatePropertyForCSSAnimation(base_value);
+    return CreateUnderlyingValueForAttributeAnimation();
   }
+  DCHECK(IsAnimatingCSSProperty());
+  // Presentation attributes that have an SVG DOM representation should use
+  // the "SVG DOM" code-path (above.)
+  DCHECK(SVGElement::IsAnimatableCSSProperty(AttributeName()));
+
+  // CSS properties animation code-path.
+  String base_value =
+      ComputeCSSPropertyValue(targetElement(), css_property_id_);
+  return CreatePropertyForCSSAnimation(base_value);
+}
+
+SMILAnimationValue SVGAnimateElement::CreateAnimationValue() const {
+  SMILAnimationValue animation_value;
+  animation_value.property_value = CreateUnderlyingValueForAnimation();
   return animation_value;
 }
 

diff --git a/third_party/blink/renderer/core/svg/svg_animate_element.h b/third_party/blink/renderer/core/svg/svg_animate_element.h
@@ -104,7 +104,9 @@ class CORE_EXPORT SVGAnimateElement : public SVGAnimationElement {
   void WillChangeAnimatedType();
   void DidChangeAnimatedType();
 
+  virtual SVGPropertyBase* CreateUnderlyingValueForAnimation() const;
   virtual SVGPropertyBase* ParseValue(const String&) const;
+  SVGPropertyBase* CreateUnderlyingValueForAttributeAnimation() const;
   SVGPropertyBase* CreatePropertyForAttributeAnimation(const String&) const;
   SVGPropertyBase* CreatePropertyForCSSAnimation(const String&) const;
 

diff --git a/third_party/blink/renderer/core/svg/svg_animate_transform_element.cc b/third_party/blink/renderer/core/svg/svg_animate_transform_element.cc
@@ -55,6 +55,12 @@ void SVGAnimateTransformElement::ResolveTargetProperty() {
   css_property_id_ = CSSPropertyID::kInvalid;
 }
 
+SVGPropertyBase* SVGAnimateTransformElement::CreateUnderlyingValueForAnimation()
+    const {
+  DCHECK(IsAnimatingSVGDom());
+  return To<SVGTransformList>(target_property_->BaseValueBase()).Clone();
+}
+
 SVGPropertyBase* SVGAnimateTransformElement::ParseValue(
     const String& value) const {
   DCHECK(IsAnimatingSVGDom());

diff --git a/third_party/blink/renderer/core/svg/svg_animate_transform_element.h b/third_party/blink/renderer/core/svg/svg_animate_transform_element.h
@@ -40,6 +40,7 @@ class SVGAnimateTransformElement final : public SVGAnimateElement {
 
   void ParseAttribute(const AttributeModificationParams&) override;
 
+  SVGPropertyBase* CreateUnderlyingValueForAnimation() const override;
   SVGPropertyBase* ParseValue(const String&) const override;
 
   SVGTransformType transform_type_;

diff --git a/third_party/blink/renderer/core/svg/svg_boolean.h b/third_party/blink/renderer/core/svg/svg_boolean.h
@@ -34,6 +34,7 @@
 #include "third_party/blink/renderer/core/svg/properties/svg_property_helper.h"
 #include "third_party/blink/renderer/core/svg/svg_parsing_error.h"
 #include "third_party/blink/renderer/platform/heap/garbage_collected.h"
+#include "third_party/blink/renderer/platform/wtf/casting.h"
 
 namespace blink {
 
@@ -71,6 +72,13 @@ class SVGBoolean final : public SVGPropertyHelper<SVGBoolean> {
   bool value_;
 };
 
+template <>
+struct DowncastTraits<SVGBoolean> {
+  static bool AllowFrom(const SVGPropertyBase& value) {
+    return value.GetType() == SVGBoolean::ClassType();
+  }
+};
+
 }  // namespace blink
 
 #endif  // THIRD_PARTY_BLINK_RENDERER_CORE_SVG_SVG_BOOLEAN_H_
diff --git a/third_party/blink/renderer/core/svg/svg_enumeration.h b/third_party/blink/renderer/core/svg/svg_enumeration.h
@@ -35,6 +35,7 @@
 #include "third_party/blink/renderer/core/svg/properties/svg_property.h"
 #include "third_party/blink/renderer/core/svg/svg_parsing_error.h"
 #include "third_party/blink/renderer/platform/heap/garbage_collected.h"
+#include "third_party/blink/renderer/platform/wtf/casting.h"
 
 namespace blink {
 
@@ -115,6 +116,13 @@ class SVGEnumeration : public SVGPropertyBase {
   const SVGEnumerationMap& map_;
 };
 
+template <>
+struct DowncastTraits<SVGEnumeration> {
+  static bool AllowFrom(const SVGPropertyBase& value) {
+    return value.GetType() == SVGEnumeration::ClassType();
+  }
+};
+
 #define DECLARE_SVG_ENUM_MAP(cpp_enum_type) \
   template <>                               \
   const SVGEnumerationMap& GetEnumerationMap<cpp_enum_type>()

diff --git a/third_party/blink/renderer/core/svg/svg_preserve_aspect_ratio.h b/third_party/blink/renderer/core/svg/svg_preserve_aspect_ratio.h
@@ -23,6 +23,7 @@
 
 #include "third_party/blink/renderer/core/svg/properties/svg_property_helper.h"
 #include "third_party/blink/renderer/core/svg/svg_parsing_error.h"
+#include "third_party/blink/renderer/platform/wtf/casting.h"
 
 namespace gfx {
 class RectF;
@@ -114,6 +115,13 @@ class SVGPreserveAspectRatio final
   SVGMeetOrSliceType meet_or_slice_;
 };
 
+template <>
+struct DowncastTraits<SVGPreserveAspectRatio> {
+  static bool AllowFrom(const SVGPropertyBase& value) {
+    return value.GetType() == SVGPreserveAspectRatio::ClassType();
+  }
+};
+
 }  // namespace blink
 
 #endif  // THIRD_PARTY_BLINK_RENDERER_CORE_SVG_SVG_PRESERVE_ASPECT_RATIO_H_
diff --git a/third_party/blink/renderer/core/svg/svg_static_string_list.cc b/third_party/blink/renderer/core/svg/svg_static_string_list.cc
@@ -61,11 +61,6 @@ bool SVGStaticStringList::IsAnimating() const {
   return false;
 }
 
-SVGPropertyBase* SVGStaticStringList::CreateAnimatedValue() {
-  NOTREACHED();
-  return nullptr;
-}
-
 void SVGStaticStringList::SetAnimatedValue(SVGPropertyBase*) {
   NOTREACHED();
 }

diff --git a/third_party/blink/renderer/core/svg/svg_static_string_list.h b/third_party/blink/renderer/core/svg/svg_static_string_list.h
@@ -61,7 +61,6 @@ class SVGStaticStringList final : public GarbageCollected<SVGStaticStringList>,
   // SVGAnimatedPropertyBase:
   const SVGPropertyBase& BaseValueBase() const override;
   bool IsAnimating() const override;
-  SVGPropertyBase* CreateAnimatedValue() override;
   void SetAnimatedValue(SVGPropertyBase*) override;
   void AnimationEnded() override;
 

diff --git a/third_party/blink/renderer/core/svg/svg_string.h b/third_party/blink/renderer/core/svg/svg_string.h
@@ -34,6 +34,7 @@
 #include "third_party/blink/renderer/core/svg/properties/svg_property.h"
 #include "third_party/blink/renderer/core/svg/svg_parsing_error.h"
 #include "third_party/blink/renderer/platform/heap/garbage_collected.h"
+#include "third_party/blink/renderer/platform/wtf/casting.h"
 
 namespace blink {
 
@@ -79,6 +80,13 @@ class SVGString final : public SVGPropertyBase {
   String value_;
 };
 
+template <>
+struct DowncastTraits<SVGString> {
+  static bool AllowFrom(const SVGPropertyBase& value) {
+    return value.GetType() == SVGString::ClassType();
+  }
+};
+
 }  // namespace blink
 
 #endif  // THIRD_PARTY_BLINK_RENDERER_CORE_SVG_SVG_STRING_H_