[webkit-reviews] review granted: [Bug 109010] SVG paced value animations overwrite user-provided keyTimes : [Attachment 425821] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has granted Said Abou-Hallawa
<sabouhallawa at apple.com>'s request for review:
Bug 109010: SVG paced value animations overwrite user-provided keyTimes
https://bugs.webkit.org/show_bug.cgi?id=109010

Attachment 425821: Patch

https://bugs.webkit.org/attachment.cgi?id=425821&action=review




--- Comment #2 from Ryosuke Niwa <rniwa at webkit.org> ---
Comment on attachment 425821
  --> https://bugs.webkit.org/attachment.cgi?id=425821
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=425821&action=review

> Source/WebCore/svg/SVGAnimationElement.cpp:385
> -	   if (m_keyTimes[index] > percent)
> +	   if (keyTimes()[index] > percent)

Can we store a reference instead of keep calling this function?

> Source/WebCore/svg/SVGAnimationElement.cpp:414
> +    float fromPercent = keyTimes()[index];
> +    float toPercent = keyTimes()[index + 1];

Ditto.

> Source/WebCore/svg/SVGAnimationElement.cpp:432
> +    if (calcMode() == CalcMode::Discrete && keyTimes().size() == 2)

Ditto.

> Source/WebCore/svg/SVGAnimationElement.cpp:489
> +	   fromPercent = keyTimes()[index];
> +	   toPercent = keyTimes()[index + 1];

Ditto.

> Source/WebCore/svg/SVGAnimationElement.cpp:537
> +	   && (hasAttributeWithoutSynchronization(SVGNames::keyPointsAttr) &&
hasAttributeWithoutSynchronization(SVGNames::keyTimesAttr) &&
(keyTimes().size() < 2 || keyTimes().size() != m_keyPoints.size())))

Ditto.

> Source/WebCore/svg/SVGAnimationElement.cpp:552
> +	       && (calcMode == CalcMode::Discrete || !keyTimes().size() ||
keyTimes().last() == 1)

Ditto.

> Source/WebCore/svg/SVGAnimationElement.cpp:554
> +	       && (!hasAttributeWithoutSynchronization(SVGNames::keyPointsAttr)
|| (keyTimes().size() > 1 && keyTimes().size() == m_keyPoints.size()));

Ditto.

> Source/WebCore/svg/SVGAnimationElement.cpp:560
> +	   m_animationValid = calcMode == CalcMode::Paced ||
!hasAttributeWithoutSynchronization(SVGNames::keyPointsAttr) ||
(keyTimes().size() > 1 && keyTimes().size() == m_keyPoints.size());

Ditto.