Changeset 282443 in webkit

Timestamp:

Sep 15, 2021 12:26:18 AM (10 months ago)

Author:

Said Abou-Hallawa

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/svg/gradients/gradient-flipped-start-end-points-expected.svg (added)
• LayoutTests/svg/gradients/gradient-flipped-start-end-points.svg (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/platform/graphics/cg/GradientCG.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2021-09-15  Said Abou-Hallawa  <said@apple.com>
+
+        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  <mmaxfield@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-09-15  Said Abou-Hallawa  <said@apple.com>
+
+        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  <simon.fraser@apple.com>
 

trunk/Source/WebCore/platform/graphics/cg/GradientCG.cpp

@@ -120 +120 @@
             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);
-
-                CGRect boundingBox = CGContextGetClipBoundingBox(platformContext);
-                CGFloat width = point1.x() - point0.x();
+                CGFloat dx = point1.x() - point0.x();
+
                 CGFloat pixelSize = CGFAbs(CGContextConvertSizeToUserSpace(platformContext, CGSizeMake(1, 1)).width);
-
-                if (width > 0 && !CGRectIsInfinite(boundingBox) && !CGRectIsEmpty(boundingBox)) {
-                    extendOptions = 0;
-                    if (width < pixelSize)
-                        width = pixelSize;
-
-                    CGFloat gradientStart = point0.x();
-                    CGFloat gradientEnd = point1.x();
-                    bool flip = m_spreadMethod == GradientSpreadMethod::Reflect;
-
-                    // 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;
-
-                    gradientEnd -= CGFloor((gradientEnd - CGRectGetMaxX(boundingBox)) / width) * width;
-
-                    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);
-
-                        CGContextDrawLinearGradient(platformContext, m_gradient.get(), left, right, extendOptions);
-
-                        if (m_spreadMethod == GradientSpreadMethod::Reflect)
-                            flip = !flip;
-                    }
-
-                    break;
-                }
-
-                FALLTHROUGH;
+                if (CGFAbs(dx) < pixelSize)
+                    dx = dx < 0 ? -pixelSize : pixelSize;
+
+                auto drawLinearGradient = [&](CGFloat start, CGFloat end, bool flip) {
+                    CGPoint left = CGPointMake(flip ? end : start, 0);
+                    CGPoint right = CGPointMake(flip ? start : end, 0);
+
+                    CGContextDrawLinearGradient(platformContext, m_gradient.get(), left, right, extendOptions);
+                };
+
+                auto isLeftOf = [](CGFloat start, CGFloat end, CGRect boundingBox) -> bool {
+                    return std::max(start, end) <= CGRectGetMinX(boundingBox);
+                };
+
+                auto isRightOf = [](CGFloat start, CGFloat end, CGRect boundingBox) -> bool {
+                    return std::min(start, end) >= CGRectGetMaxX(boundingBox);
+                };
+
+                auto isIntersecting = [](CGFloat start, CGFloat end, CGRect boundingBox) -> bool {
+                    return std::min(start, end) < CGRectGetMaxX(boundingBox) && CGRectGetMinX(boundingBox) < std::max(start, end);
+                };
+
+                bool flip = false;
+                CGFloat start = point0.x();
+
+                // 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;
+                }
+
+                // 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;
+                }
+
+                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: