Title: [137669] trunk
Revision
137669
Author
[email protected]
Date
2012-12-13 15:34:31 -0800 (Thu, 13 Dec 2012)

Log Message

Switch the gradient drawing code to use bearing angles
https://bugs.webkit.org/show_bug.cgi?id=104616

Patch by Tab Atkins <[email protected]> on 2012-12-13
Reviewed by Dean Jackson.

Source/WebCore:

Switch the gradient math from polar-coordinate angles to bearing angles.
To make this possible without behavior changes,
switch the m_deprecatedType boolean to an m_gradientType enum,
and fix all usages accordingly.

No new tests, as this is a refactoring and should have no behavior changes. Existing tests suffice.

* css/CSSGradientValue.cpp:
(WebCore::endPointsFromAngle):
This is the only mechanical change. The rest are just fixing the plumbing around the boolean->enum change.

* css/CSSGradientValue.cpp:
(WebCore::CSSGradientValue::sortStopsIfNeeded):
(WebCore::CSSGradientValue::addStops):
(WebCore::CSSLinearGradientValue::customCssText):
(WebCore::CSSLinearGradientValue::createGradient):
(WebCore::CSSRadialGradientValue::customCssText):
* css/CSSGradientValue.h:
(WebCore::CSSGradientValue::gradientType):
(WebCore::CSSGradientValue::CSSGradientValue):
(WebCore::CSSLinearGradientValue::create):
(WebCore::CSSLinearGradientValue::CSSLinearGradientValue):
(WebCore::CSSRadialGradientValue::create):
(WebCore::CSSRadialGradientValue::CSSRadialGradientValue):
* css/CSSParser.cpp:
(WebCore::CSSParser::parseDeprecatedGradient):
(WebCore::CSSParser::parseDeprecatedLinearGradient):
(WebCore::CSSParser::parseDeprecatedRadialGradient):

LayoutTests:

This change alters a few pixels on the linux baseline.
It may or may not trigger similar failures on Windows and Mac - we'll see.

* platform/chromium-linux/fast/gradients/css3-linear-angle-gradients-expected.png:
* platform/chromium/TestExpectations:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (137668 => 137669)


--- trunk/LayoutTests/ChangeLog	2012-12-13 23:33:45 UTC (rev 137668)
+++ trunk/LayoutTests/ChangeLog	2012-12-13 23:34:31 UTC (rev 137669)
@@ -1,3 +1,16 @@
+2012-12-13  Tab Atkins  <[email protected]>
+
+        Switch the gradient drawing code to use bearing angles
+        https://bugs.webkit.org/show_bug.cgi?id=104616
+
+        Reviewed by Dean Jackson.
+
+        This change alters a few pixels on the linux baseline.
+        It may or may not trigger similar failures on Windows and Mac - we'll see.
+
+        * platform/chromium-linux/fast/gradients/css3-linear-angle-gradients-expected.png:
+        * platform/chromium/TestExpectations:
+
 2012-12-13  Adam Klein  <[email protected]>
 
         Mark one test as failing and rebaseline another after r137646

Modified: trunk/LayoutTests/platform/chromium/TestExpectations (137668 => 137669)


--- trunk/LayoutTests/platform/chromium/TestExpectations	2012-12-13 23:33:45 UTC (rev 137668)
+++ trunk/LayoutTests/platform/chromium/TestExpectations	2012-12-13 23:34:31 UTC (rev 137669)
@@ -4190,6 +4190,8 @@
 webkit.org/b/104727 transforms/3d/hit-testing/backface-hit-test.html [ Pass ImageOnlyFailure ]
 webkit.org/b/104727 transforms/3d/hit-testing/backface-no-transform-hit-test.html [ Pass ImageOnlyFailure ]
 
+webkit.org/b/104616 [ Win Mac ] fast/gradients/css3-linear-angle-gradients.html [ ImageOnlyFailure Pass ]
+
 # Fail only on WebKit-only checkout
 webkit.org/b/104548 [ Linux ] fast/text/hyphenate-character.html [ Failure ]
 webkit.org/b/104548 [ Linux ] fast/text/hyphenate-first-word.html [ Failure ]

Modified: trunk/LayoutTests/platform/chromium-linux/fast/gradients/css3-linear-angle-gradients-expected.png


(Binary files differ)

Modified: trunk/Source/WebCore/ChangeLog (137668 => 137669)


