Title: [282443] trunk
Revision
282443
Author
[email protected]
Date
2021-09-15 00:26:18 -0700 (Wed, 15 Sep 2021)

Log Message

Linear gradient sometimes is drawn incorrectly and sometimes hangs
https://bugs.webkit.org/show_bug.cgi?id=230145
<rdar://82428383>

Reviewed by Simon Fraser.

Source/WebCore:

Gradient::paint() has a few flaws:

1) We have to use atan2() instead of acos() to get the angle of the vector
   (p0, p1) in the correct quadrant. acos() returns an angle between [0, pi].

2) The order in the case of 'repeat' and 'reflect' should be the following:
    -- Move the points forward towards the bounding box.
    -- Draw gradient and move the points forward till they are not
       intersecting the bounding box.
    -- Move the points backward towards the bounding box.
    -- Draw gradient and move the points backward till they are not
       intersecting the bounding box.

Tests: svg/gradients/gradient-flipped-start-end-points-expected.svg
       svg/gradients/gradient-flipped-start-end-points.svg

* platform/graphics/cg/GradientCG.cpp:
(WebCore::Gradient::paint):

LayoutTests:

* svg/gradients/gradient-flipped-start-end-points-expected.svg: Added.
* svg/gradients/gradient-flipped-start-end-points.svg: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (282442 => 282443)


--- trunk/LayoutTests/ChangeLog	2021-09-15 07:19:07 UTC (rev 282442)
+++ trunk/LayoutTests/ChangeLog	2021-09-15 07:26:18 UTC (rev 282443)
@@ -1,3 +1,14 @@
+2021-09-15  Said Abou-Hallawa  <[email protected]>
+
+        Linear gradient sometimes is drawn incorrectly and sometimes hangs
+        https://bugs.webkit.org/show_bug.cgi?id=230145
+        <rdar://82428383>
+
+        Reviewed by Simon Fraser.
+
+        * svg/gradients/gradient-flipped-start-end-points-expected.svg: Added.
+        * svg/gradients/gradient-flipped-start-end-points.svg: Added.
+
 2021-09-15  Myles C. Maxfield  <[email protected]>
 
         CSSFontFaceSrcValue.cssText should be quoted consistently with other browsers

Added: trunk/LayoutTests/svg/gradients/gradient-flipped-start-end-points-expected.svg (0 => 282443)


--- trunk/LayoutTests/svg/gradients/gradient-flipped-start-end-points-expected.svg	                        (rev 0)
+++ trunk/LayoutTests/svg/gradients/gradient-flipped-start-end-points-expected.svg	2021-09-15 07:26:18 UTC (rev 282443)
@@ -0,0 +1,18 @@
+<svg xmlns="http://www.w3.org/2000/svg">
+    <defs>
+        <linearGradient id="repeat-gradient" x1="0" y1="0" x2="0.5" y2="0" spreadMethod="repeat">
+            <stop offset="0%" stop-color="green"/>
+            <stop offset="50%" stop-color="green"/>
+            <stop offset="50%" stop-color="blue"/>
+            <stop offset="100%" stop-color="blue"/>
+        </linearGradient>
+        <linearGradient id="reflect-gradient" x1="0" y1="0" x2="0.5" y2="0" spreadMethod="reflect">
+            <stop offset="0%" stop-color="blue"/>
+            <stop offset="50%" stop-color="blue"/>
+            <stop offset="50%" stop-color="green"/>
+            <stop offset="100%" stop-color="green"/>
+        </linearGradient>
+    </defs>
+    <rect fill="url(#repeat-gradient)" width="100" height="100"/>
+    <rect fill="url(#reflect-gradient)" x="110" width="100" height="100"/>
+</svg>

Added: trunk/LayoutTests/svg/gradients/gradient-flipped-start-end-points.svg (0 => 282443)


