[CSS Shapes] Basic shapes' computed position should be a horizontal and vertical offset

Source/core/css/BasicShapeFunctions.cpp

Index: Source/core/css/BasicShapeFunctions.cpp
diff --git a/Source/core/css/BasicShapeFunctions.cpp b/Source/core/css/BasicShapeFunctions.cpp
index b2e5f4fdb46727b78246620d7016d6f96b5677cb..8ec7f86d6782a492a11fe6afedf5cd1541f9dddc 100644
--- a/Source/core/css/BasicShapeFunctions.cpp
+++ b/Source/core/css/BasicShapeFunctions.cpp
@@ -40,25 +40,12 @@
 
 namespace WebCore {
 
-static PassRefPtr<CSSPrimitiveValue> valueForCenterCoordinate(CSSValuePool& pool, const RenderStyle& style, const BasicShapeCenterCoordinate& center)
+static PassRefPtr<CSSPrimitiveValue> valueForCenterCoordinate(CSSValuePool& pool, const RenderStyle& style, const BasicShapeCenterCoordinate& center, EBoxOrient orientation)
 {
-    CSSValueID keyword = CSSValueInvalid;
-    switch (center.keyword()) {
-    case BasicShapeCenterCoordinate::None:
+    if (center.direction() == BasicShapeCenterCoordinate::TopLeft)
         return pool.createValue(center.length(), style);
-    case BasicShapeCenterCoordinate::Top:
-        keyword = CSSValueTop;
-        break;
-    case BasicShapeCenterCoordinate::Right:
-        keyword = CSSValueRight;
-        break;
-    case BasicShapeCenterCoordinate::Bottom:
-        keyword = CSSValueBottom;
-        break;
-    case BasicShapeCenterCoordinate::Left:
-        keyword = CSSValueLeft;
-        break;
-    }
+
+    CSSValueID keyword = orientation == HORIZONTAL ? CSSValueRight : CSSValueBottom;
 
     return pool.createValue(Pair::create(pool.createIdentifierValue(keyword), pool.createValue(center.length(), style), Pair::DropIdenticalValues));
 }
@@ -113,8 +100,8 @@ PassRefPtr<CSSValue> valueForBasicShape(const RenderStyle& style, const BasicSha
         const BasicShapeCircle* circle = static_cast<const BasicShapeCircle*>(basicShape);
         RefPtr<CSSBasicShapeCircle> circleValue = CSSBasicShapeCircle::create();
 
-        circleValue->setCenterX(valueForCenterCoordinate(pool, style, circle->centerX()));
-        circleValue->setCenterY(valueForCenterCoordinate(pool, style, circle->centerY()));
+        circleValue->setCenterX(valueForCenterCoordinate(pool, style, circle->centerX(), HORIZONTAL));
+        circleValue->setCenterY(valueForCenterCoordinate(pool, style, circle->centerY(), VERTICAL));
         circleValue->setRadius(basicShapeRadiusToCSSValue(pool, style, circle->radius()));
         basicShapeValue = circleValue.release();
         break;
@@ -135,8 +122,8 @@ PassRefPtr<CSSValue> valueForBasicShape(const RenderStyle& style, const BasicSha
         const BasicShapeEllipse* ellipse = static_cast<const BasicShapeEllipse*>(basicShape);
         RefPtr<CSSBasicShapeEllipse> ellipseValue = CSSBasicShapeEllipse::create();
 
-        ellipseValue->setCenterX(valueForCenterCoordinate(pool, style, ellipse->centerX()));
-        ellipseValue->setCenterY(valueForCenterCoordinate(pool, style, ellipse->centerY()));
+        ellipseValue->setCenterX(valueForCenterCoordinate(pool, style, ellipse->centerX(), HORIZONTAL));
+        ellipseValue->setCenterY(valueForCenterCoordinate(pool, style, ellipse->centerY(), VERTICAL));
         ellipseValue->setRadiusX(basicShapeRadiusToCSSValue(pool, style, ellipse->radiusX()));
         ellipseValue->setRadiusY(basicShapeRadiusToCSSValue(pool, style, ellipse->radiusY()));
         basicShapeValue = ellipseValue.release();
@@ -185,29 +172,41 @@ static Length convertToLength(const StyleResolverState& state, CSSPrimitiveValue
 
 static BasicShapeCenterCoordinate convertToCenterCoordinate(const StyleResolverState& state, CSSPrimitiveValue* value)
 {
-    if (Pair* pair = value->getPairValue()) {
-        BasicShapeCenterCoordinate::Keyword keyword = BasicShapeCenterCoordinate::None;
-        switch (pair->first()->getValueID()) {
-        case CSSValueTop:
-            keyword = BasicShapeCenterCoordinate::Top;
-            break;
-        case CSSValueRight:
-            keyword = BasicShapeCenterCoordinate::Right;
-            break;
-        case CSSValueBottom:
-            keyword = BasicShapeCenterCoordinate::Bottom;
-            break;
-        case CSSValueLeft:
-            keyword = BasicShapeCenterCoordinate::Left;
-            break;
-        default:
-            ASSERT_NOT_REACHED();
-            break;
-        }
-        return BasicShapeCenterCoordinate(keyword, convertToLength(state, pair->second()));
+    BasicShapeCenterCoordinate::Direction direction;
+    Length offset = Length(0, Fixed);
+
+    CSSValueID keyword = CSSValueTop;
+    if (!value) {
+        keyword = CSSValueCenter;
+    } else if (value->isValueID()) {
+        keyword = value->getValueID();
+    } else if (Pair* pair = value->getPairValue()) {
+        keyword = pair->first()->getValueID();
+        offset = convertToLength(state, pair->second());
+    } else {
+        offset = convertToLength(state, value);
+    }
+
+    switch (keyword) {
+    case CSSValueTop:
+    case CSSValueLeft:
+        direction = BasicShapeCenterCoordinate::TopLeft;
+        break;
+    case CSSValueRight:
+    case CSSValueBottom:
+        direction = BasicShapeCenterCoordinate::BottomRight;
+        break;
+    case CSSValueCenter:
+        direction = BasicShapeCenterCoordinate::TopLeft;
+        offset = Length(50, Percent);
+        break;
+    default:
+        ASSERT_NOT_REACHED();
+        direction = BasicShapeCenterCoordinate::TopLeft;
+        break;
     }
 
-    return BasicShapeCenterCoordinate(convertToLength(state, value));
+    return BasicShapeCenterCoordinate(direction, offset);
 }
 
 static BasicShapeRadius cssValueToBasicShapeRadius(const StyleResolverState& state, PassRefPtr<CSSPrimitiveValue> radius)
@@ -272,13 +271,8 @@ PassRefPtr<BasicShape> basicShapeForValue(const StyleResolverState& state, const
         const CSSBasicShapeCircle* circleValue = static_cast<const CSSBasicShapeCircle *>(basicShapeValue);
         RefPtr<BasicShapeCircle> circle = BasicShapeCircle::create();
 
-        if (circleValue->centerX() && circleValue->centerY()) {
-            circle->setCenterX(convertToCenterCoordinate(state, circleValue->centerX()));
-            circle->setCenterY(convertToCenterCoordinate(state, circleValue->centerY()));
-        } else {
-            circle->setCenterX(BasicShapeCenterCoordinate(Length(50, Percent)));
-            circle->setCenterY(BasicShapeCenterCoordinate(Length(50, Percent)));
-        }
+        circle->setCenterX(convertToCenterCoordinate(state, circleValue->centerX()));
+        circle->setCenterY(convertToCenterCoordinate(state, circleValue->centerY()));
         circle->setRadius(cssValueToBasicShapeRadius(state, circleValue->radius()));
 
         basicShape = circle.release();
@@ -300,13 +294,8 @@ PassRefPtr<BasicShape> basicShapeForValue(const StyleResolverState& state, const
         const CSSBasicShapeEllipse* ellipseValue = static_cast<const CSSBasicShapeEllipse *>(basicShapeValue);
         RefPtr<BasicShapeEllipse> ellipse = BasicShapeEllipse::create();
 
-        if (ellipseValue->centerX() && ellipseValue->centerY()) {
-            ellipse->setCenterX(convertToCenterCoordinate(state, ellipseValue->centerX()));
-            ellipse->setCenterY(convertToCenterCoordinate(state, ellipseValue->centerY()));
-        } else {
-            ellipse->setCenterX(BasicShapeCenterCoordinate(Length(50, Percent)));
-            ellipse->setCenterY(BasicShapeCenterCoordinate(Length(50, Percent)));
-        }
+        ellipse->setCenterX(convertToCenterCoordinate(state, ellipseValue->centerX()));
+        ellipse->setCenterY(convertToCenterCoordinate(state, ellipseValue->centerY()));
         ellipse->setRadiusX(cssValueToBasicShapeRadius(state, ellipseValue->radiusX()));
         ellipse->setRadiusY(cssValueToBasicShapeRadius(state, ellipseValue->radiusY()));
 
@@ -361,30 +350,16 @@ FloatPoint floatPointForCenterCoordinate(const BasicShapeCenterCoordinate& cente
 {
     FloatPoint p;
     float offset = floatValueForLength(centerX.length(), boxSize.width());
-    switch (centerX.keyword()) {
-    case BasicShapeCenterCoordinate::None:
-    case BasicShapeCenterCoordinate::Left:
+    if (centerX.direction() == BasicShapeCenterCoordinate::TopLeft)

[Bear Travis, 2014/01/23 22:04:42]

As a nit, this method feels like it's a little too long and repetitive for what
it's doing.
Perhaps using a ternary operator or splitting out the common code into an inline
static function?

         p.setX(offset);
-        break;
-    case BasicShapeCenterCoordinate::Right:
+    else
         p.setX(boxSize.width() - offset);
-        break;
-    default:
-        ASSERT_NOT_REACHED();
-    }
 
     offset = floatValueForLength(centerY.length(), boxSize.height());
-    switch (centerY.keyword()) {
-    case BasicShapeCenterCoordinate::None:
-    case BasicShapeCenterCoordinate::Top:
+    if (centerY.direction() == BasicShapeCenterCoordinate::TopLeft)
         p.setY(offset);
-        break;
-    case BasicShapeCenterCoordinate::Bottom:
+    else
         p.setY(boxSize.height() - offset);
-        break;
-    default:
-        ASSERT_NOT_REACHED();
-    }
 
     return p;
 }