--- trunk/Source/WebCore/ChangeLog	2012-12-13 23:33:45 UTC (rev 137668)
+++ trunk/Source/WebCore/ChangeLog	2012-12-13 23:34:31 UTC (rev 137669)
@@ -1,3 +1,39 @@
+2012-12-13  Tab Atkins  <[email protected]>
+
+        Switch the gradient drawing code to use bearing angles
+        https://bugs.webkit.org/show_bug.cgi?id=104616
+
+        Reviewed by Dean Jackson.
+
+        Switch the gradient math from polar-coordinate angles to bearing angles.
+        To make this possible without behavior changes, 
+        switch the m_deprecatedType boolean to an m_gradientType enum,
+        and fix all usages accordingly.
+
+        No new tests, as this is a refactoring and should have no behavior changes. Existing tests suffice.
+
+        * css/CSSGradientValue.cpp:
+        (WebCore::endPointsFromAngle):
+        This is the only mechanical change. The rest are just fixing the plumbing around the boolean->enum change.
+
+        * css/CSSGradientValue.cpp:
+        (WebCore::CSSGradientValue::sortStopsIfNeeded):
+        (WebCore::CSSGradientValue::addStops):
+        (WebCore::CSSLinearGradientValue::customCssText):
+        (WebCore::CSSLinearGradientValue::createGradient):
+        (WebCore::CSSRadialGradientValue::customCssText):
+        * css/CSSGradientValue.h:
+        (WebCore::CSSGradientValue::gradientType):
+        (WebCore::CSSGradientValue::CSSGradientValue):
+        (WebCore::CSSLinearGradientValue::create):
+        (WebCore::CSSLinearGradientValue::CSSLinearGradientValue):
+        (WebCore::CSSRadialGradientValue::create):
+        (WebCore::CSSRadialGradientValue::CSSRadialGradientValue):
+        * css/CSSParser.cpp:
+        (WebCore::CSSParser::parseDeprecatedGradient):
+        (WebCore::CSSParser::parseDeprecatedLinearGradient):
+        (WebCore::CSSParser::parseDeprecatedRadialGradient):
+
 2012-12-13  Ian Vollick  <[email protected]>
 
         [chromium] Automatically use composited scrolling

Modified: trunk/Source/WebCore/css/CSSGradientValue.cpp (137668 => 137669)