--- trunk/LayoutTests/svg/gradients/gradient-flipped-start-end-points.svg	                        (rev 0)
+++ trunk/LayoutTests/svg/gradients/gradient-flipped-start-end-points.svg	2021-09-15 07:26:18 UTC (rev 282443)
@@ -0,0 +1,14 @@
+<svg xmlns="http://www.w3.org/2000/svg">
+    <defs>
+        <linearGradient id="base-gradient" x1="0" y1="0.5" x2="0" y2="0" gradientTransform="rotate(90)">
+            <stop offset="0%" stop-color="green"/>
+            <stop offset="50%" stop-color="green"/>
+            <stop offset="50%" stop-color="blue"/>
+            <stop offset="100%" stop-color="blue"/>
+        </linearGradient>
+        <linearGradient id="repeat-gradient" href="" spreadMethod="repeat" />
+        <linearGradient id="reflect-gradient" href="" spreadMethod="reflect" />
+    </defs>
+    <rect fill="url(#repeat-gradient)" width="100" height="100" />
+    <rect fill="url(#reflect-gradient)" x="110" width="100" height="100" />
+</svg>

Modified: trunk/Source/WebCore/ChangeLog (282442 => 282443)


--- trunk/Source/WebCore/ChangeLog	2021-09-15 07:19:07 UTC (rev 282442)
+++ trunk/Source/WebCore/ChangeLog	2021-09-15 07:26:18 UTC (rev 282443)
@@ -1,3 +1,30 @@
+2021-09-15  Said Abou-Hallawa  <[email protected]>
+
+        Linear gradient sometimes is drawn incorrectly and sometimes hangs
+        https://bugs.webkit.org/show_bug.cgi?id=230145
+        <rdar://82428383>
+
+        Reviewed by Simon Fraser.
+
+        Gradient::paint() has a few flaws:
+
+        1) We have to use atan2() instead of acos() to get the angle of the vector
+           (p0, p1) in the correct quadrant. acos() returns an angle between [0, pi].
+
+        2) The order in the case of 'repeat' and 'reflect' should be the following:
+            -- Move the points forward towards the bounding box.
+            -- Draw gradient and move the points forward till they are not
+               intersecting the bounding box.
+            -- Move the points backward towards the bounding box.
+            -- Draw gradient and move the points backward till they are not
+               intersecting the bounding box.
+
+        Tests: svg/gradients/gradient-flipped-start-end-points-expected.svg
+               svg/gradients/gradient-flipped-start-end-points.svg
+
+        * platform/graphics/cg/GradientCG.cpp:
+        (WebCore::Gradient::paint):
+
 2021-09-14  Simon Fraser  <[email protected]>
 
         Implement the borderBoxSize/contentBoxSize parts of ResizeObserver

Modified: trunk/Source/WebCore/platform/graphics/cg/GradientCG.cpp (282442 => 282443)


