From 04e03bc1816b20ff5f0eb5d3a41ee2e725f8a953 Mon Sep 17 00:00:00 2001 From: Tyler Denniston Date: Wed, 9 Dec 2020 14:16:25 -0500 Subject: [PATCH] [svg] Convert stop-color and stop-opacity to presentation attrs These are somewhat the first presentation attributes of their kind, in that they are non-inherited but also not applied via canvas layers. Implementation-wise the main difference is that these attributes are not propagated through the fInherited field of the render context's presentation attribute list. Change-Id: I0909507b0ecbd21732b3f80c46a343f5a0a9bf7a Reviewed-on: https://skia-review.googlesource.com/c/skia/+/340661 Reviewed-by: Florin Malita Commit-Queue: Tyler Denniston --- modules/svg/include/SkSVGAttribute.h | 6 ++---- modules/svg/include/SkSVGAttributeParser.h | 1 - modules/svg/include/SkSVGNode.h | 2 ++ modules/svg/include/SkSVGStop.h | 8 +------ modules/svg/src/SkSVGAttribute.cpp | 3 +++ modules/svg/src/SkSVGAttributeParser.cpp | 17 --------------- modules/svg/src/SkSVGDOM.cpp | 25 ---------------------- modules/svg/src/SkSVGGradient.cpp | 25 ++++++++++++---------- modules/svg/src/SkSVGNode.cpp | 8 ++++++- modules/svg/src/SkSVGRenderContext.cpp | 6 ++++++ modules/svg/src/SkSVGStop.cpp | 18 ---------------- 11 files changed, 35 insertions(+), 84 deletions(-) diff --git a/modules/svg/include/SkSVGAttribute.h b/modules/svg/include/SkSVGAttribute.h index 7bf1a63a50..f75b383dbb 100644 --- a/modules/svg/include/SkSVGAttribute.h +++ b/modules/svg/include/SkSVGAttribute.h @@ -43,8 +43,6 @@ enum class SkSVGAttribute { kRx, // ,: horizontal (corner) radius kRy, // ,: vertical (corner) radius kSpreadMethod, - kStopColor, - kStopOpacity, kStroke, kStrokeDashArray, kStrokeDashOffset, @@ -98,12 +96,12 @@ struct SkSVGPresentationAttributes { SkSVGProperty fFontWeight; SkSVGProperty fTextAnchor; - // TODO(tdenniston): add SkSVGStopColor - // uninherited SkSVGProperty fOpacity; SkSVGProperty fClipPath; SkSVGProperty fFilter; + SkSVGProperty fStopColor; + SkSVGProperty fStopOpacity; }; #endif // SkSVGAttribute_DEFINED diff --git a/modules/svg/include/SkSVGAttributeParser.h b/modules/svg/include/SkSVGAttributeParser.h index 72114e6565..41f772f493 100644 --- a/modules/svg/include/SkSVGAttributeParser.h +++ b/modules/svg/include/SkSVGAttributeParser.h @@ -19,7 +19,6 @@ public: bool parseInteger(SkSVGIntegerType*); bool parseViewBox(SkSVGViewBoxType*); bool parsePoints(SkSVGPointsType*); - bool parseStopColor(SkSVGStopColor*); bool parsePreserveAspectRatio(SkSVGPreserveAspectRatio*); // TODO: Migrate all parse*() functions to this style (and delete the old version) diff --git a/modules/svg/include/SkSVGNode.h b/modules/svg/include/SkSVGNode.h index 04d8bd4ce5..9c7ce16d69 100644 --- a/modules/svg/include/SkSVGNode.h +++ b/modules/svg/include/SkSVGNode.h @@ -121,6 +121,8 @@ public: SVG_PRES_ATTR(ClipPath , SkSVGClip , false) SVG_PRES_ATTR(Filter , SkSVGFilterType, false) SVG_PRES_ATTR(Opacity , SkSVGNumberType, false) + SVG_PRES_ATTR(StopColor , SkSVGColor , false) + SVG_PRES_ATTR(StopOpacity , SkSVGNumberType, false) protected: SkSVGNode(SkSVGTag); diff --git a/modules/svg/include/SkSVGStop.h b/modules/svg/include/SkSVGStop.h index a558f57261..8b5f9501a7 100644 --- a/modules/svg/include/SkSVGStop.h +++ b/modules/svg/include/SkSVGStop.h @@ -21,12 +21,8 @@ public: } const SkSVGLength& offset() const { return fOffset; } - const SkSVGStopColor& stopColor() const { return fStopColor; } - const SkSVGNumberType& stopOpacity() const { return fStopOpacity; } void setOffset(const SkSVGLength&); - void setStopColor(const SkSVGStopColor&); - void setStopOpacity(const SkSVGNumberType&); protected: void onSetAttribute(SkSVGAttribute, const SkSVGValue&) override; @@ -34,9 +30,7 @@ protected: private: SkSVGStop(); - SkSVGLength fOffset = SkSVGLength(0 , SkSVGLength::Unit::kPercentage); - SkSVGStopColor fStopColor = SkSVGStopColor(SK_ColorBLACK); - SkSVGNumberType fStopOpacity = SkSVGNumberType(1); + SkSVGLength fOffset = SkSVGLength(0, SkSVGLength::Unit::kPercentage); using INHERITED = SkSVGHiddenContainer; }; diff --git a/modules/svg/src/SkSVGAttribute.cpp b/modules/svg/src/SkSVGAttribute.cpp index 6a9129d3fd..d07aa91ea2 100644 --- a/modules/svg/src/SkSVGAttribute.cpp +++ b/modules/svg/src/SkSVGAttribute.cpp @@ -34,5 +34,8 @@ SkSVGPresentationAttributes SkSVGPresentationAttributes::MakeInitial() { result.fFontWeight.init(SkSVGFontWeight::Type::kNormal); result.fTextAnchor.init(SkSVGTextAnchor::Type::kStart); + result.fStopColor.set(SkSVGColor(SK_ColorBLACK)); + result.fStopOpacity.set(SkSVGNumberType(1)); + return result; } diff --git a/modules/svg/src/SkSVGAttributeParser.cpp b/modules/svg/src/SkSVGAttributeParser.cpp index d34f153afb..5bfea3b9d3 100644 --- a/modules/svg/src/SkSVGAttributeParser.cpp +++ b/modules/svg/src/SkSVGAttributeParser.cpp @@ -604,23 +604,6 @@ bool SkSVGAttributeParser::parse(SkSVGLineJoin* join) { return parsedValue && this->parseEOSToken(); } -// https://www.w3.org/TR/SVG11/pservers.html#StopElement -bool SkSVGAttributeParser::parseStopColor(SkSVGStopColor* stopColor) { - SkSVGColorType c; - bool parsedValue = false; - if (this->parse(&c)) { - *stopColor = SkSVGStopColor(c); - parsedValue = true; - } else if (this->parseExpectedStringToken("currentColor")) { - *stopColor = SkSVGStopColor(SkSVGStopColor::Type::kCurrentColor); - parsedValue = true; - } else if (this->parseExpectedStringToken("inherit")) { - *stopColor = SkSVGStopColor(SkSVGStopColor::Type::kInherit); - parsedValue = true; - } - return parsedValue && this->parseEOSToken(); -} - // https://www.w3.org/TR/SVG11/coords.html#ObjectBoundingBoxUnits template <> bool SkSVGAttributeParser::parse(SkSVGObjectBoundingBoxUnits* objectBoundingBoxUnits) { diff --git a/modules/svg/src/SkSVGDOM.cpp b/modules/svg/src/SkSVGDOM.cpp index 356de27e0e..643230ee08 100644 --- a/modules/svg/src/SkSVGDOM.cpp +++ b/modules/svg/src/SkSVGDOM.cpp @@ -93,17 +93,6 @@ bool SetLengthAttribute(const sk_sp& node, SkSVGAttribute attr, return true; } -bool SetNumberAttribute(const sk_sp& node, SkSVGAttribute attr, - const char* stringValue) { - auto parseResult = SkSVGAttributeParser::parse(stringValue); - if (!parseResult.isValid()) { - return false; - } - - node->setAttribute(attr, SkSVGNumberValue(*parseResult)); - return true; -} - bool SetViewBoxAttribute(const sk_sp& node, SkSVGAttribute attr, const char* stringValue) { SkSVGViewBoxType viewBox; @@ -116,18 +105,6 @@ bool SetViewBoxAttribute(const sk_sp& node, SkSVGAttribute attr, return true; } -bool SetStopColorAttribute(const sk_sp& node, SkSVGAttribute attr, - const char* stringValue) { - SkSVGStopColor stopColor; - SkSVGAttributeParser parser(stringValue); - if (!parser.parseStopColor(&stopColor)) { - return false; - } - - node->setAttribute(attr, SkSVGStopColorValue(stopColor)); - return true; -} - bool SetObjectBoundingBoxUnitsAttribute(const sk_sp& node, SkSVGAttribute attr, const char* stringValue) { @@ -259,8 +236,6 @@ SortedDictionaryEntry gAttributeParseInfo[] = { { "r" , { SkSVGAttribute::kR , SetLengthAttribute }}, { "rx" , { SkSVGAttribute::kRx , SetLengthAttribute }}, { "ry" , { SkSVGAttribute::kRy , SetLengthAttribute }}, - { "stop-color" , { SkSVGAttribute::kStopColor , SetStopColorAttribute }}, - { "stop-opacity" , { SkSVGAttribute::kStopOpacity , SetNumberAttribute }}, { "style" , { SkSVGAttribute::kUnknown , SetStyleAttributes }}, { "text" , { SkSVGAttribute::kText , SetStringAttribute }}, { "transform" , { SkSVGAttribute::kTransform , SetTransformAttribute }}, diff --git a/modules/svg/src/SkSVGGradient.cpp b/modules/svg/src/SkSVGGradient.cpp index 2448d5bf72..cc5b297e08 100644 --- a/modules/svg/src/SkSVGGradient.cpp +++ b/modules/svg/src/SkSVGGradient.cpp @@ -53,25 +53,28 @@ void SkSVGGradient::collectColorStops(const SkSVGRenderContext& ctx, SkColor SkSVGGradient::resolveStopColor(const SkSVGRenderContext& ctx, const SkSVGStop& stop) const { - const SkSVGStopColor& stopColor = stop.stopColor(); + const auto& stopColor = stop.getStopColor(); + const auto& stopOpacity = stop.getStopOpacity(); + // Uninherited presentation attrs should have a concrete value at this point. + if (!stopColor.isValue() || !stopOpacity.isValue()) { + SkDebugf("unhandled: stop-color or stop-opacity has no value\n"); + return SK_ColorBLACK; + } + SkColor color; - switch (stopColor.type()) { - case SkSVGStopColor::Type::kColor: - color = stopColor.color(); + switch (stopColor->type()) { + case SkSVGColor::Type::kColor: + color = stopColor->color(); break; - case SkSVGStopColor::Type::kCurrentColor: + case SkSVGColor::Type::kCurrentColor: color = *ctx.presentationContext().fInherited.fColor; break; - case SkSVGStopColor::Type::kICCColor: + case SkSVGColor::Type::kICCColor: SkDebugf("unimplemented 'icccolor' stop-color type\n"); color = SK_ColorBLACK; break; - case SkSVGStopColor::Type::kInherit: - SkDebugf("unimplemented 'inherit' stop-color type\n"); - color = SK_ColorBLACK; - break; } - return SkColorSetA(color, SkScalarRoundToInt(stop.stopOpacity() * 255)); + return SkColorSetA(color, SkScalarRoundToInt(*stopOpacity * 255)); } bool SkSVGGradient::onAsPaint(const SkSVGRenderContext& ctx, SkPaint* paint) const { diff --git a/modules/svg/src/SkSVGNode.cpp b/modules/svg/src/SkSVGNode.cpp index a8f39ad364..acf8f8f49d 100644 --- a/modules/svg/src/SkSVGNode.cpp +++ b/modules/svg/src/SkSVGNode.cpp @@ -14,7 +14,11 @@ #include "modules/svg/include/SkSVGValue.h" #include "src/core/SkTLazy.h" -SkSVGNode::SkSVGNode(SkSVGTag t) : fTag(t) { } +SkSVGNode::SkSVGNode(SkSVGTag t) : fTag(t) { + // Uninherited presentation attributes need a non-null default value. + fPresentationAttributes.fStopColor.set(SkSVGColor(SK_ColorBLACK)); + fPresentationAttributes.fStopOpacity.set(SkSVGNumberType(1.0f)); +} SkSVGNode::~SkSVGNode() { } @@ -94,6 +98,8 @@ bool SkSVGNode::parseAndSetAttribute(const char* n, const char* v) { || PARSE_AND_SET("font-style" , FontStyle) || PARSE_AND_SET("font-weight" , FontWeight) || PARSE_AND_SET("opacity" , Opacity) + || PARSE_AND_SET("stop-color" , StopColor) + || PARSE_AND_SET("stop-opacity" , StopOpacity) || PARSE_AND_SET("stroke" , Stroke) || PARSE_AND_SET("stroke-dasharray" , StrokeDashArray) || PARSE_AND_SET("stroke-dashoffset", StrokeDashOffset) diff --git a/modules/svg/src/SkSVGRenderContext.cpp b/modules/svg/src/SkSVGRenderContext.cpp index 7f7be49e79..6925a4dc0e 100644 --- a/modules/svg/src/SkSVGRenderContext.cpp +++ b/modules/svg/src/SkSVGRenderContext.cpp @@ -455,6 +455,12 @@ void SkSVGRenderContext::applyPresentationAttributes(const SkSVGPresentationAttr if (attrs.fFilter.isValue()) { this->applyFilter(*attrs.fFilter); } + + // Remaining uninherited presentation attributes are accessed as SkSVGNode fields, not via + // the render context. + // TODO: resolve these in a pre-render styling pass and assert here that they are values. + // - stop-color + // - stop-opacity } void SkSVGRenderContext::applyOpacity(SkScalar opacity, uint32_t flags) { diff --git a/modules/svg/src/SkSVGStop.cpp b/modules/svg/src/SkSVGStop.cpp index edf39bc7ea..03b80cc8c7 100644 --- a/modules/svg/src/SkSVGStop.cpp +++ b/modules/svg/src/SkSVGStop.cpp @@ -16,14 +16,6 @@ void SkSVGStop::setOffset(const SkSVGLength& offset) { fOffset = offset; } -void SkSVGStop::setStopColor(const SkSVGStopColor& color) { - fStopColor = color; -} - -void SkSVGStop::setStopOpacity(const SkSVGNumberType& opacity) { - fStopOpacity = SkTPin(opacity, 0, 1); -} - void SkSVGStop::onSetAttribute(SkSVGAttribute attr, const SkSVGValue& v) { switch (attr) { case SkSVGAttribute::kOffset: @@ -31,16 +23,6 @@ void SkSVGStop::onSetAttribute(SkSVGAttribute attr, const SkSVGValue& v) { this->setOffset(*offset); } break; - case SkSVGAttribute::kStopColor: - if (const auto* color = v.as()) { - this->setStopColor(*color); - } - break; - case SkSVGAttribute::kStopOpacity: - if (const auto* opacity = v.as()) { - this->setStopOpacity(*opacity); - } - break; default: this->INHERITED::onSetAttribute(attr, v); }