Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 75c38f9

Browse files
tdennistonSkia Commit-Bot
tdenniston
authored and
Skia Commit-Bot
committed
[svg] Add SkSVGProperty class for presentation attributes
This CL adds a new SkSVGProperty<T,B> class and uses it instead of SkTLazy for the presentation attributes. Ideally this will form the foundation for improvements to our presentation attribute parsing as well as correctness for inherited/non-inherited properties. Change-Id: Ie1cdb3db9674c55376e127cc1a8b8cb303a1bd13 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/334837 Commit-Queue: Tyler Denniston <tdenniston@google.com> Reviewed-by: Florin Malita <fmalita@chromium.org>
1 parent 98e17bf commit 75c38f9

File tree

4 files changed

+106
-46
lines changed

4 files changed

+106
-46
lines changed

‎modules/svg/include/SkSVGAttribute.h‎

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -72,38 +72,38 @@ enum class SkSVGAttribute {
7272
struct SkSVGPresentationAttributes {
7373
static SkSVGPresentationAttributes MakeInitial();
7474

75-
// TODO: SkTLazy adds an extra ptr per attribute; refactor to reduce overhead.
75+
// TODO: SkSVGProperty adds an extra ptr per attribute; refactor to reduce overhead.
7676

77-
SkTLazy<SkSVGPaint> fFill;
78-
SkTLazy<SkSVGNumberType> fFillOpacity;
79-
SkTLazy<SkSVGFillRule> fFillRule;
80-
SkTLazy<SkSVGFillRule> fClipRule;
77+
SkSVGProperty<SkSVGPaint , true> fFill;
78+
SkSVGProperty<SkSVGNumberType, true> fFillOpacity;
79+
SkSVGProperty<SkSVGFillRule , true> fFillRule;
80+
SkSVGProperty<SkSVGFillRule , true> fClipRule;
8181

82-
SkTLazy<SkSVGPaint> fStroke;
83-
SkTLazy<SkSVGDashArray> fStrokeDashArray;
84-
SkTLazy<SkSVGLength> fStrokeDashOffset;
85-
SkTLazy<SkSVGLineCap> fStrokeLineCap;
86-
SkTLazy<SkSVGLineJoin> fStrokeLineJoin;
87-
SkTLazy<SkSVGNumberType> fStrokeMiterLimit;
88-
SkTLazy<SkSVGNumberType> fStrokeOpacity;
89-
SkTLazy<SkSVGLength> fStrokeWidth;
82+
SkSVGProperty<SkSVGPaint , true> fStroke;
83+
SkSVGProperty<SkSVGDashArray , true> fStrokeDashArray;
84+
SkSVGProperty<SkSVGLength , true> fStrokeDashOffset;
85+
SkSVGProperty<SkSVGLineCap , true> fStrokeLineCap;
86+
SkSVGProperty<SkSVGLineJoin , true> fStrokeLineJoin;
87+
SkSVGProperty<SkSVGNumberType, true> fStrokeMiterLimit;
88+
SkSVGProperty<SkSVGNumberType, true> fStrokeOpacity;
89+
SkSVGProperty<SkSVGLength , true> fStrokeWidth;
9090

91-
SkTLazy<SkSVGVisibility> fVisibility;
91+
SkSVGProperty<SkSVGVisibility, true> fVisibility;
9292

93-
SkTLazy<SkSVGColorType> fColor;
93+
SkSVGProperty<SkSVGColorType , true> fColor;
9494

95-
SkTLazy<SkSVGFontFamily> fFontFamily;
96-
SkTLazy<SkSVGFontStyle> fFontStyle;
97-
SkTLazy<SkSVGFontSize> fFontSize;
98-
SkTLazy<SkSVGFontWeight> fFontWeight;
99-
SkTLazy<SkSVGTextAnchor> fTextAnchor;
95+
SkSVGProperty<SkSVGFontFamily, true> fFontFamily;
96+
SkSVGProperty<SkSVGFontStyle , true> fFontStyle;
97+
SkSVGProperty<SkSVGFontSize , true> fFontSize;
98+
SkSVGProperty<SkSVGFontWeight, true> fFontWeight;
99+
SkSVGProperty<SkSVGTextAnchor, true> fTextAnchor;
100100

101101
// TODO(tdenniston): add SkSVGStopColor
102102

103103
// uninherited
104-
SkTLazy<SkSVGNumberType> fOpacity;
105-
SkTLazy<SkSVGClip> fClipPath;
106-
SkTLazy<SkSVGFilterType> fFilter;
104+
SkSVGProperty<SkSVGNumberType, false> fOpacity;
105+
SkSVGProperty<SkSVGClip , false> fClipPath;
106+
SkSVGProperty<SkSVGFilterType, false> fFilter;
107107
};
108108

109109
#endif // SkSVGAttribute_DEFINED

‎modules/svg/include/SkSVGNode.h‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -55,21 +55,21 @@ public: \
5555
return fPresentationAttributes.f##attr_name.getMaybeNull(); \
5656
} \
5757
void set##attr_name(const attr_type& v) { \
58-
if (!attr_inherited || v.type() != attr_type::Type::kInherit) { \
59-
fPresentationAttributes.f##attr_name.set(v); \
58+
auto* dest = &fPresentationAttributes.f##attr_name; \
59+
if (!dest->isInheritable() || \
60+
v.type() != attr_type::Type::kInherit) { \
61+
dest->set(v); \
6062
} else { \
61-
/* kInherited values are semantically equivalent to \
62-
the absence of a local presentation attribute.*/ \
63-
fPresentationAttributes.f##attr_name.reset(); \
63+
dest->set(SkSVGPropertyState::kInherit); \
6464
} \
6565
} \
6666
void set##attr_name(attr_type&& v) { \
67-
if (!attr_inherited || v.type() != attr_type::Type::kInherit) { \
68-
fPresentationAttributes.f##attr_name.set(std::move(v)); \
67+
auto* dest = &fPresentationAttributes.f##attr_name; \
68+
if (!dest->isInheritable() || \
69+
v.type() != attr_type::Type::kInherit) { \
70+
dest->set(std::move(v)); \
6971
} else { \
70-
/* kInherited values are semantically equivalent to \
71-
the absence of a local presentation attribute.*/ \
72-
fPresentationAttributes.f##attr_name.reset(); \
72+
dest->set(SkSVGPropertyState::kInherit); \
7373
} \
7474
}
7575