--- trunk/Source/WebCore/platform/graphics/cg/GradientCG.cpp	2021-09-15 07:19:07 UTC (rev 282442)
+++ trunk/Source/WebCore/platform/graphics/cg/GradientCG.cpp	2021-09-15 07:26:18 UTC (rev 282443)
@@ -119,51 +119,78 @@
             case GradientSpreadMethod::Repeat:
             case GradientSpreadMethod::Reflect: {
                 CGContextStateSaver saveState(platformContext);
+                extendOptions = 0;
 
                 FloatPoint gradientVectorNorm(data.point1 - data.point0);
                 gradientVectorNorm.normalize();
-                CGFloat angle = acos(gradientVectorNorm.dot({ 1, 0 }));
+                CGFloat angle = gradientVectorNorm.isZero() ? 0 : atan2(gradientVectorNorm.y(), gradientVectorNorm.x());
                 CGContextRotateCTM(platformContext, angle);
 
+                CGRect boundingBox = CGContextGetClipBoundingBox(platformContext);
+                if (CGRectIsInfinite(boundingBox) || CGRectIsEmpty(boundingBox))
+                    break;
+
                 CGAffineTransform transform = CGAffineTransformMakeRotation(-angle);
                 FloatPoint point0 = CGPointApplyAffineTransform(data.point0, transform);
                 FloatPoint point1 = CGPointApplyAffineTransform(data.point1, transform);
+                CGFloat dx = point1.x() - point0.x();
 
-                CGRect boundingBox = CGContextGetClipBoundingBox(platformContext);
-                CGFloat width = point1.x() - point0.x();
                 CGFloat pixelSize = CGFAbs(CGContextConvertSizeToUserSpace(platformContext, CGSizeMake(1, 1)).width);
+                if (CGFAbs(dx) < pixelSize)
+                    dx = dx < 0 ? -pixelSize : pixelSize;
 
-                if (width > 0 && !CGRectIsInfinite(boundingBox) && !CGRectIsEmpty(boundingBox)) {
-                    extendOptions = 0;
-                    if (width < pixelSize)
-                        width = pixelSize;
+                auto drawLinearGradient = [&](CGFloat start, CGFloat end, bool flip) {
+                    CGPoint left = CGPointMake(flip ? end : start, 0);
+                    CGPoint right = CGPointMake(flip ? start : end, 0);
 
-                    CGFloat gradientStart = point0.x();
-                    CGFloat gradientEnd = point1.x();
-                    bool flip = m_spreadMethod == GradientSpreadMethod::Reflect;
+                    CGContextDrawLinearGradient(platformContext, m_gradient.get(), left, right, extendOptions);
+                };
 
-                    // Find first gradient position to the left of the bounding box
-                    int n = CGFloor((boundingBox.origin.x - gradientStart) / width);
-                    gradientStart += n * width;
-                    if (!(n % 2))
-                        flip = false;
+                auto isLeftOf = [](CGFloat start, CGFloat end, CGRect boundingBox) -> bool {
+                    return std::max(start, end) <= CGRectGetMinX(boundingBox);
+                };
 
-                    gradientEnd -= CGFloor((gradientEnd - CGRectGetMaxX(boundingBox)) / width) * width;
+                auto isRightOf = [](CGFloat start, CGFloat end, CGRect boundingBox) -> bool {
+                    return std::min(start, end) >= CGRectGetMaxX(boundingBox);
+                };
 
-                    for (CGFloat start = gradientStart; start <= gradientEnd; start += width) {
-                        CGPoint left = CGPointMake(flip ? start + width : start, boundingBox.origin.y);
-                        CGPoint right = CGPointMake(flip ? start : start + width, boundingBox.origin.y);
+                auto isIntersecting = [](CGFloat start, CGFloat end, CGRect boundingBox) -> bool {
+                    return std::min(start, end) < CGRectGetMaxX(boundingBox) && CGRectGetMinX(boundingBox) < std::max(start, end);
+                };
 
-                        CGContextDrawLinearGradient(platformContext, m_gradient.get(), left, right, extendOptions);
+                bool flip = false;
+                CGFloat start = point0.x();
 
-                        if (m_spreadMethod == GradientSpreadMethod::Reflect)
-                            flip = !flip;
-                    }
+                // Should the points be moved forward towards boundingBox?
+                if ((dx > 0 && isLeftOf(start, start + dx, boundingBox)) || (dx < 0 && isRightOf(start, start + dx, boundingBox))) {
+                    // Move the 'start' point towards boundingBox.
+                    for (; !isIntersecting(start, start + dx, boundingBox); start += dx)
+                        flip = !flip && m_spreadMethod == GradientSpreadMethod::Reflect;
+                }
 
-                    break;
+                // Draw gradient forward till the points are outside boundingBox.
+                for (; isIntersecting(start, start + dx, boundingBox); start += dx) {
+                    drawLinearGradient(start, start + dx, flip);
+                    flip = !flip && m_spreadMethod == GradientSpreadMethod::Reflect;
                 }
 
-                FALLTHROUGH;
+                flip = m_spreadMethod == GradientSpreadMethod::Reflect;
+                CGFloat end = point0.x();
+
+                // Should the points be moved backward towards boundingBox?
+                if ((dx < 0 && isLeftOf(end, end - dx, boundingBox)) || (dx > 0 && isRightOf(end, end - dx, boundingBox))) {
+                    // Move the 'end' point towards boundingBox.
+                    for (; !isIntersecting(end, end - dx, boundingBox); end -= dx)
+                        flip = !flip && m_spreadMethod == GradientSpreadMethod::Reflect;
+                }
+
+                // Draw gradient backward till the points are outside boundingBox.
+                for (; isIntersecting(end, end - dx, boundingBox); end -= dx) {
+                    drawLinearGradient(end - dx, end, flip);
+                    flip = !flip && m_spreadMethod == GradientSpreadMethod::Reflect;
+                }
+
+                break;
             }
             case GradientSpreadMethod::Pad:
                 CGContextDrawLinearGradient(platformContext, m_gradient.get(), data.point0, data.point1, extendOptions);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to