Initial conic-gradient() implementation

third_party/WebKit/Source/core/css/CSSGradientValue.cpp

 /*
  * Copyright (C) 2008 Apple Inc.  All rights reserved.
  * Copyright (C) 2015 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
@@ skipping 440 matching lines @@
   if (m_gradientType == CSSDeprecatedLinearGradient ||
       m_gradientType == CSSDeprecatedRadialGradient) {
     addDeprecatedStops(desc, object);
     return;
   }
 
   size_t numStops = m_stops.size();
 
   Vector<GradientStop> stops(numStops);
 
   bool hasHints = false;

[fs, 2017/03/30 22:09:11]

Nit: Maybe move this down to just before the for() while we're at it.

[f(malita), 2017/03/31 13:56:23]

Done.

+  float gradientLength;
 
-  FloatPoint gradientStart = desc.p0;
-  FloatPoint gradientEnd;
-  if (isLinearGradientValue())
-    gradientEnd = desc.p1;
-  else if (isRadialGradientValue())
-    gradientEnd = gradientStart + FloatSize(desc.r1, 0);
-  float gradientLength =
-      FloatSize(gradientStart - gradientEnd).diagonalLength();
+  switch (getClassType()) {

[fs, 2017/03/30 22:09:11]

(The old code here had me scratch my head...)

+    case LinearGradientClass:
+      gradientLength = FloatSize(desc.p1 - desc.p0).diagonalLength();
+      break;
+    case RadialGradientClass:
+      gradientLength = desc.r1;
+      break;
+    case ConicGradientClass:
+      gradientLength = 1;
+      break;
+    default:
+      NOTREACHED();
+      gradientLength = 0;
+  }
 
   for (size_t i = 0; i < numStops; ++i) {
     const CSSGradientColorStop& stop = m_stops[i];
 
     if (stop.isHint())
       hasHints = true;
     else
       stops[i].color = resolveStopColor(*stop.m_color, object);
 
     if (stop.m_offset) {
@@ skipping 78 matching lines @@
     }
   }
 
   ASSERT(stops.size() == m_stops.size());
   if (hasHints) {
     replaceColorHintsWithColorStops(stops, m_stops);
   }
 
   // At this point we have a fully resolved set of stops. Time to perform
   // adjustments for repeat gradients and degenerate values if needed.
-  if (requiresStopsNormalization(stops, desc)) {
-    // Negative offsets are only an issue for non-repeating radial gradients:
-    // linear gradient points can be repositioned arbitrarily, and for repeating
-    // radial gradients we shift the radii into equivalent positive values.
-    if (isRadialGradientValue() && !m_repeating)
-      clampNegativeOffsets(stops);
-
-    if (normalizeAndAddStops(stops, desc)) {
-      if (isLinearGradientValue()) {
-        adjustGradientPointsForOffsetRange(desc, stops.front().offset,
-                                           stops.back().offset);
-      } else {
-        adjustGradientRadiiForOffsetRange(desc, stops.front().offset,
-                                          stops.back().offset);
-      }
-    } else {
-      // Normalization failed because the stop set is coincident.
-    }
-  } else {
+  // Note: the normalization trick doesn't work for conic gradients, because
+  // the underlying Skia implementation doesn't support tiling.
+  if (isConicGradientValue() || !requiresStopsNormalization(stops, desc)) {
     // No normalization required, just add the current stops.
     for (const auto& stop : stops)
       desc.stops.emplace_back(stop.offset, stop.color);
+    return;
+  }
+
+  switch (getClassType()) {
+    case LinearGradientClass:
+      if (normalizeAndAddStops(stops, desc)) {
+        adjustGradientPointsForOffsetRange(desc, stops.front().offset,
+                                           stops.back().offset);
+      }
+      break;
+    case RadialGradientClass:
+      // Negative offsets are only an issue for non-repeating radial gradients:
+      // linear gradient points can be repositioned arbitrarily, and for
+      // repeating radial gradients we shift the radii into equivalent positive
+      // values.
+      if (!m_repeating)
+        clampNegativeOffsets(stops);
+
+      if (normalizeAndAddStops(stops, desc)) {
+        adjustGradientRadiiForOffsetRange(desc, stops.front().offset,
+                                          stops.back().offset);
+    }

[fs, 2017/03/30 22:09:11]

Strange indent (here and next line)? (Or late hour?)

[f(malita), 2017/03/31 13:56:23]

Done.

+    break;
+    default:
+      NOTREACHED();
   }
 }
 
 static float positionFromValue(const CSSValue* value,
                                const CSSToLengthConversionData& conversionData,
                                const IntSize& size,
                                bool isHorizontal) {
   int origin = 0;
   int sign = 1;
   int edgeDistance = isHorizontal ? size.width() : size.height();
@@ skipping 760 matching lines @@
   result.append(')');
   return result.toString();
 }
 
 PassRefPtr<Gradient> CSSConicGradientValue::createGradient(
     const CSSToLengthConversionData& conversionData,
     const IntSize& size,
     const LayoutObject& object) {
   DCHECK(!size.isEmpty());
 
-  // TODO(fmalita): implement
-  return Gradient::createLinear(FloatPoint(), FloatPoint());
+  const float angle = m_fromAngle ? m_fromAngle->computeDegrees() : 0;
+
+  const FloatPoint position(
+      m_firstX ? positionFromValue(m_firstX, conversionData, size, true)
+               : size.width() / 2,
+      m_firstY ? positionFromValue(m_firstY, conversionData, size, false)
+               : size.height() / 2);
+
+  GradientDesc desc(position, position,
+                    m_repeating ? SpreadMethodRepeat : SpreadMethodPad);
+  addStops(desc, conversionData, object);
+
+  RefPtr<Gradient> gradient = Gradient::createConic(position, angle);

[fs, 2017/03/30 22:09:11]

position, angle, ..., Gradient::ColorInterpolation::Premultiplied

right? Whether ... is desc.spreadMethod or not I guess doesn't matter much for
now, but maybe best to use that.

(Maybe this also means we should have a test for stops with alpha? or
'transparent' stops)

[f(malita), 2017/03/31 13:56:23]

Good catch, done.
The last conic-gradient.html example uses some translucent colors and does catch
this diff.

[fs, 2017/03/31 14:05:43]

Yeah, I saw that... it's very subtle though. So maybe a more "plain" test would
be good to have - like "conic-gradient(blue, transparent)", where the effect
ought to be fairly noticable.

+  gradient->addColorStops(desc.stops);
+
+  return gradient.release();
 }
 
 bool CSSConicGradientValue::equals(const CSSConicGradientValue& other) const {
   return m_repeating == other.m_repeating &&
          dataEquivalent(m_firstX, other.m_firstX) &&
          dataEquivalent(m_firstY, other.m_firstY) &&
          dataEquivalent(m_fromAngle, other.m_fromAngle) &&
          m_stops == other.m_stops;
 }
 
 DEFINE_TRACE_AFTER_DISPATCH(CSSConicGradientValue) {
   visitor->trace(m_fromAngle);
   CSSGradientValue::traceAfterDispatch(visitor);
 }
 
 }  // namespace blink