‎modules/svg/include/SkSVGTypes.h‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include "include/core/SkString.h"
1818
#include "include/core/SkTypes.h"
1919
#include "include/private/SkTDArray.h"
20+
#include "src/core/SkTLazy.h"
2021

2122
using SkSVGColorType = SkColor;
2223
using SkSVGIntegerType = int;
@@ -26,6 +27,65 @@ using SkSVGViewBoxType = SkRect;
2627
using SkSVGTransformType = SkMatrix;
2728
using SkSVGPointsType = SkTDArray<SkPoint>;
2829

30+
enum class SkSVGPropertyState {
31+
kUnspecified,
32+
kInherit,
33+
kValue,
34+
};
35+
36+
// https://www.w3.org/TR/SVG11/intro.html#TermProperty
37+
template <typename T, bool kInheritable> class SkSVGProperty {
38+
public:
39+
SkSVGProperty() : fState(SkSVGPropertyState::kUnspecified) {}
40+
41+
template <typename... Args>
42+
void init(Args&&... args) {
43+
fState = SkSVGPropertyState::kValue;
44+
fValue.init(std::forward<Args>(args)...);
45+
}
46+
47+
constexpr bool isInheritable() const { return kInheritable; }
48+
49+
bool isValue() const { return fState == SkSVGPropertyState::kValue; }
50+
51+
T* getMaybeNull() const {
52+
return fValue.getMaybeNull();
53+
}
54+
55+
void set(SkSVGPropertyState state) {
56+
fState = state;
57+
if (fState != SkSVGPropertyState::kValue) {
58+
fValue.reset();
59+
}
60+
}
61+
62+
void set(const T& value) {
63+
fState = SkSVGPropertyState::kValue;
64+
fValue.set(value);
65+
}
66+
67+
void set(T&& value) {
68+
fState = SkSVGPropertyState::kValue;
69+
fValue.set(std::move(value));
70+
}
71+
72+
T* operator->() const {
73+
SkASSERT(fState == SkSVGPropertyState::kValue);
74+
SkASSERT(fValue.isValid());
75+
return fValue.get();
76+
}
77+
78+
T& operator*() const {
79+
SkASSERT(fState == SkSVGPropertyState::kValue);
80+
SkASSERT(fValue.isValid());
81+
return *fValue;
82+
}
83+
84+
private:
85+
SkSVGPropertyState fState;
86+
SkTLazy<T> fValue;
87+
};
88+
2989
class SkSVGLength {
3090
public:
3191
enum class Unit {

‎modules/svg/src/SkSVGRenderContext.cpp‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ template <>
168168
void commitToPaint<SkSVGAttribute::kStrokeDashArray>(const SkSVGPresentationAttributes& attrs,
169169
const SkSVGRenderContext& ctx,
170170
SkSVGPresentationContext* pctx) {
171-
const auto& dashArray = attrs.fStrokeDashArray.get();
171+
const auto& dashArray = attrs.fStrokeDashArray.getMaybeNull();
172172
SkASSERT(dashArray->type() != SkSVGDashArray::Type::kInherit);
173173

174174
if (dashArray->type() != SkSVGDashArray::Type::kDashArray) {
@@ -391,13 +391,13 @@ void SkSVGRenderContext::applyPresentationAttributes(const SkSVGPresentationAttr
391391
#define ApplyLazyInheritedAttribute(ATTR) \
392392
do { \
393393
/* All attributes should be defined on the inherited context. */ \
394-
SkASSERT(fPresentationContext->fInherited.f ## ATTR.isValid()); \
395-
const auto* value = attrs.f ## ATTR.getMaybeNull(); \
396-
if (value && *value != *fPresentationContext->fInherited.f ## ATTR.get()) { \
394+
SkASSERT(fPresentationContext->fInherited.f ## ATTR.isValue()); \
395+
const auto& attr = attrs.f ## ATTR; \
396+
if (attr.isValue() && *attr != *fPresentationContext->fInherited.f ## ATTR) { \
397397
/* Update the local attribute value */ \
398-
fPresentationContext.writable()->fInherited.f ## ATTR.set(*value); \
398+
fPresentationContext.writable()->fInherited.f ## ATTR.set(*attr); \
399399
/* Update the cached paints */ \
400-
commitToPaint<SkSVGAttribute::k ## ATTR>(attrs, *this, \
400+
commitToPaint<SkSVGAttribute::k ## ATTR>(attrs, *this, \
401401
fPresentationContext.writable()); \
402402
} \
403403
} while (false)
@@ -423,25 +423,25 @@ void SkSVGRenderContext::applyPresentationAttributes(const SkSVGPresentationAttr
423423
ApplyLazyInheritedAttribute(Color);
424424

425425
// Local 'color' attribute: update paints for attributes that are set to 'currentColor'.
426-
if (attrs.fColor.isValid()) {
426+
if (attrs.fColor.isValue()) {
427427
updatePaintsWithCurrentColor(attrs);
428428
}
429429

430430
#undef ApplyLazyInheritedAttribute
431431

432432
// Uninherited attributes. Only apply to the current context.
433433

434-
if (auto* opacity = attrs.fOpacity.getMaybeNull()) {
435-
this->applyOpacity(*opacity, flags);
434+
if (attrs.fOpacity.isValue()) {
435+
this->applyOpacity(*attrs.fOpacity, flags);
436436
}
437437

438-
if (auto* clip = attrs.fClipPath.getMaybeNull()) {
439-
this->applyClip(*clip);
438+
if (attrs.fClipPath.isValue()) {
439+
this->applyClip(*attrs.fClipPath);
440440
}
441441

442442
// TODO: when both a filter and opacity are present, we can apply both with a single layer
443-
if (auto* filter = attrs.fFilter.getMaybeNull()) {
444-
this->applyFilter(*filter);
443+
if (attrs.fFilter.isValue()) {
444+
this->applyFilter(*attrs.fFilter);
445445
}
446446
}
447447

0 commit comments

Comments
 (0)