[webkit-reviews] review granted: [Bug 137171] Add support for midpoint to CSS gradients : [Attachment 238995] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Oct 1 07:36:37 PDT 2014
Darin Adler <darin at apple.com> has granted Rik Cabanier <cabanier at adobe.com>'s
request for review:
Bug 137171: Add support for midpoint to CSS gradients
https://bugs.webkit.org/show_bug.cgi?id=137171
Attachment 238995: Patch
https://bugs.webkit.org/attachment.cgi?id=238995&action=review
------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=238995&action=review
> Source/WebCore/css/CSSGradientValue.cpp:116
> + for (unsigned i = 0; i < m_stops.size(); i++) {
> + if (!m_stops[i].isMidpoint &&
styleResolver->colorFromPrimitiveValueIsDerivedFromElement(m_stops[i].m_color.g
et())) {
> m_stops[i].m_colorIsDerivedFromElement = true;
> derived = true;
> break;
> }
> + }
Should come back here after this patch and change this to:
for (auto& stop : m_stops)
This would read more clearly written that way.
> Source/WebCore/css/CSSGradientValue.cpp:133
> + for (unsigned i = 0; i < result->m_stops.size(); i++) {
> + if (!result->m_stops[i].isMidpoint)
> + result->m_stops[i].m_resolvedColor =
styleResolver->colorFromPrimitiveValue(result->m_stops[i].m_color.get());
> + }
Should come back here after this patch and change this to:
for (auto& stop : result->m_stops)
This would read more clearly written that way.
> Source/WebCore/css/CSSGradientValue.cpp:279
> + // a line in that region. We then add 5 more color stops on that side to
minimize the change
> + // how the luminance changes at each of the colorstops. We don't have to
add as many on the other side
"minimize the change how the luminance changes"?
"colorstops" -> "color stops"
More information about the webkit-reviews
mailing list