Skip to content

Commit

Permalink
Refactor computed <paint> value handling
Browse files Browse the repository at this point in the history
Combine all the fields that make up a <paint> into a class: SVGPaint.
Move the bulk of the style builder function for <paint> from the
template to StyleBuilderConverter. This should make future work on
<paint> resolution easier.
Also split the existing setters on {SVG,}ComputedStyle to avoid having
bools to indicate if the "visited" portion should be set or not. The
new way more closely resemble other color properties.
Add accessors to SVGPaint to both make code easier to read, and reduce
the dependence on SVGPaintType.

Bug: 769774
Change-Id: Ifad043c9e423f0fc7aa79fef78e635b2abb4636c
Reviewed-on: https://chromium-review.googlesource.com/961062
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Fredrik Söderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/master@{#543333}
  • Loading branch information
Fredrik Söderquist authored and Commit Bot committed Mar 15, 2018
1 parent dbbbaa9 commit dcbfc95
Show file tree
Hide file tree
Showing 17 changed files with 203 additions and 335 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -278,56 +278,28 @@ namespace blink {

{% macro apply_svg_paint(property_id, paint_type) %}
{% set property = properties_by_id[property_id] %}
{% set visited_link_setter = 'SetVisitedLink' + paint_type %}
{{declare_initial_function(property_id)}} {
{{set_value(property)}}(
SVGComputedStyle::Initial{{paint_type}}Type(),
SVGComputedStyle::Initial{{paint_type}}Color(),
SVGComputedStyle::Initial{{paint_type}}Uri(),
state.ApplyPropertyToRegularStyle(),
state.ApplyPropertyToVisitedLinkStyle());
if (state.ApplyPropertyToRegularStyle())
{{set_value(property)}}(SVGComputedStyle::Initial{{paint_type}}());
if (state.ApplyPropertyToVisitedLinkStyle())
state.Style()->AccessSVGStyle().{{visited_link_setter}}(SVGComputedStyle::Initial{{paint_type}}());
}

{{declare_inherit_function(property_id)}} {
const SVGComputedStyle& svgParentStyle = state.ParentStyle()->SvgStyle();
{{set_value(property)}}(
svgParentStyle.{{paint_type}}Type(),
svgParentStyle.{{paint_type}}Color(),
svgParentStyle.{{paint_type}}Uri(),
state.ApplyPropertyToRegularStyle(),
state.ApplyPropertyToVisitedLinkStyle());
const SVGComputedStyle& parent_svg_style = state.ParentStyle()->SvgStyle();
if (state.ApplyPropertyToRegularStyle())
{{set_value(property)}}(parent_svg_style.{{paint_type}}());
if (state.ApplyPropertyToVisitedLinkStyle())
state.Style()->AccessSVGStyle().{{visited_link_setter}}(parent_svg_style.{{paint_type}}());
}

{{declare_value_function(property_id)}} {
const CSSValue* localValue = &value;
String url;
if (value.IsValueList()) {
const CSSValueList& list = ToCSSValueList(value);
DCHECK_EQ(list.length(), 2U);
url = ToCSSURIValue(list.Item(0)).Value();
localValue = &list.Item(1);
}

Color color;
SVGPaintType paintType = SVG_PAINTTYPE_RGBCOLOR;
if (localValue->IsURIValue()) {
paintType = SVG_PAINTTYPE_URI;
url = ToCSSURIValue(localValue)->Value();
} else if (localValue->IsIdentifierValue() &&
ToCSSIdentifierValue(localValue)->GetValueID() == CSSValueNone) {
paintType = url.IsEmpty() ? SVG_PAINTTYPE_NONE : SVG_PAINTTYPE_URI_NONE;
} else if (localValue->IsIdentifierValue() &&
ToCSSIdentifierValue(localValue)->GetValueID() == CSSValueCurrentcolor) {
color = state.Style()->GetColor();
paintType = url.IsEmpty() ? SVG_PAINTTYPE_CURRENTCOLOR
: SVG_PAINTTYPE_URI_CURRENTCOLOR;
} else {
color = StyleBuilderConverter::ConvertColor(state, *localValue);
paintType = url.IsEmpty() ? SVG_PAINTTYPE_RGBCOLOR
: SVG_PAINTTYPE_URI_RGBCOLOR;
}
{{set_value(property)}}(paintType, color, url,
state.ApplyPropertyToRegularStyle(),
state.ApplyPropertyToVisitedLinkStyle());
SVGPaint paint = StyleBuilderConverter::ConvertSVGPaint(state, value);
if (state.ApplyPropertyToRegularStyle())
{{set_value(property)}}(paint);
if (state.ApplyPropertyToVisitedLinkStyle())
state.Style()->AccessSVGStyle().{{visited_link_setter}}(paint);
}
{% endmacro %}
{{apply_svg_paint('CSSPropertyFill', 'FillPaint')}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,36 +17,31 @@
namespace blink {

namespace {
static bool GetColorFromPaint(const SVGPaintType type,
const Color color,
StyleColor& result) {
switch (type) {
case SVG_PAINTTYPE_RGBCOLOR:
result = color;
return true;
case SVG_PAINTTYPE_CURRENTCOLOR:
result = StyleColor::CurrentColor();
return true;
default:
return false;
}

static bool GetColorFromPaint(const SVGPaint& paint, StyleColor& result) {
if (!paint.IsColor())
return false;
if (paint.HasCurrentColor())
result = StyleColor::CurrentColor();
else
result = paint.GetColor();
return true;
}

bool GetColor(const CSSProperty& property,
const ComputedStyle& style,
StyleColor& result) {
switch (property.PropertyID()) {
case CSSPropertyFill:
return GetColorFromPaint(style.SvgStyle().FillPaintType(),
style.SvgStyle().FillPaintColor(), result);
return GetColorFromPaint(style.SvgStyle().FillPaint(), result);
case CSSPropertyStroke:
return GetColorFromPaint(style.SvgStyle().StrokePaintType(),
style.SvgStyle().StrokePaintColor(), result);
return GetColorFromPaint(style.SvgStyle().StrokePaint(), result);
default:
NOTREACHED();
return false;
}
}

} // namespace

InterpolationValue CSSPaintInterpolationType::MaybeConvertNeutral(
Expand Down Expand Up @@ -143,14 +138,15 @@ void CSSPaintInterpolationType::ApplyStandardPropertyValue(
StyleResolverState& state) const {
Color color = CSSColorInterpolationType::ResolveInterpolableColor(
interpolable_color, state);
SVGComputedStyle& mutable_svg_style = state.Style()->AccessSVGStyle();
switch (CssProperty().PropertyID()) {
case CSSPropertyFill:
state.Style()->AccessSVGStyle().SetFillPaint(SVG_PAINTTYPE_RGBCOLOR,
color, String(), true, true);
mutable_svg_style.SetFillPaint(SVGPaint(color));
mutable_svg_style.SetVisitedLinkFillPaint(SVGPaint(color));
break;
case CSSPropertyStroke:
state.Style()->AccessSVGStyle().SetStrokePaint(
SVG_PAINTTYPE_RGBCOLOR, color, String(), true, true);
mutable_svg_style.SetStrokePaint(SVGPaint(color));
mutable_svg_style.SetVisitedLinkStrokePaint(SVGPaint(color));
break;
default:
NOTREACHED();
Expand Down
22 changes: 6 additions & 16 deletions third_party/WebKit/Source/core/css/CSSPropertyEquality.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -134,14 +134,9 @@ bool CSSPropertyEquality::PropertiesEqual(const PropertyHandle& property,
case CSSPropertyFill: {
const SVGComputedStyle& a_svg = a.SvgStyle();
const SVGComputedStyle& b_svg = b.SvgStyle();
return a_svg.FillPaintType() == b_svg.FillPaintType() &&
(a_svg.FillPaintType() != SVG_PAINTTYPE_RGBCOLOR ||
a_svg.FillPaintColor() == b_svg.FillPaintColor()) &&
a_svg.VisitedLinkFillPaintType() ==
b_svg.VisitedLinkFillPaintType() &&
(a_svg.VisitedLinkFillPaintType() != SVG_PAINTTYPE_RGBCOLOR ||
a_svg.VisitedLinkFillPaintColor() ==
b_svg.VisitedLinkFillPaintColor());
return a_svg.FillPaint().EqualTypeOrColor(b_svg.FillPaint()) &&
a_svg.VisitedLinkFillPaint().EqualTypeOrColor(
b_svg.VisitedLinkFillPaint());
}
case CSSPropertyFillOpacity:
return a.FillOpacity() == b.FillOpacity();
Expand Down Expand Up @@ -248,14 +243,9 @@ bool CSSPropertyEquality::PropertiesEqual(const PropertyHandle& property,
case CSSPropertyStroke: {
const SVGComputedStyle& a_svg = a.SvgStyle();
const SVGComputedStyle& b_svg = b.SvgStyle();
return a_svg.StrokePaintType() == b_svg.StrokePaintType() &&
(a_svg.StrokePaintType() != SVG_PAINTTYPE_RGBCOLOR ||
a_svg.StrokePaintColor() == b_svg.StrokePaintColor()) &&
a_svg.VisitedLinkStrokePaintType() ==
b_svg.VisitedLinkStrokePaintType() &&
(a_svg.VisitedLinkStrokePaintType() != SVG_PAINTTYPE_RGBCOLOR ||
a_svg.VisitedLinkStrokePaintColor() ==
b_svg.VisitedLinkStrokePaintColor());
return a_svg.StrokePaint().EqualTypeOrColor(b_svg.StrokePaint()) &&
a_svg.VisitedLinkStrokePaint().EqualTypeOrColor(
b_svg.VisitedLinkStrokePaint());
}
case CSSPropertyStrokeDasharray:
return a.StrokeDashArray() == b.StrokeDashArray();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1866,27 +1866,25 @@ CSSValue* ComputedStyleUtils::PaintOrderToCSSValueList(
}

CSSValue* ComputedStyleUtils::AdjustSVGPaintForCurrentColor(
SVGPaintType paint_type,
const String& url,
const Color& color,
const SVGPaint& paint,
const Color& current_color) {
if (paint_type >= SVG_PAINTTYPE_URI_NONE) {
if (paint.type >= SVG_PAINTTYPE_URI_NONE) {
CSSValueList* values = CSSValueList::CreateSpaceSeparated();
values->Append(*CSSURIValue::Create(AtomicString(url)));
if (paint_type == SVG_PAINTTYPE_URI_NONE)
values->Append(*CSSURIValue::Create(AtomicString(paint.GetUrl())));
if (paint.type == SVG_PAINTTYPE_URI_NONE)
values->Append(*CSSIdentifierValue::Create(CSSValueNone));
else if (paint_type == SVG_PAINTTYPE_URI_CURRENTCOLOR)
else if (paint.type == SVG_PAINTTYPE_URI_CURRENTCOLOR)
values->Append(*CSSColorValue::Create(current_color.Rgb()));
else if (paint_type == SVG_PAINTTYPE_URI_RGBCOLOR)
values->Append(*CSSColorValue::Create(color.Rgb()));
else if (paint.type == SVG_PAINTTYPE_URI_RGBCOLOR)
values->Append(*CSSColorValue::Create(paint.GetColor().Rgb()));
return values;
}
if (paint_type == SVG_PAINTTYPE_NONE)
if (paint.type == SVG_PAINTTYPE_NONE)
return CSSIdentifierValue::Create(CSSValueNone);
if (paint_type == SVG_PAINTTYPE_CURRENTCOLOR)
if (paint.type == SVG_PAINTTYPE_CURRENTCOLOR)
return CSSColorValue::Create(current_color.Rgb());

return CSSColorValue::Create(color.Rgb());
return CSSColorValue::Create(paint.GetColor().Rgb());
}

CSSValue* ComputedStyleUtils::ValueForShadowData(const ShadowData& shadow,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,7 @@ class ComputedStyleUtils {
static CSSValue* StrokeDashArrayToCSSValueList(const SVGDashArray&,
const ComputedStyle&);
static CSSValue* PaintOrderToCSSValueList(const SVGComputedStyle&);
static CSSValue* AdjustSVGPaintForCurrentColor(SVGPaintType,
const String&,
const Color&,
const Color&);
static CSSValue* AdjustSVGPaintForCurrentColor(const SVGPaint&, const Color&);
static CSSValue* ValueForShadowData(const ShadowData&,
const ComputedStyle&,
bool use_spread);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ const CSSValue* Fill::CSSValueFromComputedStyleInternal(
Node* styled_node,
bool allow_visited_style) const {
return ComputedStyleUtils::AdjustSVGPaintForCurrentColor(
svg_style.FillPaintType(), svg_style.FillPaintUri(),
svg_style.FillPaintColor(), style.GetColor());
svg_style.FillPaint(), style.GetColor());
}

} // namespace CSSLonghand
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ const CSSValue* Stroke::CSSValueFromComputedStyleInternal(
Node* styled_node,
bool allow_visited_style) const {
return ComputedStyleUtils::AdjustSVGPaintForCurrentColor(
svg_style.StrokePaintType(), svg_style.StrokePaintUri(),
svg_style.StrokePaintColor(), style.GetColor());
svg_style.StrokePaint(), style.GetColor());
}

} // namespace CSSLonghand
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1396,6 +1396,38 @@ StyleAutoColor StyleBuilderConverter::ConvertStyleAutoColor(
value, Color(), for_visited_link);
}

SVGPaint StyleBuilderConverter::ConvertSVGPaint(StyleResolverState& state,
const CSSValue& value) {
const CSSValue* local_value = &value;
SVGPaint paint;
if (value.IsValueList()) {
const CSSValueList& list = ToCSSValueList(value);
DCHECK_EQ(list.length(), 2u);
paint.url = ToCSSURIValue(list.Item(0)).Value();
local_value = &list.Item(1);
}

if (local_value->IsURIValue()) {
paint.type = SVG_PAINTTYPE_URI;
paint.url = ToCSSURIValue(local_value)->Value();
} else if (local_value->IsIdentifierValue() &&
ToCSSIdentifierValue(local_value)->GetValueID() == CSSValueNone) {
paint.type =
paint.url.IsEmpty() ? SVG_PAINTTYPE_NONE : SVG_PAINTTYPE_URI_NONE;
} else if (local_value->IsIdentifierValue() &&
ToCSSIdentifierValue(local_value)->GetValueID() ==
CSSValueCurrentcolor) {
paint.color = state.Style()->GetColor();
paint.type = paint.url.IsEmpty() ? SVG_PAINTTYPE_CURRENTCOLOR
: SVG_PAINTTYPE_URI_CURRENTCOLOR;
} else {
paint.color = ConvertColor(state, *local_value);
paint.type = paint.url.IsEmpty() ? SVG_PAINTTYPE_RGBCOLOR
: SVG_PAINTTYPE_URI_RGBCOLOR;
}
return paint;
}

TextEmphasisPosition StyleBuilderConverter::ConvertTextTextEmphasisPosition(
StyleResolverState& state,
const CSSValue& value) {
Expand All @@ -1414,6 +1446,7 @@ TextEmphasisPosition StyleBuilderConverter::ConvertTextTextEmphasisPosition(
return TextEmphasisPosition::kUnderLeft;
return TextEmphasisPosition::kOverRight;
}

float StyleBuilderConverter::ConvertTextStrokeWidth(StyleResolverState& state,
const CSSValue& value) {
if (value.IsIdentifierValue() && ToCSSIdentifierValue(value).GetValueID()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ class StyleBuilderConverter {
static StyleAutoColor ConvertStyleAutoColor(StyleResolverState&,
const CSSValue&,
bool for_visited_link = false);
static SVGPaint ConvertSVGPaint(StyleResolverState&, const CSSValue&);
static TextEmphasisPosition ConvertTextTextEmphasisPosition(
StyleResolverState&,
const CSSValue&);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,66 +66,42 @@ void SVGPaintServer::PrependTransform(const AffineTransform& transform) {
static SVGPaintDescription RequestPaint(const LayoutObject& object,
const ComputedStyle& style,
LayoutSVGResourceMode mode) {
// If we have no style at all, ignore it.
bool apply_to_fill = mode == kApplyToFillMode;

const SVGComputedStyle& svg_style = style.SvgStyle();
const SVGPaint& paint =
apply_to_fill ? svg_style.FillPaint() : svg_style.StrokePaint();
const SVGPaint& visited_paint = apply_to_fill
? svg_style.VisitedLinkFillPaint()
: svg_style.VisitedLinkStrokePaint();

// If we have no fill/stroke, return 0.
if (mode == kApplyToFillMode) {
if (!svg_style.HasFill())
return SVGPaintDescription();
} else {
if (!svg_style.HasStroke())
return SVGPaintDescription();
}
// If we have no, ignore it.
if (paint.IsNone())
return SVGPaintDescription();

bool apply_to_fill = mode == kApplyToFillMode;
SVGPaintType paint_type =
apply_to_fill ? svg_style.FillPaintType() : svg_style.StrokePaintType();
DCHECK_NE(paint_type, SVG_PAINTTYPE_NONE);

Color color;
bool has_color = false;
switch (paint_type) {
case SVG_PAINTTYPE_CURRENTCOLOR:
case SVG_PAINTTYPE_URI_CURRENTCOLOR:
// The keyword `currentcolor` takes its value from the value of the
// `color` property on the same element.
color = style.VisitedDependentColor(GetCSSPropertyColor());
has_color = true;
break;
case SVG_PAINTTYPE_RGBCOLOR:
case SVG_PAINTTYPE_URI_RGBCOLOR:
color = apply_to_fill ? svg_style.FillPaintColor()
: svg_style.StrokePaintColor();
has_color = true;
break;
default:
break;
}
Color color = paint.GetColor();
bool has_color = paint.HasColor();

if (paint.HasCurrentColor())
color = style.VisitedDependentColor(GetCSSPropertyColor());

if (style.InsideLink() == EInsideLink::kInsideVisitedLink) {
// FIXME: This code doesn't support the uri component of the visited link
// paint, https://bugs.webkit.org/show_bug.cgi?id=70006
SVGPaintType visited_paint_type =
apply_to_fill ? svg_style.VisitedLinkFillPaintType()
: svg_style.VisitedLinkStrokePaintType();

// For SVG_PAINTTYPE_CURRENTCOLOR, 'color' already contains the
// 'visitedColor'.
if (visited_paint_type < SVG_PAINTTYPE_URI_NONE &&
visited_paint_type != SVG_PAINTTYPE_CURRENTCOLOR) {
const Color& visited_color =
apply_to_fill ? svg_style.VisitedLinkFillPaintColor()
: svg_style.VisitedLinkStrokePaintColor();

// If the color (primary or fallback) is 'currentcolor', then |color|
// already contains the 'visited color'.
if (!visited_paint.HasUrl() && !visited_paint.HasCurrentColor()) {
const Color& visited_color = visited_paint.GetColor();
color = Color(visited_color.Red(), visited_color.Green(),
visited_color.Blue(), color.Alpha());
has_color = true;
}
}

// If the primary resource is just a color, return immediately.
if (paint_type < SVG_PAINTTYPE_URI_NONE) {
// |paintType| will be either <current-color> or <rgb-color> here - both of
if (!paint.HasUrl()) {
// |paint.type| will be either <current-color> or <rgb-color> here - both of
// which will have a color.
DCHECK(has_color);
return SVGPaintDescription(color);
Expand All @@ -141,7 +117,7 @@ static SVGPaintDescription RequestPaint(const LayoutObject& object,
if (!uri_resource) {
// The fallback is 'none'. (SVG2 say 'none' is implied when no fallback is
// specified.)
if (paint_type == SVG_PAINTTYPE_URI_NONE || !has_color)
if (!paint.HasFallbackColor() || !has_color)
return SVGPaintDescription();

return SVGPaintDescription(color);
Expand Down
Loading

0 comments on commit dcbfc95

Please sign in to comment.