- Revision
- 268704
- Author
- wenson_hs...@apple.com
- Date
- 2020-10-19 17:46:01 -0700 (Mon, 19 Oct 2020)
Log Message
[MotionMark] Add state change items to represent changes to stroke and fill state
https://bugs.webkit.org/show_bug.cgi?id=217911
Reviewed by Simon Fraser.
Source/WebCore:
Add two new display list items to represent changes to stroke thickness and/or stroke color, and fill color.
These state changes account for almost all of the state changes in MotionMark's canvas rendering subtests (i.e.
lines, arcs, and paths).
This significantly reduces the memory overhead caused by maintaining display list items in the web and GPU
processes when running Canvas Lines and Canvas Arcs. Currently, each `SetState` item is currently 120 bytes;
after this change, each state change item will decrease to either 24 (for `SetFillColor`) or 32 (for
`SetStrokeState`) bytes.
No change in behavior.
* platform/graphics/displaylists/DisplayList.h:
(WebCore::DisplayList::Item::isStateItem const): Deleted.
(WebCore::DisplayList::Item::isStateItemType): Deleted.
Remove a couple of unused methods, as well as an unused constexpr.
* platform/graphics/displaylists/DisplayListItems.cpp:
(WebCore::DisplayList::Item::sizeInBytes):
(WebCore::DisplayList::SetFillColor::create):
(WebCore::DisplayList::SetFillColor::apply const):
(WebCore::DisplayList::operator<<):
Add the `SetFillColor` item, which represents a call to `GraphicsContext::setFillColor`.
(WebCore::DisplayList::SetStrokeState::create):
(WebCore::DisplayList::SetStrokeState::apply const):
Add the `SetStrokeState` item, which represents a call to `GraphicsContext::setStrokeThickness` and/or
`GraphicsContext::setStrokeColor`.
(WebCore::DisplayList::SetState::accumulate): Deleted.
Remove an unused method.
* platform/graphics/displaylists/DisplayListItems.h:
(WebCore::DisplayList::SetFillColor::color const):
(WebCore::DisplayList::SetFillColor::SetFillColor):
(WebCore::DisplayList::SetFillColor::encode const):
(WebCore::DisplayList::SetFillColor::decode):
(WebCore::DisplayList::SetStrokeState::color const):
(WebCore::DisplayList::SetStrokeState::hasColor const):
(WebCore::DisplayList::SetStrokeState::thickness const):
(WebCore::DisplayList::SetStrokeState::hasThickness const):
Instead of using two `Optional<T>` members, store two `bool` flags corresponding to thickness and stroke color.
This allows us to make the `SetStrokeState` item 32 bytes instead of 40 bytes, due to the extra padding that
would otherwise exist after each of the `Optional` members.
(WebCore::DisplayList::SetStrokeState::SetStrokeState):
(WebCore::DisplayList::SetStrokeState::encode const):
(WebCore::DisplayList::SetStrokeState::decode):
(WebCore::DisplayList::Item::encode const):
(WebCore::DisplayList::Item::decode):
* platform/graphics/displaylists/DisplayListRecorder.cpp:
(WebCore::DisplayList::containsOnlyStrokeColorOrThicknessChange):
(WebCore::DisplayList::containsOnlyFillColorChange):
(WebCore::DisplayList::createStateChangeItem):
Instead of always appending `SetState` items, append either `SetStrokeState` or `SetFillColor` if the only
changed flags are stroke thickness or stroke color, or fill color (respectively).
(WebCore::DisplayList::Recorder::willAppendItem):
LayoutTests:
* displaylists/canvas-display-list-expected.txt: Rebaseline an existing display list item dump test.
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (268703 => 268704)
--- trunk/LayoutTests/ChangeLog 2020-10-20 00:41:24 UTC (rev 268703)
+++ trunk/LayoutTests/ChangeLog 2020-10-20 00:46:01 UTC (rev 268704)
@@ -1,3 +1,12 @@
+2020-10-19 Wenson Hsieh <wenson_hs...@apple.com>
+
+ [MotionMark] Add state change items to represent changes to stroke and fill state
+ https://bugs.webkit.org/show_bug.cgi?id=217911
+
+ Reviewed by Simon Fraser.
+
+ * displaylists/canvas-display-list-expected.txt: Rebaseline an existing display list item dump test.
+
2020-10-19 Ryan Haddad <ryanhad...@apple.com>
[mac-wk1] inspector/debugger/breakpoint-resolve-when-script-added.html is a flaky timeout
Modified: trunk/LayoutTests/displaylists/canvas-display-list-expected.txt (268703 => 268704)
--- trunk/LayoutTests/displaylists/canvas-display-list-expected.txt 2020-10-20 00:41:24 UTC (rev 268703)
+++ trunk/LayoutTests/displaylists/canvas-display-list-expected.txt 2020-10-20 00:46:01 UTC (rev 268704)
@@ -8,9 +8,8 @@
(fill-rect
(extent at (10,140) size 55x50)
(rect at (10,10) size 55x50))
-(set-state
- (change-flags 128)
- (fill-color #0000C880))
+(set-fill-color
+ (color #0000C880))
(fill-rect
(extent at (30,120) size 55x50)
(rect at (30,30) size 55x50))
Modified: trunk/Source/WebCore/ChangeLog (268703 => 268704)
--- trunk/Source/WebCore/ChangeLog 2020-10-20 00:41:24 UTC (rev 268703)
+++ trunk/Source/WebCore/ChangeLog 2020-10-20 00:46:01 UTC (rev 268704)
@@ -1,3 +1,74 @@
+2020-10-19 Wenson Hsieh <wenson_hs...@apple.com>
+
+ [MotionMark] Add state change items to represent changes to stroke and fill state
+ https://bugs.webkit.org/show_bug.cgi?id=217911
+
+ Reviewed by Simon Fraser.
+
+ Add two new display list items to represent changes to stroke thickness and/or stroke color, and fill color.
+ These state changes account for almost all of the state changes in MotionMark's canvas rendering subtests (i.e.
+ lines, arcs, and paths).
+
+ This significantly reduces the memory overhead caused by maintaining display list items in the web and GPU
+ processes when running Canvas Lines and Canvas Arcs. Currently, each `SetState` item is currently 120 bytes;
+ after this change, each state change item will decrease to either 24 (for `SetFillColor`) or 32 (for
+ `SetStrokeState`) bytes.
+
+ No change in behavior.
+
+ * platform/graphics/displaylists/DisplayList.h:
+ (WebCore::DisplayList::Item::isStateItem const): Deleted.
+ (WebCore::DisplayList::Item::isStateItemType): Deleted.
+
+ Remove a couple of unused methods, as well as an unused constexpr.
+
+ * platform/graphics/displaylists/DisplayListItems.cpp:
+ (WebCore::DisplayList::Item::sizeInBytes):
+ (WebCore::DisplayList::SetFillColor::create):
+ (WebCore::DisplayList::SetFillColor::apply const):
+ (WebCore::DisplayList::operator<<):
+
+ Add the `SetFillColor` item, which represents a call to `GraphicsContext::setFillColor`.
+
+ (WebCore::DisplayList::SetStrokeState::create):
+ (WebCore::DisplayList::SetStrokeState::apply const):
+
+ Add the `SetStrokeState` item, which represents a call to `GraphicsContext::setStrokeThickness` and/or
+ `GraphicsContext::setStrokeColor`.
+
+ (WebCore::DisplayList::SetState::accumulate): Deleted.
+
+ Remove an unused method.
+
+ * platform/graphics/displaylists/DisplayListItems.h:
+ (WebCore::DisplayList::SetFillColor::color const):
+ (WebCore::DisplayList::SetFillColor::SetFillColor):
+ (WebCore::DisplayList::SetFillColor::encode const):
+ (WebCore::DisplayList::SetFillColor::decode):
+ (WebCore::DisplayList::SetStrokeState::color const):
+ (WebCore::DisplayList::SetStrokeState::hasColor const):
+ (WebCore::DisplayList::SetStrokeState::thickness const):
+ (WebCore::DisplayList::SetStrokeState::hasThickness const):
+
+ Instead of using two `Optional<T>` members, store two `bool` flags corresponding to thickness and stroke color.
+ This allows us to make the `SetStrokeState` item 32 bytes instead of 40 bytes, due to the extra padding that
+ would otherwise exist after each of the `Optional` members.
+
+ (WebCore::DisplayList::SetStrokeState::SetStrokeState):
+ (WebCore::DisplayList::SetStrokeState::encode const):
+ (WebCore::DisplayList::SetStrokeState::decode):
+ (WebCore::DisplayList::Item::encode const):
+ (WebCore::DisplayList::Item::decode):
+ * platform/graphics/displaylists/DisplayListRecorder.cpp:
+ (WebCore::DisplayList::containsOnlyStrokeColorOrThicknessChange):
+ (WebCore::DisplayList::containsOnlyFillColorChange):
+ (WebCore::DisplayList::createStateChangeItem):
+
+ Instead of always appending `SetState` items, append either `SetStrokeState` or `SetFillColor` if the only
+ changed flags are stroke thickness or stroke color, or fill color (respectively).
+
+ (WebCore::DisplayList::Recorder::willAppendItem):
+
2020-10-19 Simon Fraser <simon.fra...@apple.com>
Fix possible crash in GraphicsLayerCA::computeVisibleAndCoverageRect()
Modified: trunk/Source/WebCore/platform/graphics/displaylists/DisplayList.h (268703 => 268704)
--- trunk/Source/WebCore/platform/graphics/displaylists/DisplayList.h 2020-10-20 00:41:24 UTC (rev 268703)
+++ trunk/Source/WebCore/platform/graphics/displaylists/DisplayList.h 2020-10-20 00:46:01 UTC (rev 268704)
@@ -47,6 +47,8 @@
Scale,
ConcatenateCTM,
SetCTM,
+ SetFillColor,
+ SetStrokeState,
SetState,
SetLineCap,
SetLineDash,
@@ -109,37 +111,8 @@
virtual void apply(GraphicsContext&) const = 0;
- static constexpr bool isDisplayListItem = true;
-
virtual bool isDrawingItem() const { return false; }
- // A state item is one preserved by Save/Restore.
- bool isStateItem() const
- {
- return isStateItemType(m_type);
- }
-
- static bool isStateItemType(ItemType itemType)
- {
- switch (itemType) {
- case ItemType::Translate:
- case ItemType::Rotate:
- case ItemType::Scale:
- case ItemType::ConcatenateCTM:
- case ItemType::SetCTM:
- case ItemType::SetState:
- case ItemType::SetLineCap:
- case ItemType::SetLineDash:
- case ItemType::SetLineJoin:
- case ItemType::SetMiterLimit:
- case ItemType::ClearShadow:
- return true;
- default:
- return false;
- }
- return false;
- }
-
#if !defined(NDEBUG) || !LOG_DISABLED
WTF::CString description() const;
#endif
Modified: trunk/Source/WebCore/platform/graphics/displaylists/DisplayListItems.cpp (268703 => 268704)
--- trunk/Source/WebCore/platform/graphics/displaylists/DisplayListItems.cpp 2020-10-20 00:41:24 UTC (rev 268703)
+++ trunk/Source/WebCore/platform/graphics/displaylists/DisplayListItems.cpp 2020-10-20 00:46:01 UTC (rev 268704)
@@ -76,6 +76,10 @@
return sizeof(downcast<SetCTM>(item));
case ItemType::ConcatenateCTM:
return sizeof(downcast<ConcatenateCTM>(item));
+ case ItemType::SetFillColor:
+ return sizeof(downcast<SetFillColor>(item));
+ case ItemType::SetStrokeState:
+ return sizeof(downcast<SetStrokeState>(item));
case ItemType::SetState:
return sizeof(downcast<SetState>(item));
case ItemType::SetLineCap:
@@ -316,6 +320,49 @@
return ts;
}
+Ref<SetFillColor> SetFillColor::create(Color color)
+{
+ return adoptRef(*new SetFillColor(color));
+}
+
+SetFillColor::~SetFillColor() = default;
+
+void SetFillColor::apply(GraphicsContext& context) const
+{
+ context.setFillColor(m_color);
+}
+
+static TextStream& operator<<(TextStream& ts, const SetFillColor& state)
+{
+ ts.dumpProperty("color", state.color());
+ return ts;
+}
+
+Ref<SetStrokeState> SetStrokeState::create(Optional<Color>&& color, Optional<float>&& thickness)
+{
+ return adoptRef(*new SetStrokeState(WTFMove(color), WTFMove(thickness)));
+}
+
+SetStrokeState::~SetStrokeState() = default;
+
+void SetStrokeState::apply(GraphicsContext& context) const
+{
+ if (m_hasColor)
+ context.setStrokeColor(m_color);
+
+ if (m_hasThickness)
+ context.setStrokeThickness(m_thickness);
+}
+
+static TextStream& operator<<(TextStream& ts, const SetStrokeState& state)
+{
+ if (state.hasColor())
+ ts.dumpProperty("color", state.color());
+ if (state.hasThickness())
+ ts.dumpProperty("thickness", state.thickness());
+ return ts;
+}
+
SetState::SetState(const GraphicsContextState& state, GraphicsContextState::StateChangeFlags flags)
: Item(ItemType::SetState)
, m_state(state, flags)
@@ -335,11 +382,6 @@
m_state.apply(context);
}
-void SetState::accumulate(const GraphicsContextState& state, GraphicsContextState::StateChangeFlags flags)
-{
- m_state.accumulate(state, flags);
-}
-
static TextStream& operator<<(TextStream& ts, const SetState& state)
{
ts << state.state();
@@ -1438,6 +1480,8 @@
case ItemType::Scale: ts << "scale"; break;
case ItemType::SetCTM: ts << "set-ctm"; break;
case ItemType::ConcatenateCTM: ts << "concatentate-ctm"; break;
+ case ItemType::SetFillColor: ts << "set-fill-color"; break;
+ case ItemType::SetStrokeState: ts << "set-stroke-state"; break;
case ItemType::SetState: ts << "set-state"; break;
case ItemType::SetLineCap: ts << "set-line-cap"; break;
case ItemType::SetLineDash: ts << "set-line-dash"; break;
@@ -1510,6 +1554,12 @@
case ItemType::ConcatenateCTM:
ts << downcast<ConcatenateCTM>(item);
break;
+ case ItemType::SetFillColor:
+ ts << downcast<SetFillColor>(item);
+ break;
+ case ItemType::SetStrokeState:
+ ts << downcast<SetStrokeState>(item);
+ break;
case ItemType::SetState:
ts << downcast<SetState>(item);
break;
Modified: trunk/Source/WebCore/platform/graphics/displaylists/DisplayListItems.h (268703 => 268704)
--- trunk/Source/WebCore/platform/graphics/displaylists/DisplayListItems.h 2020-10-20 00:41:24 UTC (rev 268703)
+++ trunk/Source/WebCore/platform/graphics/displaylists/DisplayListItems.h 2020-10-20 00:46:01 UTC (rev 268704)
@@ -336,6 +336,121 @@
return ConcatenateCTM::create(*transform);
}
+class SetFillColor : public Item {
+public:
+ WEBCORE_EXPORT static Ref<SetFillColor> create(Color);
+
+ WEBCORE_EXPORT virtual ~SetFillColor();
+
+ Color color() const { return m_color; }
+
+ template<class Encoder> void encode(Encoder&) const;
+ template<class Decoder> static Optional<Ref<SetFillColor>> decode(Decoder&);
+
+private:
+ SetFillColor(Color color)
+ : Item(ItemType::SetFillColor)
+ , m_color(color)
+ {
+ }
+
+ void apply(GraphicsContext&) const override;
+
+ Color m_color;
+};
+
+template<class Encoder>
+void SetFillColor::encode(Encoder& encoder) const
+{
+ encoder << m_color;
+}
+
+template<class Decoder>
+Optional<Ref<SetFillColor>> SetFillColor::decode(Decoder& decoder)
+{
+ Optional<Color> color;
+ decoder >> color;
+ if (!color)
+ return WTF::nullopt;
+
+ return SetFillColor::create(*color);
+}
+
+class SetStrokeState : public Item {
+public:
+ WEBCORE_EXPORT static Ref<SetStrokeState> create(Optional<Color>&&, Optional<float>&& thickness);
+
+ WEBCORE_EXPORT virtual ~SetStrokeState();
+
+ Color color() const { return m_color; }
+ bool hasColor() const { return m_hasColor; }
+
+ float thickness() const { return m_thickness; }
+ bool hasThickness() const { return m_hasThickness; }
+
+ template<class Encoder> void encode(Encoder&) const;
+ template<class Decoder> static Optional<Ref<SetStrokeState>> decode(Decoder&);
+
+private:
+ SetStrokeState(Optional<Color>&& color, Optional<float>&& thickness)
+ : Item(ItemType::SetStrokeState)
+ , m_color(color.valueOr(Color()))
+ , m_thickness(thickness.valueOr(0))
+ , m_hasColor(color.hasValue())
+ , m_hasThickness(thickness.hasValue())
+ {
+ }
+
+ void apply(GraphicsContext&) const override;
+
+ Color m_color;
+ float m_thickness { 0 };
+ bool m_hasColor { false };
+ bool m_hasThickness { false };
+};
+
+template<class Encoder>
+void SetStrokeState::encode(Encoder& encoder) const
+{
+ encoder << m_hasColor;
+ if (m_hasColor)
+ encoder << m_color;
+
+ encoder << m_hasThickness;
+ if (m_hasThickness)
+ encoder << m_thickness;
+}
+
+template<class Decoder>
+Optional<Ref<SetStrokeState>> SetStrokeState::decode(Decoder& decoder)
+{
+ Optional<bool> hasColor;
+ decoder >> hasColor;
+ if (!hasColor)
+ return WTF::nullopt;
+
+ Optional<Color> color;
+ if (*hasColor) {
+ decoder >> color;
+ if (!color)
+ return WTF::nullopt;
+ }
+
+ Optional<bool> hasThickness;
+ decoder >> hasThickness;
+ if (!hasThickness)
+ return WTF::nullopt;
+
+ Optional<float> thickness;
+ if (*hasThickness) {
+ decoder >> thickness;
+ if (!thickness)
+ return WTF::nullopt;
+ }
+
+ return SetStrokeState::create(WTFMove(color), WTFMove(thickness));
+}
+
class SetState : public Item {
public:
static Ref<SetState> create(const GraphicsContextState& state, GraphicsContextState::StateChangeFlags flags)
@@ -352,10 +467,6 @@
const GraphicsContextStateChange& state() const { return m_state; }
- void accumulate(const GraphicsContextState&, GraphicsContextState::StateChangeFlags);
-
- void accumulate(GraphicsContextState&) const;
-
static void builderState(GraphicsContext&, const GraphicsContextState&, GraphicsContextState::StateChangeFlags);
static void dumpStateChanges(WTF::TextStream&, const GraphicsContextState&, GraphicsContextState::StateChangeFlags);
@@ -2904,6 +3015,12 @@
case ItemType::ConcatenateCTM:
encoder << downcast<ConcatenateCTM>(*this);
break;
+ case ItemType::SetFillColor:
+ encoder << downcast<SetFillColor>(*this);
+ break;
+ case ItemType::SetStrokeState:
+ encoder << downcast<SetStrokeState>(*this);
+ break;
case ItemType::SetState:
encoder << downcast<SetState>(*this);
break;
@@ -3078,6 +3195,14 @@
if (auto item = ConcatenateCTM::decode(decoder))
return static_reference_cast<Item>(WTFMove(*item));
break;
+ case ItemType::SetFillColor:
+ if (auto item = SetFillColor::decode(decoder))
+ return static_reference_cast<Item>(WTFMove(*item));
+ break;
+ case ItemType::SetStrokeState:
+ if (auto item = SetStrokeState::decode(decoder))
+ return static_reference_cast<Item>(WTFMove(*item));
+ break;
case ItemType::SetState:
if (auto item = SetState::decode(decoder))
return static_reference_cast<Item>(WTFMove(*item));
@@ -3284,6 +3409,8 @@
SPECIALIZE_TYPE_TRAITS_DISPLAYLIST_ITEM(Scale)
SPECIALIZE_TYPE_TRAITS_DISPLAYLIST_ITEM(SetCTM)
SPECIALIZE_TYPE_TRAITS_DISPLAYLIST_ITEM(ConcatenateCTM)
+SPECIALIZE_TYPE_TRAITS_DISPLAYLIST_ITEM(SetFillColor)
+SPECIALIZE_TYPE_TRAITS_DISPLAYLIST_ITEM(SetStrokeState)
SPECIALIZE_TYPE_TRAITS_DISPLAYLIST_ITEM(SetState)
SPECIALIZE_TYPE_TRAITS_DISPLAYLIST_ITEM(SetLineCap)
SPECIALIZE_TYPE_TRAITS_DISPLAYLIST_ITEM(SetLineDash)
@@ -3343,6 +3470,8 @@
WebCore::DisplayList::ItemType::Scale,
WebCore::DisplayList::ItemType::SetCTM,
WebCore::DisplayList::ItemType::ConcatenateCTM,
+ WebCore::DisplayList::ItemType::SetFillColor,
+ WebCore::DisplayList::ItemType::SetStrokeState,
WebCore::DisplayList::ItemType::SetState,
WebCore::DisplayList::ItemType::SetLineCap,
WebCore::DisplayList::ItemType::SetLineDash,
Modified: trunk/Source/WebCore/platform/graphics/displaylists/DisplayListRecorder.cpp (268703 => 268704)
--- trunk/Source/WebCore/platform/graphics/displaylists/DisplayListRecorder.cpp 2020-10-20 00:41:24 UTC (rev 268703)
+++ trunk/Source/WebCore/platform/graphics/displaylists/DisplayListRecorder.cpp 2020-10-20 00:46:01 UTC (rev 268704)
@@ -57,6 +57,40 @@
appendItem(WebCore::DisplayList::PutImageData::create(inputFormat, imageData, srcRect, destPoint, destFormat));
}
+static bool containsOnlyStrokeColorOrThicknessChange(GraphicsContextState::StateChangeFlags changes)
+{
+ static constexpr GraphicsContextState::StateChangeFlags strokeStateChangeFlags {
+ GraphicsContextState::StrokeThicknessChange,
+ GraphicsContextState::StrokeColorChange
+ };
+ return changes == (changes & strokeStateChangeFlags);
+}
+
+static bool containsOnlyFillColorChange(GraphicsContextState::StateChangeFlags changes)
+{
+ return changes == (changes & GraphicsContextState::FillColorChange);
+}
+
+static Ref<Item> createStateChangeItem(const GraphicsContextStateChange& changes, GraphicsContextState::StateChangeFlags changeFlags)
+{
+ if (containsOnlyFillColorChange(changeFlags))
+ return SetFillColor::create(changes.m_state.fillColor);
+
+ if (containsOnlyStrokeColorOrThicknessChange(changeFlags)) {
+ Optional<Color> strokeColor;
+ if (changeFlags.contains(GraphicsContextState::StrokeColorChange))
+ strokeColor = changes.m_state.strokeColor;
+
+ Optional<float> strokeThickness;
+ if (changeFlags.contains(GraphicsContextState::StrokeThicknessChange))
+ strokeThickness = changes.m_state.strokeThickness;
+
+ return SetStrokeState::create(WTFMove(strokeColor), WTFMove(strokeThickness));
+ }
+
+ return SetState::create(changes.m_state, changeFlags);
+}
+
void Recorder::willAppendItem(const Item& item)
{
if (m_observer)
@@ -71,7 +105,7 @@
GraphicsContextState::StateChangeFlags changesFromLastState = stateChanges.changesFromState(currentState().lastDrawingState);
if (changesFromLastState) {
LOG_WITH_STREAM(DisplayLists, stream << "pre-drawing, saving state " << GraphicsContextStateChange(stateChanges.m_state, changesFromLastState));
- m_displayList.append(SetState::create(stateChanges.m_state, changesFromLastState));
+ m_displayList.append(createStateChangeItem(stateChanges, changesFromLastState));
stateChanges.m_changeFlags = { };
currentState().lastDrawingState = stateChanges.m_state;
}
