Add support for new canvas ellipse method.

Source/core/html/canvas/CanvasPathMethods.cpp

Index: Source/core/html/canvas/CanvasPathMethods.cpp
diff --git a/Source/core/html/canvas/CanvasPathMethods.cpp b/Source/core/html/canvas/CanvasPathMethods.cpp
index 0226af21d52095cce66cdd6c9ba0d00b2dd05aa8..566e94ce93a34d0ea0b14f5d320a34fa74bfd31c 100644
--- a/Source/core/html/canvas/CanvasPathMethods.cpp
+++ b/Source/core/html/canvas/CanvasPathMethods.cpp
@@ -130,37 +130,126 @@ void CanvasPathMethods::arcTo(float x1, float y1, float x2, float y2, float r, E
         m_path.addArcTo(p1, p2, r);
 }
 
-void CanvasPathMethods::arc(float x, float y, float r, float sa, float ea, bool anticlockwise, ExceptionCode& ec)
+static float adjustEndAngle(float startAngle, float endAngle, bool anticlockwise)
+{
+    // If 'startAngle' and 'endAngle' differ by more than 2Pi, just add a circle starting/ending at 'startAngle'.
+    if (anticlockwise && startAngle - endAngle >= 2 * piFloat)
+        return startAngle - 2 * piFloat;
+    if (!anticlockwise && endAngle - startAngle >= 2 * piFloat)
+        return startAngle + 2 * piFloat;
+    return endAngle;
+}
+
+void CanvasPathMethods::arc(float x, float y, float radius, float startAngle, float endAngle, bool anticlockwise, ExceptionCode& ec)
 {
     ec = 0;
-    if (!std::isfinite(x) || !std::isfinite(y) || !std::isfinite(r) || !std::isfinite(sa) || !std::isfinite(ea))
+    if (!std::isfinite(x) || !std::isfinite(y) || !std::isfinite(radius) || !std::isfinite(startAngle) || !std::isfinite(endAngle))
         return;
 
-    if (r < 0) {
+    if (radius < 0) {
         ec = INDEX_SIZE_ERR;
         return;
     }
 
-    if (!r || sa == ea) {
+    if (!radius || startAngle == endAngle) {
         // The arc is empty but we still need to draw the connecting line.
-        lineTo(x + r * cosf(sa), y + r * sinf(sa));
+        lineTo(x + radius * cosf(startAngle), y + radius * sinf(startAngle));
         return;
     }
 
     if (!isTransformInvertible())
         return;
 
-    // If 'sa' and 'ea' differ by more than 2Pi, just add a circle starting/ending at 'sa'.
-    if (anticlockwise && sa - ea >= 2 * piFloat) {
-        m_path.addArc(FloatPoint(x, y), r, sa, sa - 2 * piFloat, anticlockwise);
+    float adjustedEndAngle = adjustEndAngle(startAngle, endAngle, anticlockwise);
+    m_path.addArc(FloatPoint(x, y), radius, startAngle, adjustedEndAngle, anticlockwise);
+}
+
+inline static void lineTo(CanvasPathMethods* path, const FloatPoint& p)
+{
+    path->lineTo(p.x(), p.y());
+}
+
+inline static FloatPoint ellipsePoint(float radiusX, float radiusY, float theta)
+{
+    return FloatPoint(radiusX * cosf(theta), radiusY * sinf(theta));
+}
+
+static void degenerateEllipse(CanvasPathMethods* path, float x, float y, float radiusX, float radiusY, float rotation, float startAngle, float endAngle, bool anticlockwise)
+{
+    FloatPoint center(x, y);
+    AffineTransform rotationMatrix;
+    rotationMatrix.rotate(rad2deg(rotation));
+    lineTo(path, center + rotationMatrix.mapPoint(ellipsePoint(radiusX, radiusY, startAngle)));
+    if ((!radiusX && !radiusY) || startAngle == endAngle)
+        return;
+
+    float sweep = endAngle - startAngle;
+    if (anticlockwise)
+        sweep = startAngle - endAngle;
+
+    if (sweep < 0)
+        sweep = std::fmod(-sweep,  piFloat * 2);

[alph, 2013/07/09 17:18:18]

I'm not sure how to map negative sweep. Unfortunately the specification is not
clear on that. We should probably mimic the behavior of m_path.addEllipse
function.

[dshwang, 2013/07/09 17:40:53]

The spec said
"
If the anticlockwise argument is false and endAngle-startAngle is equal to or
greater than 2π, or, if the anticlockwise argument is true and
startAngle-endAngle is equal to or greater than 2π, then the arc is the whole
circumference of this ellipse.

Otherwise, the arc is the path along the circumference of this ellipse from the
start point to the end point, going anti-clockwise if the anticlockwise argument
is true, and clockwise otherwise. Since the points are on the ellipse, as
opposed to being simply angles from zero, the arc can never cover an angle
greater than 2π radians.
"

adjustEndAngle() can handle sweep angle in the manner which the spec want. I'll
reuse adjustEndAngle().

+
+    FloatPoint endPoint(center + rotationMatrix.mapPoint(ellipsePoint(radiusX, radiusY, endAngle)));
+    if (sweep <= piFloat * 0.5) {
+        lineTo(path, endPoint);

[alph, 2013/07/09 17:18:18]

It now looks much better. But unfortunately this stuff just won't work. See my
counter example to the previous patch.
How about this approach:
1. canonicalize startAngle and endAngle, so that
  0 <= startAngle < 2*pi
  0 <= endAngle - startAngle <= 2*pi

2. then run something like:
lineTo(<point-for-startAngle>);
for (angle = startAngle - fmod(startAngle, pi/2) + pi/2; angle < endAngle; angle
+= pi/2)
  lineTo(<point-for-angle>);
lineTo(<point-for-endAngle>);

[dshwang, 2013/07/09 17:40:53]

Good idea! I buy it. I'll change.

On the other hands, when it comes to your previous counter example.
This implementation is right :)

quote your example.
Good example, but the premise is inconsistent with the result.
you may want endAngle to be 0, not pi/4.

The amended example,
rotation = x = y = 0;
startAngle = -pi/4;
endAngle = 0;
radiusX = 100;
radiusY = 0;

The result is the line from (sqrt(2)/2*100; -sqrt(2)/2*100) to (100; 0).

This implementation will draw the line
from lineTo(path, center + rotationMatrix.mapPoint(ellipsePoint(radiusX,
radiusY, startAngle)));
to   lineTo(path, endPoint);
The result of the implementation equals to what we expect.

[alph, 2013/07/09 18:16:11]

Just though that it's probably better to have two loops. One for clockwise and
another for anticlockwise case. Otherwise if you exchange the start and end
points to make end > start, you'll have to make two additional moveTo's to real
start point in the beginning and to real end point at the end to not break the
path ordering.
I meant the line should go through points:
start is (sqrt(2)/2*100; 0) as it corresponds to -pi/4
then (100; 0) as it corresponds to angle=0 which is inside [-pi/4; pi/4]
and end is just the same as start at (sqrt(2)/2*100; 0) corresponding to pi/4

So visually it will be a line from (sqrt(2)/2*100; 0) to (100; 0)
But your original code won't go through (100; 0). That's what I meant.
Hmm, I don't get it. y of the first point cannot be non zero as long as radiusY
is 0. I.e. all the arc points have y=0.

+    } else if (sweep <= piFloat) {
+        lineTo(path, center + rotationMatrix.mapPoint(ellipsePoint(radiusX, radiusY, piFloat * 0.5)));
+        lineTo(path, endPoint);
+    } else if (sweep <= piFloat * 1.5) {
+        lineTo(path, center + rotationMatrix.mapPoint(ellipsePoint(radiusX, radiusY, piFloat * 0.5)));
+        lineTo(path, center + rotationMatrix.mapPoint(ellipsePoint(radiusX, radiusY, piFloat)));
+        lineTo(path, endPoint);
+    } else if (sweep <= piFloat * 2) {
+        lineTo(path, center + rotationMatrix.mapPoint(ellipsePoint(radiusX, radiusY, piFloat * 0.5)));
+        lineTo(path, center + rotationMatrix.mapPoint(ellipsePoint(radiusX, radiusY, piFloat)));
+        lineTo(path, center + rotationMatrix.mapPoint(ellipsePoint(radiusX, radiusY, piFloat * 1.5)));
+        lineTo(path, endPoint);
+    } else { //  sweep > piFloat * 2
+        lineTo(path, center + rotationMatrix.mapPoint(ellipsePoint(radiusX, radiusY, piFloat * 0.5)));
+        lineTo(path, center + rotationMatrix.mapPoint(ellipsePoint(radiusX, radiusY, piFloat)));
+        lineTo(path, center + rotationMatrix.mapPoint(ellipsePoint(radiusX, radiusY, piFloat * 1.5)));
+        lineTo(path, center + rotationMatrix.mapPoint(ellipsePoint(radiusX, radiusY, piFloat * 2)));
+        lineTo(path, endPoint);
+    }
+}
+
+void CanvasPathMethods::ellipse(float x, float y, float radiusX, float radiusY, float rotation, float startAngle, float endAngle, bool anticlockwise, ExceptionCode& ec)
+{
+    ec = 0;
+    if (!std::isfinite(x) || !std::isfinite(y) || !std::isfinite(radiusX) || !std::isfinite(radiusY) || !std::isfinite(rotation) || !std::isfinite(startAngle) || !std::isfinite(endAngle))
+        return;
+
+    if (radiusX < 0 || radiusY < 0) {
+        ec = INDEX_SIZE_ERR;
         return;
     }
-    if (!anticlockwise && ea - sa >= 2 * piFloat) {
-        m_path.addArc(FloatPoint(x, y), r, sa, sa + 2 * piFloat, anticlockwise);
+
+    if (!radiusX || !radiusY || startAngle == endAngle) {
+        // The ellipse is empty but we still need to draw the connecting line to start point.
+        degenerateEllipse(this, x, y, radiusX, radiusY, rotation, startAngle, endAngle, anticlockwise);
+        // we can do as follows instead of using degenerateEllipse
+        /*
+        if ((!radiusX && !radiusY) || startAngle == endAngle) {
+            AffineTransform rotationMatrix;
+            rotationMatrix.rotate(rad2deg(rotation));
+            lineTo(path, FloatPoint(x, y) + rotationMatrix.mapPoint(ellipsePoint(radiusX, radiusY, startAngle)));
+        } else {
+            ASSERT(startAngle != endAngle);
+            if (!radiusX)
+                radiusX = 0.0001;
+            if (!radiusY)
+                radiusY = 0.0001;
+        }
+        */

[dshwang, 2013/07/09 16:37:22]

Which do you think right: degenerateEllipse() or approximation(radiusX = 0.0001)
?

And do you think degenerateEllipse() still looks scary?

[alph, 2013/07/09 17:18:18]

This way degenerateEllipse looks much better. I'd go for it. ;-)

[dshwang, 2013/07/09 17:40:53]

Good choice :)

         return;
     }
 
-    m_path.addArc(FloatPoint(x, y), r, sa, ea, anticlockwise);
+    if (!isTransformInvertible())
+        return;
+
+    float adjustedEndAngle = adjustEndAngle(startAngle, endAngle, anticlockwise);
+    m_path.addEllipse(FloatPoint(x, y), radiusX, radiusY, rotation, startAngle, adjustedEndAngle, anticlockwise);
 }
 
 void CanvasPathMethods::rect(float x, float y, float width, float height)