--- trunk/Source/WebCore/css/CSSGradientValue.cpp	2012-12-13 23:33:45 UTC (rev 137668)
+++ trunk/Source/WebCore/css/CSSGradientValue.cpp	2012-12-13 23:34:31 UTC (rev 137669)
@@ -96,7 +96,7 @@
 
 void CSSGradientValue::sortStopsIfNeeded()
 {
-    ASSERT(m_deprecatedType);
+    ASSERT(m_gradientType == CSSDeprecatedLinearGradient || m_gradientType == CSSDeprecatedRadialGradient);
     if (!m_stopsSorted) {
         if (m_stops.size())
             std::stable_sort(m_stops.begin(), m_stops.end(), compareStops);
@@ -147,7 +147,7 @@
 {
     RenderStyle* style = renderer->style();
 
-    if (m_deprecatedType) {
+    if (m_gradientType == CSSDeprecatedLinearGradient || m_gradientType == CSSDeprecatedRadialGradient) {
         sortStopsIfNeeded();
 
         for (unsigned i = 0; i < m_stops.size(); i++) {
@@ -485,7 +485,7 @@
 String CSSLinearGradientValue::customCssText() const
 {
     StringBuilder result;
-    if (m_deprecatedType) {
+    if (m_gradientType == CSSDeprecatedLinearGradient) {
         result.appendLiteral("-webkit-gradient(linear, ");
         result.append(m_firstX->cssText());
         result.append(' ');
@@ -552,64 +552,71 @@
 }
 
 // Compute the endpoints so that a gradient of the given angle covers a box of the given size.
-static void endPointsFromAngle(float angleDeg, const IntSize& size, FloatPoint& firstPoint, FloatPoint& secondPoint)
+static void endPointsFromAngle(float angleDeg, const IntSize& size, FloatPoint& firstPoint, FloatPoint& secondPoint, CSSGradientType type)
 {
+    // Prefixed gradients use "polar coordinate" angles, rather than "bearing" angles.
+    if (type == CSSPrefixedLinearGradient)
+        angleDeg = 90 - angleDeg;
+
     angleDeg = fmodf(angleDeg, 360);
     if (angleDeg < 0)
         angleDeg += 360;
 
     if (!angleDeg) {
-        firstPoint.set(0, 0);
-        secondPoint.set(size.width(), 0);
+        firstPoint.set(0, size.height());
+        secondPoint.set(0, 0);
         return;
     }
 
     if (angleDeg == 90) {
-        firstPoint.set(0, size.height());
-        secondPoint.set(0, 0);
+        firstPoint.set(0, 0);
+        secondPoint.set(size.width(), 0);
         return;
     }
 
     if (angleDeg == 180) {
-        firstPoint.set(size.width(), 0);
-        secondPoint.set(0, 0);
+        firstPoint.set(0, 0);
+        secondPoint.set(0, size.height());
         return;
     }
 
     if (angleDeg == 270) {
-        firstPoint.set(0, 0);
-        secondPoint.set(0, size.height());
+        firstPoint.set(size.width(), 0);
+        secondPoint.set(0, 0);
         return;
     }
 
-    float slope = tan(deg2rad(angleDeg));
+    // angleDeg is a "bearing angle" (0deg = N, 90deg = E),
+    // but tan expects 0deg = E, 90deg = N.
+    float slope = tan(deg2rad(90.0 - angleDeg));
 
     // We find the endpoint by computing the intersection of the line formed by the slope,
     // and a line perpendicular to it that intersects the corner.
     float perpendicularSlope = -1 / slope;
 
-    // Compute start corner relative to center.
+    // Compute start corner relative to center, in Cartesian space (+y = up).
     float halfHeight = size.height() / 2;
     float halfWidth = size.width() / 2;
     FloatPoint endCorner;
     if (angleDeg < 90)
         endCorner.set(halfWidth, halfHeight);
     else if (angleDeg < 180)
-        endCorner.set(-halfWidth, halfHeight);
+        endCorner.set(halfWidth, -halfHeight);
     else if (angleDeg < 270)
         endCorner.set(-halfWidth, -halfHeight);
     else
-        endCorner.set(halfWidth, -halfHeight);
+        endCorner.set(-halfWidth, halfHeight);
 
     // Compute c (of y = mx + c) using the corner point.
     float c = endCorner.y() - perpendicularSlope * endCorner.x();
     float endX = c / (slope - perpendicularSlope);
     float endY = perpendicularSlope * endX + c;
 
-    // We computed the end point, so set the second point, flipping the Y to account for angles going anticlockwise.
-    secondPoint.set(halfWidth + endX, size.height() - (halfHeight + endY));
+    // We computed the end point, so set the second point, 
+    // taking into account the moved origin and the fact that we're in drawing space (+y = down).
+    secondPoint.set(halfWidth + endX, halfHeight - endY);
     // Reflect around the center for the start point.
-    firstPoint.set(size.width() - secondPoint.x(), size.height() - secondPoint.y());
+    firstPoint.set(halfWidth - endX, halfHeight + endY);
 }
 
 PassRefPtr<Gradient> CSSLinearGradientValue::createGradient(RenderObject* renderer, const IntSize& size)
@@ -622,7 +629,7 @@
     FloatPoint secondPoint;
     if (m_angle) {
         float angle = m_angle->getFloatValue(CSSPrimitiveValue::CSS_DEG);
-        endPointsFromAngle(angle, size, firstPoint, secondPoint);
+        endPointsFromAngle(angle, size, firstPoint, secondPoint, m_gradientType);
     } else {
         firstPoint = computeEndPoint(m_firstX.get(), m_firstY.get(), renderer->style(), rootStyle, size);
 
@@ -655,7 +662,7 @@
 {
     StringBuilder result;
 
-    if (m_deprecatedType) {
+    if (m_gradientType == CSSDeprecatedRadialGradient) {
         result.appendLiteral("-webkit-gradient(radial, ");
         result.append(m_firstX->cssText());
         result.append(' ');

Modified: trunk/Source/WebCore/css/CSSGradientValue.h (137668 => 137669)


--- trunk/Source/WebCore/css/CSSGradientValue.h	2012-12-13 23:33:45 UTC (rev 137668)
+++ trunk/Source/WebCore/css/CSSGradientValue.h	2012-12-13 23:34:31 UTC (rev 137669)
@@ -36,7 +36,14 @@
 class FloatPoint;
 class Gradient;
 
-enum CSSGradientType { CSSLinearGradient, CSSRadialGradient };
+enum CSSGradientType {
+    CSSDeprecatedLinearGradient,
+    CSSDeprecatedRadialGradient,
+    CSSPrefixedLinearGradient,
+    CSSPrefixedRadialGradient,
+    CSSLinearGradient,
+    CSSRadialGradient
+};
 enum CSSGradientRepeat { NonRepeating, Repeating };
 
 struct CSSGradientColorStop {
@@ -68,7 +75,7 @@
 
     bool isRepeating() const { return m_repeating; }
 
-    bool deprecatedType() const { return m_deprecatedType; } // came from -webkit-gradient
+    CSSGradientType gradientType() const { return m_gradientType; }
 
     bool isFixedSize() const { return false; }
     IntSize fixedSize(const RenderObject*) const { return IntSize(); }
@@ -80,15 +87,15 @@
     PassRefPtr<CSSGradientValue> gradientWithStylesResolved(StyleResolver*);
 
 protected:
-    CSSGradientValue(ClassType classType, CSSGradientRepeat repeat, bool deprecatedType = false)
+    CSSGradientValue(ClassType classType, CSSGradientRepeat repeat, CSSGradientType gradientType)
         : CSSImageGeneratorValue(classType)
         , m_stopsSorted(false)
-        , m_deprecatedType(deprecatedType)
+        , m_gradientType(gradientType)
         , m_repeating(repeat == Repeating)
     {
     }
 
-    CSSGradientValue(const CSSGradientValue& other, ClassType classType, bool deprecatedType = false)
+    CSSGradientValue(const CSSGradientValue& other, ClassType classType, CSSGradientType gradientType)
         : CSSImageGeneratorValue(classType)
         , m_firstX(other.m_firstX)
         , m_firstY(other.m_firstY)
@@ -96,7 +103,7 @@
         , m_secondY(other.m_secondY)
         , m_stops(other.m_stops)
         , m_stopsSorted(other.m_stopsSorted)
-        , m_deprecatedType(deprecatedType)
+        , m_gradientType(gradientType)
         , m_repeating(other.isRepeating() ? Repeating : NonRepeating)
     {
     }
@@ -120,16 +127,17 @@
     // Stops
     Vector<CSSGradientColorStop, 2> m_stops;
     bool m_stopsSorted;
-    bool m_deprecatedType; // -webkit-gradient()
+    CSSGradientType m_gradientType;
     bool m_repeating;
 };
 
 
 class CSSLinearGradientValue : public CSSGradientValue {
 public:
-    static PassRefPtr<CSSLinearGradientValue> create(CSSGradientRepeat repeat, bool deprecatedType = false)
+
+    static PassRefPtr<CSSLinearGradientValue> create(CSSGradientRepeat repeat, CSSGradientType gradientType = CSSLinearGradient)
     {
-        return adoptRef(new CSSLinearGradientValue(repeat, deprecatedType));
+        return adoptRef(new CSSLinearGradientValue(repeat, gradientType));
     }
 
     void setAngle(PassRefPtr<CSSPrimitiveValue> val) { m_angle = val; }
@@ -147,13 +155,13 @@
     void reportDescendantMemoryUsage(MemoryObjectInfo*) const;
 
 private:
-    CSSLinearGradientValue(CSSGradientRepeat repeat, bool deprecatedType = false)
-        : CSSGradientValue(LinearGradientClass, repeat, deprecatedType)
+    CSSLinearGradientValue(CSSGradientRepeat repeat, CSSGradientType gradientType = CSSLinearGradient)
+        : CSSGradientValue(LinearGradientClass, repeat, gradientType)
     {
     }
 
     CSSLinearGradientValue(const CSSLinearGradientValue& other)
-        : CSSGradientValue(other, LinearGradientClass, other.deprecatedType())
+        : CSSGradientValue(other, LinearGradientClass, other.gradientType())
         , m_angle(other.m_angle)
     {
     }
@@ -163,9 +171,9 @@
 
 class CSSRadialGradientValue : public CSSGradientValue {
 public:
-    static PassRefPtr<CSSRadialGradientValue> create(CSSGradientRepeat repeat, bool deprecatedType = false)
+    static PassRefPtr<CSSRadialGradientValue> create(CSSGradientRepeat repeat, CSSGradientType gradientType = CSSRadialGradient)
     {
-        return adoptRef(new CSSRadialGradientValue(repeat, deprecatedType));
+        return adoptRef(new CSSRadialGradientValue(repeat, gradientType));
     }
 
     PassRefPtr<CSSRadialGradientValue> clone() const
@@ -190,13 +198,13 @@
     void reportDescendantMemoryUsage(MemoryObjectInfo*) const;
 
 private:
-    CSSRadialGradientValue(CSSGradientRepeat repeat, bool deprecatedType = false)
-        : CSSGradientValue(RadialGradientClass, repeat, deprecatedType)
+    CSSRadialGradientValue(CSSGradientRepeat repeat, CSSGradientType gradientType = CSSRadialGradient)
+        : CSSGradientValue(RadialGradientClass, repeat, gradientType)
     {
     }
 
     CSSRadialGradientValue(const CSSRadialGradientValue& other)
-        : CSSGradientValue(other, RadialGradientClass, other.deprecatedType())
+        : CSSGradientValue(other, RadialGradientClass, other.gradientType())
         , m_firstRadius(other.m_firstRadius)
         , m_secondRadius(other.m_secondRadius)
         , m_shape(other.m_shape)

Modified: trunk/Source/WebCore/css/CSSParser.cpp (137668 => 137669)


--- trunk/Source/WebCore/css/CSSParser.cpp	2012-12-13 23:33:45 UTC (rev 137668)
+++ trunk/Source/WebCore/css/CSSParser.cpp	2012-12-13 23:34:31 UTC (rev 137669)
@@ -6985,20 +6985,23 @@
     if (!a || a->unit != CSSPrimitiveValue::CSS_IDENT)
         return false;
     if (equalIgnoringCase(a->string, "linear"))
-        gradientType = CSSLinearGradient;
+        gradientType = CSSDeprecatedLinearGradient;
     else if (equalIgnoringCase(a->string, "radial"))
-        gradientType = CSSRadialGradient;
+        gradientType = CSSDeprecatedRadialGradient;
     else
         return false;
 
     RefPtr<CSSGradientValue> result;
     switch (gradientType) {
-        case CSSLinearGradient:
-            result = CSSLinearGradientValue::create(NonRepeating, true);
-            break;
-        case CSSRadialGradient:
-            result = CSSRadialGradientValue::create(NonRepeating, true);
-            break;
+    case CSSDeprecatedLinearGradient:
+        result = CSSLinearGradientValue::create(NonRepeating, gradientType);
+        break;
+    case CSSDeprecatedRadialGradient:
+        result = CSSRadialGradientValue::create(NonRepeating, gradientType);
+        break;
+    default:
+        // The rest of the gradient types shouldn't appear here.
+        ASSERT_NOT_REACHED();
     }
 
     // Comma.
@@ -7032,7 +7035,7 @@
         return false;
 
     // For radial gradients only, we now expect a numeric radius.
-    if (gradientType == CSSRadialGradient) {
+    if (gradientType == CSSDeprecatedRadialGradient) {
         a = args->next();
         if (!a || a->unit != CSSPrimitiveValue::CSS_NUMBER)
             return false;
@@ -7064,7 +7067,7 @@
     result->setSecondY(point.release());
 
     // For radial gradients only, we now expect the second radius.
-    if (gradientType == CSSRadialGradient) {
+    if (gradientType == CSSDeprecatedRadialGradient) {
         // Comma after the second point.
         a = args->next();
         if (!isComma(a))
@@ -7133,7 +7136,7 @@
 
 bool CSSParser::parseDeprecatedLinearGradient(CSSParserValueList* valueList, RefPtr<CSSValue>& gradient, CSSGradientRepeat repeating)
 {
-    RefPtr<CSSLinearGradientValue> result = CSSLinearGradientValue::create(repeating);
+    RefPtr<CSSLinearGradientValue> result = CSSLinearGradientValue::create(repeating, CSSPrefixedLinearGradient);
 
     // Walk the arguments.
     CSSParserValueList* args = valueList->current()->function->args.get();
@@ -7201,7 +7204,7 @@
 
 bool CSSParser::parseDeprecatedRadialGradient(CSSParserValueList* valueList, RefPtr<CSSValue>& gradient, CSSGradientRepeat repeating)
 {
-    RefPtr<CSSRadialGradientValue> result = CSSRadialGradientValue::create(repeating);
+    RefPtr<CSSRadialGradientValue> result = CSSRadialGradientValue::create(repeating, CSSPrefixedRadialGradient);
 
     // Walk the arguments.
     CSSParserValueList* args = valueList->current()->function->args.get();
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to