Simplify SVGAnimationElement::IsAdditive

Hoist the check for 'by' animations out of IsAdditive() and leave it to
only check the 'additive' attribute value.
The SVGAnimateElement::IsAdditive() override can be removed since the
additional check for non-additive types is handled during the animation
value calculation, and non-additive types are rejected while validating
the animation parameters.

While at it, randomly add some comments for additive handling, and
add more comments in OverwritesUnderlyingAnimationValue(). Also make
IsAdditive(), IsAccumulated(), GetAnimationMode() and GetCalcMode()
non-public as appropriate.

Bug: 1017723
Change-Id: I0907742ca1e568ba6faee1b08199bccad683a5e1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2379919Reviewed-by: Stephen Chenney <schenney@chromium.org>
Commit-Queue: Fredrik Söderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/master@{#803032}

third_party/blink/renderer/core/svg/svg_animate_element.cc

@@ -518,16 +518,6 @@ bool SVGAnimateElement::AnimatedPropertyTypeSupportsAddition() const {
   }
 }
 
-bool SVGAnimateElement::IsAdditive() const {
-  if (GetAnimationMode() == kByAnimation ||
-      GetAnimationMode() == kFromByAnimation) {
-    if (!AnimatedPropertyTypeSupportsAddition())
-      return false;
-  }
-
-  return SVGAnimationElement::IsAdditive();
-}
-
 float SVGAnimateElement::CalculateDistance(const String& from_string,
                                            const String& to_string) {
   DCHECK(targetElement());

third_party/blink/renderer/core/svg/svg_animate_element.h

@@ -52,7 +52,6 @@ class CORE_EXPORT SVGAnimateElement : public SVGAnimationElement {
   const QualifiedName& AttributeName() const { return attribute_name_; }
   AnimatedPropertyType GetAnimatedPropertyType() const;
   bool AnimatedPropertyTypeSupportsAddition() const;
-  bool IsAdditive() const final;
 
  protected:
   void WillChangeAnimationTarget() final;

third_party/blink/renderer/core/svg/svg_animate_motion_element.cc

@@ -211,6 +211,7 @@ void SVGAnimateMotionElement::CalculateAnimatedValue(float percentage,
   AffineTransform* transform = target_element->AnimateMotionTransform();
   DCHECK(transform);
 
+  // If additive, we accumulate into the underlying (transform) value.
   if (!parameters.is_additive)
     transform->MakeIdentity();
 

third_party/blink/renderer/core/svg/svg_animation_element.cc

@@ -351,7 +351,7 @@ String SVGAnimationElement::FromValue() const {
 bool SVGAnimationElement::IsAdditive() const {
   DEFINE_STATIC_LOCAL(const AtomicString, sum, ("sum"));
   const AtomicString& value = FastGetAttribute(svg_names::kAdditiveAttr);
-  return value == sum || GetAnimationMode() == kByAnimation;
+  return value == sum;
 }
 
 bool SVGAnimationElement::IsAccumulated() const {
@@ -654,7 +654,7 @@ SMILAnimationEffectParameters SVGAnimationElement::ComputeEffectParameters()
   SMILAnimationEffectParameters parameters;
   parameters.is_discrete = GetCalcMode() == kCalcModeDiscrete;
   parameters.is_to_animation = GetAnimationMode() == kToAnimation;
-  parameters.is_additive = IsAdditive();
+  parameters.is_additive = IsAdditive() || GetAnimationMode() == kByAnimation;
   parameters.is_cumulative = IsAccumulated();
   return parameters;
 }
@@ -664,7 +664,7 @@ void SVGAnimationElement::ApplyAnimation(SVGAnimationElement* result_element) {
     if (CheckAnimationParameters()) {
       animation_valid_ = AnimationValidity::kValid;
 
-      if (IsAdditive() ||
+      if (IsAdditive() || GetAnimationMode() == kByAnimation ||
           (IsAccumulated() && GetAnimationMode() != kToAnimation)) {
         UseCounter::Count(&GetDocument(),
                           WebFeature::kSVGSMILAdditiveAnimation);
@@ -714,11 +714,23 @@ void SVGAnimationElement::ApplyAnimation(SVGAnimationElement* result_element) {
 }
 
 bool SVGAnimationElement::OverwritesUnderlyingAnimationValue() const {
-  if (IsAdditive() || IsAccumulated())
+  // Our animation value is added to the underlying value.
+  if (IsAdditive())
+    return false;
+  // TODO(fs): Remove this. (Is a function of the repeat count and
+  // does not depend on the underlying value.)
+  if (IsAccumulated())
+    return false;
+  // Animation is from the underlying value by (adding) the specified value.
+  if (GetAnimationMode() == kByAnimation)
     return false;
-  return GetAnimationMode() != kToAnimation &&
-         GetAnimationMode() != kByAnimation &&
-         GetAnimationMode() != kNoAnimation;
+  // Animation is from the underlying value to the specified value.
+  if (GetAnimationMode() == kToAnimation)
+    return false;
+  // No animation...
+  if (GetAnimationMode() == kNoAnimation)
+    return false;
+  return true;
 }
 
 }  // namespace blink

third_party/blink/renderer/core/svg/svg_animation_element.h

@@ -71,11 +71,6 @@ class CORE_EXPORT SVGAnimationElement : public SVGSMILElement {
   DEFINE_ATTRIBUTE_EVENT_LISTENER(end, kEndEvent)
   DEFINE_ATTRIBUTE_EVENT_LISTENER(repeat, kRepeatEvent)
 
-  virtual bool IsAdditive() const;
-  bool IsAccumulated() const;
-  AnimationMode GetAnimationMode() const { return animation_mode_; }
-  CalcMode GetCalcMode() const { return calc_mode_; }
-
   virtual void ResetAnimatedType() = 0;
   virtual void ClearAnimatedType() = 0;
   virtual void ApplyResultsToTarget() = 0;
@@ -95,10 +90,13 @@ class CORE_EXPORT SVGAnimationElement : public SVGSMILElement {
   void SetAnimationMode(AnimationMode animation_mode) {
     animation_mode_ = animation_mode;
   }
+  AnimationMode GetAnimationMode() const { return animation_mode_; }
   void SetCalcMode(CalcMode calc_mode) {
     use_paced_key_times_ = false;
     calc_mode_ = calc_mode;
   }
+  CalcMode GetCalcMode() const { return calc_mode_; }
+
   virtual bool HasValidAnimation() const = 0;
   void UnregisterAnimation(const QualifiedName& attribute_name);
   void RegisterAnimation(const QualifiedName& attribute_name);
@@ -116,6 +114,9 @@ class CORE_EXPORT SVGAnimationElement : public SVGSMILElement {
  private:
   bool IsValid() const final { return SVGTests::IsValid(); }
 
+  bool IsAdditive() const;
+  bool IsAccumulated() const;
+
   String ToValue() const;
   String ByValue() const;
   String FromValue() const;

third_party/blink/renderer/core/svg/svg_transform_list.cc

@@ -473,10 +473,14 @@ void SVGTransformList::CalculateAnimatedValue(
     Append(current_transform);
     return;
   }
-  // Never resize the animatedTransformList to the toList size, instead either
-  // clear the list or append to it.
-  if (!IsEmpty() && !parameters.is_additive)
-    Clear();
+
+  // If additive, we accumulate into (append to) the underlying value.
+  if (!parameters.is_additive) {
+    // Never resize the animatedTransformList to the toList size, instead either
+    // clear the list or append to it.
+    if (!IsEmpty())
+      Clear();
+  }
 
   if (repeat_count && parameters.is_cumulative) {
     SVGTransform* effective_to_at_end =