Fix getBBox() returning (0,0) bug when width or height is zero

Source/core/rendering/svg/RenderSVGEllipse.cpp

Index: Source/core/rendering/svg/RenderSVGEllipse.cpp
diff --git a/Source/core/rendering/svg/RenderSVGEllipse.cpp b/Source/core/rendering/svg/RenderSVGEllipse.cpp
index bcd290028b12c6c03e958d032323cc9d88f77976..119f8bd7289a6af592ec0c0f47a15327dc9f641a 100644
--- a/Source/core/rendering/svg/RenderSVGEllipse.cpp
+++ b/Source/core/rendering/svg/RenderSVGEllipse.cpp
@@ -53,23 +53,26 @@ void RenderSVGEllipse::updateShapeFromElement()
     m_center = FloatPoint();
     m_radii = FloatSize();
 
+    m_usePathFallback = false;
     // Fallback to RenderSVGShape if shape has a non-scaling stroke.
     if (hasNonScalingStroke()) {
         RenderSVGShape::updateShapeFromElement();
         m_usePathFallback = true;

[pdr., 2014/03/24 17:39:32]

I'm afraid this part isn't right.

Some background: RenderSVGShape is the superclass of the renderers for path,
rect, ellipse, etc. RenderSVGShape knows how to render a path but it's wasteful
in some cases (e.g., a rect) so we have specialized renderers for the simple
cases. When the specialized renderers can't handle some quirk such as non
scaling stroke, they fall back to the generic but slow RenderSVGShape.

With this change we are mixing the fallback and specialized cases.
calculateRadiiAndCenter is for the specialized ellipse code and shouldn't be
needed in the fallback case.



I suspect this explains why so many test results were affected. I don't think we
have that many shapes with width=0 or height=0 in our tests :)

[jmunhoz, 2014/03/24 18:51:24]

On 2014/03/24 17:39:32, pdr wrote:
[...] 
I understand. I will redo the patch. Thanks!
:)

-        return;
-    } else
-        m_usePathFallback = false;
+    }
 
     calculateRadiiAndCenter();
 
-    // Spec: "A value of zero disables rendering of the element."
-    if (m_radii.width() <= 0 || m_radii.height() <= 0)
+    // Spec: "A negative value is an error. A value of zero disables rendering of the element."
+    if (m_radii.isZero() || m_radii.width() < 0 || m_radii.height() < 0)
         return;
 
     m_fillBoundingBox = FloatRect(m_center.x() - m_radii.width(), m_center.y() - m_radii.height(), 2 * m_radii.width(), 2 * m_radii.height());
+
+    if (m_usePathFallback)
+        return;
+
     m_strokeBoundingBox = m_fillBoundingBox;
-    if (style()->svgStyle()->hasStroke())
+    if (style()->svgStyle()->hasStroke() && m_radii.width() && m_radii.height());

[Stephen Chennney, 2014/03/24 15:49:16]

Typo? Does strokeWidth() depend on having non-zero radii? Otherwise, why do you
want the new check at all?

         m_strokeBoundingBox.inflate(strokeWidth() / 2);
 }