Move SVG path blend state to its own object

Source/core/svg/SVGPathBlender.cpp

Index: Source/core/svg/SVGPathBlender.cpp
diff --git a/Source/core/svg/SVGPathBlender.cpp b/Source/core/svg/SVGPathBlender.cpp
index dbd5aabda028b20d82aeb66bd3007a5044ce4e9f..27a2ceac845ce85dc5c419e4eaba7c6db12a4699 100644
--- a/Source/core/svg/SVGPathBlender.cpp
+++ b/Source/core/svg/SVGPathBlender.cpp
@@ -24,32 +24,44 @@
 #include "core/svg/SVGPathSeg.h"
 #include "core/svg/SVGPathSource.h"
 #include "platform/animation/AnimationUtilities.h"
-#include "wtf/TemporaryChange.h"
 
 namespace blink {
 
-SVGPathBlender::SVGPathBlender(SVGPathSource* fromSource, SVGPathSource* toSource, SVGPathConsumer* consumer)
-    : m_fromSource(fromSource)
-    , m_toSource(toSource)
-    , m_consumer(consumer)
-    , m_progress(0)
-    , m_addTypesCount(0)
-    , m_isInFirstHalfOfAnimation(false)
-    , m_typesAreEqual(false)
-    , m_fromIsAbsolute(false)
-    , m_toIsAbsolute(false)
-{
-    ASSERT(m_fromSource);
-    ASSERT(m_toSource);
-    ASSERT(m_consumer);
-}
+enum FloatBlendMode {
+    BlendHorizontal,
+    BlendVertical
+};
+
+class SVGPathBlender::BlendState {
+public:
+    BlendState(float progress, unsigned addTypesCount = 0)
+        : m_progress(progress)
+        , m_addTypesCount(addTypesCount)
+        , m_isInFirstHalfOfAnimation(progress < 0.5f)
+        , m_typesAreEqual(false)
+        , m_fromIsAbsolute(false)
+    {
+    }
 
-DEFINE_TRACE(SVGPathBlender)
-{
-    visitor->trace(m_fromSource);
-    visitor->trace(m_toSource);
-    visitor->trace(m_consumer);
-}
+    bool blendSegments(const PathSegmentData& fromSeg, const PathSegmentData& toSeg, PathSegmentData&);
+
+private:
+    bool canBlend(const PathSegmentData& fromSeg, const PathSegmentData& toSeg);
+    float blendAnimatedDimensonalFloat(float, float, FloatBlendMode);
+    FloatPoint blendAnimatedFloatPoint(const FloatPoint& from, const FloatPoint& to);
+    FloatPoint blendAnimatedFloatPointSameCoordinates(const FloatPoint& from, const FloatPoint& to);
+
+    FloatPoint m_fromCurrentPoint;
+    FloatPoint m_toCurrentPoint;
+
+    float m_progress;
+    unsigned m_addTypesCount;
+    bool m_isInFirstHalfOfAnimation;
+    // This is per-segment blend state corresponding to the 'from' and 'to'
+    // segments currently being blended, and only used within blendSegments().
+    bool m_typesAreEqual;
+    bool m_fromIsAbsolute;
+};
 
 // Helper functions
 static inline FloatPoint blendFloatPoint(const FloatPoint& a, const FloatPoint& b, float progress)
@@ -57,7 +69,7 @@ static inline FloatPoint blendFloatPoint(const FloatPoint& a, const FloatPoint&
     return FloatPoint(blend(a.x(), b.x(), progress), blend(a.y(), b.y(), progress));
 }
 
-float SVGPathBlender::blendAnimatedDimensonalFloat(float from, float to, FloatBlendMode blendMode)
+float SVGPathBlender::BlendState::blendAnimatedDimensonalFloat(float from, float to, FloatBlendMode blendMode)
 {
     if (m_addTypesCount) {
         ASSERT(m_typesAreEqual);
@@ -82,7 +94,7 @@ float SVGPathBlender::blendAnimatedDimensonalFloat(float from, float to, FloatBl
     return !m_fromIsAbsolute ? animValue + currentValue : animValue - currentValue;
 }
 
-FloatPoint SVGPathBlender::blendAnimatedFloatPointSameCoordinates(const FloatPoint& fromPoint, const FloatPoint& toPoint)
+FloatPoint SVGPathBlender::BlendState::blendAnimatedFloatPointSameCoordinates(const FloatPoint& fromPoint, const FloatPoint& toPoint)

[Stephen Chennney, 2015/04/02 15:00:30]

Minor nit: Could you swap the order of this and the next to match the header
declaration order. And put canBlend before both.

[fs, 2015/04/07 08:27:28]

Assuming you wanted the order of declarations and definitions to match, I did it
the other way around instead (i.e. reordered the declarations.) This yields a
smaller (more obvious) diff. If you wanted the definitions to actually move
around for another reason let me know and I'll fix that up.

 {
     if (m_addTypesCount) {
         FloatPoint repeatedToPoint = toPoint;
@@ -92,7 +104,7 @@ FloatPoint SVGPathBlender::blendAnimatedFloatPointSameCoordinates(const FloatPoi
     return blendFloatPoint(fromPoint, toPoint, m_progress);
 }
 
-FloatPoint SVGPathBlender::blendAnimatedFloatPoint(const FloatPoint& fromPoint, const FloatPoint& toPoint)
+FloatPoint SVGPathBlender::BlendState::blendAnimatedFloatPoint(const FloatPoint& fromPoint, const FloatPoint& toPoint)
 {
     if (m_typesAreEqual)
         return blendAnimatedFloatPointSameCoordinates(fromPoint, toPoint);
@@ -119,182 +131,154 @@ FloatPoint SVGPathBlender::blendAnimatedFloatPoint(const FloatPoint& fromPoint,
     return animatedPoint;
 }
 
-PathSegmentData SVGPathBlender::blendMoveToSegment(const PathSegmentData& fromSeg, const PathSegmentData& toSeg)
-{
-    PathSegmentData blendedSegment;
-    blendedSegment.command = m_isInFirstHalfOfAnimation ? fromSeg.command : toSeg.command;
-    blendedSegment.targetPoint = blendAnimatedFloatPoint(fromSeg.targetPoint, toSeg.targetPoint);
-
-    m_fromCurrentPoint = m_fromIsAbsolute ? fromSeg.targetPoint : m_fromCurrentPoint + fromSeg.targetPoint;
-    m_toCurrentPoint = m_toIsAbsolute ? toSeg.targetPoint : m_toCurrentPoint + toSeg.targetPoint;
-    return blendedSegment;
-}
-
-PathSegmentData SVGPathBlender::blendLineToSegment(const PathSegmentData& fromSeg, const PathSegmentData& toSeg)
-{
-    PathSegmentData blendedSegment;
-    blendedSegment.command = m_isInFirstHalfOfAnimation ? fromSeg.command : toSeg.command;
-    blendedSegment.targetPoint = blendAnimatedFloatPoint(fromSeg.targetPoint, toSeg.targetPoint);
-
-    m_fromCurrentPoint = m_fromIsAbsolute ? fromSeg.targetPoint : m_fromCurrentPoint + fromSeg.targetPoint;
-    m_toCurrentPoint = m_toIsAbsolute ? toSeg.targetPoint : m_toCurrentPoint + toSeg.targetPoint;
-    return blendedSegment;
-}
-
-PathSegmentData SVGPathBlender::blendLineToHorizontalSegment(const PathSegmentData& fromSeg, const PathSegmentData& toSeg)
-{
-    PathSegmentData blendedSegment;
-    blendedSegment.command = m_isInFirstHalfOfAnimation ? fromSeg.command : toSeg.command;
-    blendedSegment.targetPoint.setX(blendAnimatedDimensonalFloat(fromSeg.targetPoint.x(), toSeg.targetPoint.x(), BlendHorizontal));
-
-    m_fromCurrentPoint.setX(m_fromIsAbsolute ? fromSeg.targetPoint.x() : m_fromCurrentPoint.x() + fromSeg.targetPoint.x());
-    m_toCurrentPoint.setX(m_toIsAbsolute ? toSeg.targetPoint.x() : m_toCurrentPoint.x() + toSeg.targetPoint.x());
-    return blendedSegment;
-}
-
-PathSegmentData SVGPathBlender::blendLineToVerticalSegment(const PathSegmentData& fromSeg, const PathSegmentData& toSeg)
-{
-    PathSegmentData blendedSegment;
-    blendedSegment.command = m_isInFirstHalfOfAnimation ? fromSeg.command : toSeg.command;
-    blendedSegment.targetPoint.setY(blendAnimatedDimensonalFloat(fromSeg.targetPoint.y(), toSeg.targetPoint.y(), BlendVertical));
-
-    m_fromCurrentPoint.setY(m_fromIsAbsolute ? fromSeg.targetPoint.y() : m_fromCurrentPoint.y() + fromSeg.targetPoint.y());
-    m_toCurrentPoint.setY(m_toIsAbsolute ? toSeg.targetPoint.y() : m_toCurrentPoint.y() + toSeg.targetPoint.y());
-    return blendedSegment;
-}
-
-PathSegmentData SVGPathBlender::blendCurveToCubicSegment(const PathSegmentData& fromSeg, const PathSegmentData& toSeg)
-{
-    PathSegmentData blendedSegment;
-    blendedSegment.command = m_isInFirstHalfOfAnimation ? fromSeg.command : toSeg.command;
-    blendedSegment.targetPoint = blendAnimatedFloatPoint(fromSeg.targetPoint, toSeg.targetPoint);
-    blendedSegment.point1 = blendAnimatedFloatPoint(fromSeg.point1, toSeg.point1);
-    blendedSegment.point2 = blendAnimatedFloatPoint(fromSeg.point2, toSeg.point2);
-
-    m_fromCurrentPoint = m_fromIsAbsolute ? fromSeg.targetPoint : m_fromCurrentPoint + fromSeg.targetPoint;
-    m_toCurrentPoint = m_toIsAbsolute ? toSeg.targetPoint : m_toCurrentPoint + toSeg.targetPoint;
-    return blendedSegment;
-}
-
-PathSegmentData SVGPathBlender::blendCurveToCubicSmoothSegment(const PathSegmentData& fromSeg, const PathSegmentData& toSeg)
+bool SVGPathBlender::BlendState::canBlend(const PathSegmentData& fromSeg, const PathSegmentData& toSeg)
 {
-    PathSegmentData blendedSegment;
-    blendedSegment.command = m_isInFirstHalfOfAnimation ? fromSeg.command : toSeg.command;
-    blendedSegment.targetPoint = blendAnimatedFloatPoint(fromSeg.targetPoint, toSeg.targetPoint);
-    blendedSegment.point2 = blendAnimatedFloatPoint(fromSeg.point2, toSeg.point2);
+    // Update state first because we'll need it if we return true below.
+    m_typesAreEqual = fromSeg.command == toSeg.command;
+    m_fromIsAbsolute = isAbsolutePathSegType(fromSeg.command);
 
-    m_fromCurrentPoint = m_fromIsAbsolute ? fromSeg.targetPoint : m_fromCurrentPoint + fromSeg.targetPoint;
-    m_toCurrentPoint = m_toIsAbsolute ? toSeg.targetPoint : m_toCurrentPoint + toSeg.targetPoint;
-    return blendedSegment;
-}
+    // If the types are equal, they'll blend regardless of parameters.
+    if (m_typesAreEqual)
+        return true;
 
-PathSegmentData SVGPathBlender::blendCurveToQuadraticSegment(const PathSegmentData& fromSeg, const PathSegmentData& toSeg)
-{
-    PathSegmentData blendedSegment;
-    blendedSegment.command = m_isInFirstHalfOfAnimation ? fromSeg.command : toSeg.command;
-    blendedSegment.targetPoint = blendAnimatedFloatPoint(fromSeg.targetPoint, toSeg.targetPoint);
-    blendedSegment.point1 = blendAnimatedFloatPoint(fromSeg.point1, toSeg.point1);
+    // Addition require segments with the same type.
+    if (m_addTypesCount)
+        return false;
 
-    m_fromCurrentPoint = m_fromIsAbsolute ? fromSeg.targetPoint : m_fromCurrentPoint + fromSeg.targetPoint;
-    m_toCurrentPoint = m_toIsAbsolute ? toSeg.targetPoint : m_toCurrentPoint + toSeg.targetPoint;
-    return blendedSegment;
+    // Allow the segments to differ in "relativeness".
+    return toAbsolutePathSegType(fromSeg.command) == toAbsolutePathSegType(toSeg.command);
 }
 
-PathSegmentData SVGPathBlender::blendCurveToQuadraticSmoothSegment(const PathSegmentData& fromSeg, const PathSegmentData& toSeg)
+static void updateCurrentPoint(FloatPoint& currentPoint, const PathSegmentData& segment)
 {
-    PathSegmentData blendedSegment;
-    blendedSegment.command = m_isInFirstHalfOfAnimation ? fromSeg.command : toSeg.command;
-    blendedSegment.targetPoint = blendAnimatedFloatPoint(fromSeg.targetPoint, toSeg.targetPoint);
-
-    m_fromCurrentPoint = m_fromIsAbsolute ? fromSeg.targetPoint : m_fromCurrentPoint + fromSeg.targetPoint;
-    m_toCurrentPoint = m_toIsAbsolute ? toSeg.targetPoint : m_toCurrentPoint + toSeg.targetPoint;
-    return blendedSegment;
+    switch (segment.command) {
+    case PathSegMoveToRel:
+    case PathSegLineToRel:
+    case PathSegCurveToCubicRel:
+    case PathSegCurveToQuadraticRel:
+    case PathSegArcRel:
+    case PathSegLineToHorizontalRel:
+    case PathSegLineToVerticalRel:

[Stephen Chennney, 2015/04/02 15:00:30]

I think I get it. segment.targetPoint is already limited to just x or y, so you
can just add it here, while below you don't want to 0-out the existing y or x
position.

[fs, 2015/04/02 15:08:30]

Right, Horizontal/VerticalRel is essentially the same as LineRel but with an
implicit 0 as the other part. Horizontal/Vertical_Abs_ on the other hand is
(weird) so need to explicitly just set one of them.

+    case PathSegCurveToCubicSmoothRel:
+    case PathSegCurveToQuadraticSmoothRel:
+        currentPoint += segment.targetPoint;
+        break;
+    case PathSegMoveToAbs:
+    case PathSegLineToAbs:
+    case PathSegCurveToCubicAbs:
+    case PathSegCurveToQuadraticAbs:
+    case PathSegArcAbs:
+    case PathSegCurveToCubicSmoothAbs:
+    case PathSegCurveToQuadraticSmoothAbs:
+        currentPoint = segment.targetPoint;
+        break;
+    case PathSegLineToHorizontalAbs:
+        currentPoint.setX(segment.targetPoint.x());
+        break;
+    case PathSegLineToVerticalAbs:
+        currentPoint.setY(segment.targetPoint.y());
+        break;
+    case PathSegClosePath:
+        break;
+    default:
+        ASSERT_NOT_REACHED();
+    }
 }
 
-PathSegmentData SVGPathBlender::blendArcToSegment(const PathSegmentData& fromSeg, const PathSegmentData& toSeg)
+bool SVGPathBlender::BlendState::blendSegments(const PathSegmentData& fromSeg, const PathSegmentData& toSeg, PathSegmentData& blendedSegment)
 {
-    ASSERT(!m_addTypesCount || fromSeg.command == toSeg.command);
+    if (!canBlend(fromSeg, toSeg))
+        return false;
 
-    PathSegmentData blendedSegment;
     blendedSegment.command = m_isInFirstHalfOfAnimation ? fromSeg.command : toSeg.command;
-    blendedSegment.targetPoint = blendAnimatedFloatPoint(fromSeg.targetPoint, toSeg.targetPoint);
-    blendedSegment.point1 = blendAnimatedFloatPointSameCoordinates(fromSeg.arcRadii(), toSeg.arcRadii());
-    blendedSegment.point2 = blendAnimatedFloatPointSameCoordinates(fromSeg.point2, toSeg.point2);
-    if (m_addTypesCount) {
-        blendedSegment.arcLarge = fromSeg.arcLarge || toSeg.arcLarge;
-        blendedSegment.arcSweep = fromSeg.arcSweep || toSeg.arcSweep;
-    } else {
-        blendedSegment.arcLarge = m_isInFirstHalfOfAnimation ? fromSeg.arcLarge : toSeg.arcLarge;
-        blendedSegment.arcSweep = m_isInFirstHalfOfAnimation ? fromSeg.arcSweep : toSeg.arcSweep;
-    }
 
-    m_fromCurrentPoint = m_fromIsAbsolute ? fromSeg.targetPoint : m_fromCurrentPoint + fromSeg.targetPoint;
-    m_toCurrentPoint = m_toIsAbsolute ? toSeg.targetPoint : m_toCurrentPoint + toSeg.targetPoint;
-    return blendedSegment;
-}
-
-void SVGPathBlender::blendSegments(const PathSegmentData& fromSeg, const PathSegmentData& toSeg)
-{
-    PathSegmentData blendedSegment;
     switch (toSeg.command) {
+    case PathSegCurveToCubicRel:
+    case PathSegCurveToCubicAbs:
+        blendedSegment.point1 = blendAnimatedFloatPoint(fromSeg.point1, toSeg.point1);
+        /* fall through */
+    case PathSegCurveToCubicSmoothRel:
+    case PathSegCurveToCubicSmoothAbs:
+        blendedSegment.point2 = blendAnimatedFloatPoint(fromSeg.point2, toSeg.point2);
+        /* fall through */
     case PathSegMoveToRel:
     case PathSegMoveToAbs:
-        blendedSegment = blendMoveToSegment(fromSeg, toSeg);
-        break;
     case PathSegLineToRel:
     case PathSegLineToAbs:
-        blendedSegment = blendLineToSegment(fromSeg, toSeg);
+    case PathSegCurveToQuadraticSmoothRel:
+    case PathSegCurveToQuadraticSmoothAbs:
+        blendedSegment.targetPoint = blendAnimatedFloatPoint(fromSeg.targetPoint, toSeg.targetPoint);
         break;
     case PathSegLineToHorizontalRel:
     case PathSegLineToHorizontalAbs:
-        blendedSegment = blendLineToHorizontalSegment(fromSeg, toSeg);
+        blendedSegment.targetPoint.setX(blendAnimatedDimensonalFloat(fromSeg.targetPoint.x(), toSeg.targetPoint.x(), BlendHorizontal));
         break;
     case PathSegLineToVerticalRel:
     case PathSegLineToVerticalAbs:
-        blendedSegment = blendLineToVerticalSegment(fromSeg, toSeg);
+        blendedSegment.targetPoint.setY(blendAnimatedDimensonalFloat(fromSeg.targetPoint.y(), toSeg.targetPoint.y(), BlendVertical));
         break;
     case PathSegClosePath:
-        blendedSegment = toSeg;
-        break;
-    case PathSegCurveToCubicRel:
-    case PathSegCurveToCubicAbs:
-        blendedSegment = blendCurveToCubicSegment(fromSeg, toSeg);
-        break;
-    case PathSegCurveToCubicSmoothRel:
-    case PathSegCurveToCubicSmoothAbs:
-        blendedSegment = blendCurveToCubicSmoothSegment(fromSeg, toSeg);
         break;
     case PathSegCurveToQuadraticRel:
     case PathSegCurveToQuadraticAbs:
-        blendedSegment = blendCurveToQuadraticSegment(fromSeg, toSeg);
-        break;
-    case PathSegCurveToQuadraticSmoothRel:
-    case PathSegCurveToQuadraticSmoothAbs:
-        blendedSegment = blendCurveToQuadraticSmoothSegment(fromSeg, toSeg);
+        blendedSegment.targetPoint = blendAnimatedFloatPoint(fromSeg.targetPoint, toSeg.targetPoint);
+        blendedSegment.point1 = blendAnimatedFloatPoint(fromSeg.point1, toSeg.point1);
         break;
     case PathSegArcRel:
     case PathSegArcAbs:
-        blendedSegment = blendArcToSegment(fromSeg, toSeg);
+        blendedSegment.targetPoint = blendAnimatedFloatPoint(fromSeg.targetPoint, toSeg.targetPoint);
+        blendedSegment.point1 = blendAnimatedFloatPointSameCoordinates(fromSeg.arcRadii(), toSeg.arcRadii());
+        blendedSegment.point2 = blendAnimatedFloatPointSameCoordinates(fromSeg.point2, toSeg.point2);
+        if (m_addTypesCount) {
+            blendedSegment.arcLarge = fromSeg.arcLarge || toSeg.arcLarge;
+            blendedSegment.arcSweep = fromSeg.arcSweep || toSeg.arcSweep;
+        } else {
+            blendedSegment.arcLarge = m_isInFirstHalfOfAnimation ? fromSeg.arcLarge : toSeg.arcLarge;
+            blendedSegment.arcSweep = m_isInFirstHalfOfAnimation ? fromSeg.arcSweep : toSeg.arcSweep;
+        }
         break;
     default:
         ASSERT_NOT_REACHED();
     }
 
-    m_consumer->emitSegment(blendedSegment);
+    updateCurrentPoint(m_fromCurrentPoint, fromSeg);
+    updateCurrentPoint(m_toCurrentPoint, toSeg);
+
+    return true;
+}
+
+SVGPathBlender::SVGPathBlender(SVGPathSource* fromSource, SVGPathSource* toSource, SVGPathConsumer* consumer)
+    : m_fromSource(fromSource)
+    , m_toSource(toSource)
+    , m_consumer(consumer)
+{
+    ASSERT(m_fromSource);
+    ASSERT(m_toSource);
+    ASSERT(m_consumer);
+}
+
+DEFINE_TRACE(SVGPathBlender)
+{
+    visitor->trace(m_fromSource);
+    visitor->trace(m_toSource);
+    visitor->trace(m_consumer);
 }
 
 bool SVGPathBlender::addAnimatedPath(unsigned repeatCount)
 {
-    TemporaryChange<unsigned> change(m_addTypesCount, repeatCount);
-    return blendAnimatedPath(0);
+    BlendState blendState(0, repeatCount);
+    return blendAnimatedPath(blendState);
 }
 
 bool SVGPathBlender::blendAnimatedPath(float progress)
 {
-    m_isInFirstHalfOfAnimation = progress < 0.5f;
-    m_progress = progress;
+    BlendState blendState(progress);
+    return blendAnimatedPath(blendState);
+}
 
-    bool fromSourceHadData = m_fromSource->hasMoreData();
+bool SVGPathBlender::blendAnimatedPath(BlendState& blendState)
+{
+    bool fromSourceIsEmpty = !m_fromSource->hasMoreData();
     while (m_toSource->hasMoreData()) {
         PathSegmentData toSeg = m_toSource->parseSegment();
         if (toSeg.command == PathSegUnknown)
@@ -309,24 +293,13 @@ bool SVGPathBlender::blendAnimatedPath(float progress)
                 return false;
         }

[Stephen Chennney, 2015/04/02 15:00:30]

The code implies that you can have an empty "from" sequence and everything will
work using the command from the "to" sequence. And in this case the "from"
sequence has the default point1, point2, targetPoint etc. Is that how it is
supposed to work?

[fs, 2015/04/02 15:08:30]

I don't know where this behavior actually stems from (the additive behavior I
suppose), but my interpretation matches yours. And all the "default" values are
0 (or false).

 
-        m_typesAreEqual = fromSeg.command == toSeg.command;
-
-        // If the types are equal, they'll blend regardless of parameters.
-        if (!m_typesAreEqual) {
-            // Addition require segments with the same type.
-            if (m_addTypesCount)
-                return false;
-            // Allow the segments to differ in "relativeness".
-            if (toAbsolutePathSegType(fromSeg.command) != toAbsolutePathSegType(toSeg.command))
-                return false;
-        }
-
-        m_fromIsAbsolute = isAbsolutePathSegType(fromSeg.command);
-        m_toIsAbsolute = isAbsolutePathSegType(toSeg.command);
+        PathSegmentData blendedSeg;
+        if (!blendState.blendSegments(fromSeg, toSeg, blendedSeg))
+            return false;
 
-        blendSegments(fromSeg, toSeg);
+        m_consumer->emitSegment(blendedSeg);
 
-        if (!fromSourceHadData)
+        if (fromSourceIsEmpty)
             continue;
         if (m_fromSource->hasMoreData() != m_toSource->hasMoreData())
             return false;