Changeset 275055 in webkit for trunk/Source/WebCore/css/CSSGradientValue.cpp

Timestamp:

Mar 25, 2021, 1:46:08 PM (4 years ago)

Author:

[email protected]

Message:

Source/WebCore:
Fix edge cases in normalization of conic-gradient color stops
​https://bugs.webkit.org/show_bug.cgi?id=221294
<rdar://problem/74157218>

Patch by Tim Nguyen <​[email protected]> on 2021-03-25
Reviewed by Darin Adler.

lastOneOrLessIndex had an off-by-one error, so it did not go through the
branch handling clamping all the stop offsets when they're all above 1.

Also made normalizeStopsAndEndpointsOutsideRange logic easier to follow using
Optional<size_t> instead of placeholder values.

Test: web-platform-tests/css/css-images/normalization-conic-2.html

• css/CSSGradientValue.cpp:

(WebCore::ConicGradientAdapter::normalizeStopsAndEndpointsOutsideRange):

LayoutTests:
Enable normalization-conic-2.html WPT now that underlying bug is fixed.
Also skip some conic-gradient tests only on Windows instead of all platforms.
​https://bugs.webkit.org/show_bug.cgi?id=221294
<rdar://problem/74157218>

Patch by Tim Nguyen <​[email protected]> on 2021-03-25
Reviewed by Darin Adler.

• TestExpectations: enabled imported/w3c/web-platform-tests/css/css-images/normalization-conic-2.html.

File:

• trunk/Source/WebCore/css/CSSGradientValue.cpp (modified) (3 diffs)

trunk/Source/WebCore/css/CSSGradientValue.cpp

@@ -238 +238 @@
     void normalizeStopsAndEndpointsOutsideRange(Vector<GradientStop>& stops)
     {
-        auto numStops = stops.size();
-        
-        size_t firstZeroOrGreaterIndex = numStops;
+        size_t numStops = stops.size();
+        size_t lastStopIndex = numStops - 1;
+
+        Optional<size_t> firstZeroOrGreaterIndex;
         for (size_t i = 0; i < numStops; ++i) {
             if (*stops[i].offset >= 0) {
@@ -248 +249 @@
         }
 
-        if (firstZeroOrGreaterIndex > 0) {
-            if (firstZeroOrGreaterIndex < numStops && *stops[firstZeroOrGreaterIndex].offset > 0) {
-                float prevOffset = *stops[firstZeroOrGreaterIndex - 1].offset;
-                float nextOffset = *stops[firstZeroOrGreaterIndex].offset;
-                
-                float interStopProportion = -prevOffset / (nextOffset - prevOffset);
+        if (firstZeroOrGreaterIndex) {
+            size_t index = *firstZeroOrGreaterIndex;
+            if (index > 0) {
+                float previousOffset = *stops[index - 1].offset;
+                float nextOffset = *stops[index].offset;
+
+                float interStopProportion = -previousOffset / (nextOffset - previousOffset);
                 // FIXME: when we interpolate gradients using premultiplied colors, this should do premultiplication.
-                Color blendedColor = blend(stops[firstZeroOrGreaterIndex - 1].color, stops[firstZeroOrGreaterIndex].color, interStopProportion);
-                
+                Color blendedColor = blend(stops[index - 1].color, stops[index].color, interStopProportion);
+
                 // Clamp the positions to 0 and set the color.
-                for (size_t i = 0; i < firstZeroOrGreaterIndex; ++i) {
+                for (size_t i = 0; i < index; ++i) {
                     stops[i].offset = 0;
                     stops[i].color = blendedColor;
                 }
-            } else {
-                // All stops are below 0; just clamp them.
-                for (size_t i = 0; i < firstZeroOrGreaterIndex; ++i)
-                    stops[i].offset = 0;
-            }
-        }
-
-        size_t lastOneOrLessIndex = numStops;
-        for (int i = numStops - 1; i >= 0; --i) {
+            }
+        } else {
+            // All stop offsets below 0, clamp them.
+            for (auto& stop : stops)
+                stop.offset = 0;
+        }
+
+        Optional<size_t> lastOneOrLessIndex;
+        for (int i = lastStopIndex; i >= 0; --i) {
             if (*stops[i].offset <= 1) {
                 lastOneOrLessIndex = i;
@@ -276 +278 @@
             }
         }
-        
-        if (lastOneOrLessIndex < numStops - 1) {
-            if (lastOneOrLessIndex < numStops && *stops[lastOneOrLessIndex].offset < 1) {
-                float prevOffset = *stops[lastOneOrLessIndex].offset;
-                float nextOffset = *stops[lastOneOrLessIndex + 1].offset;
-                
-                float interStopProportion = (1 - prevOffset) / (nextOffset - prevOffset);
+
+        if (lastOneOrLessIndex) {
+            size_t index = *lastOneOrLessIndex;
+            if (index < lastStopIndex) {
+                float previousOffset = *stops[index].offset;
+                float nextOffset = *stops[index + 1].offset;
+
+                float interStopProportion = (1 - previousOffset) / (nextOffset - previousOffset);
                 // FIXME: when we interpolate gradients using premultiplied colors, this should do premultiplication.
-                Color blendedColor = blend(stops[lastOneOrLessIndex].color, stops[lastOneOrLessIndex + 1].color, interStopProportion);
-                
+                Color blendedColor = blend(stops[index].color, stops[index + 1].color, interStopProportion);
+
                 // Clamp the positions to 1 and set the color.
-                for (size_t i = lastOneOrLessIndex + 1; i < numStops; ++i) {
+                for (size_t i = index + 1; i <= lastStopIndex; ++i) {
                     stops[i].offset = 1;
                     stops[i].color = blendedColor;
                 }
-            } else {
-                // All stops are above 1; just clamp them.
-                for (size_t i = lastOneOrLessIndex; i < numStops; ++i)
-                    stops[i].offset = 1;
-            }
+            }
+        } else {
+            // All stop offsets above 1, clamp them.
+            for (auto& stop : stops)
+                stop.offset = 1;
         }
     }