[webkit-reviews] review granted: [Bug 204740] Start adding encoding support for DisplayList and some DisplayListItems : [Attachment 384603] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Tim Horton
<thorton at apple.com>'s request for review:
Bug 204740: Start adding encoding support for DisplayList and some
DisplayListItems
https://bugs.webkit.org/show_bug.cgi?id=204740

Attachment 384603: Patch

https://bugs.webkit.org/attachment.cgi?id=384603&action=review




--- Comment #7 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 384603
  --> https://bugs.webkit.org/attachment.cgi?id=384603
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=384603&action=review

> Source/WebCore/platform/graphics/GraphicsContext.cpp:101
> +    if
((m_changeFlags.contains(GraphicsContextState::CompositeOperationChange) ||
m_changeFlags.contains(GraphicsContextState::BlendModeChange))

Can this be m_changeFlags.containsAny({
GraphicsContextState::CompositeOperationChange,
GraphicsContextState::BlendModeChange })

> Source/WebCore/platform/graphics/GraphicsContext.cpp:164
> +    if (flags.contains(GraphicsContextState::CompositeOperationChange) ||
flags.contains(GraphicsContextState::BlendModeChange)) {

Ditto

> Source/WebCore/platform/graphics/GraphicsContext.cpp:239
> +    if
(m_changeFlags.contains(GraphicsContextState::CompositeOperationChange) ||
m_changeFlags.contains(GraphicsContextState::BlendModeChange))

Ditto

> Source/WebCore/platform/graphics/displaylists/DisplayList.h:134
> +	   if (!item)
> +	       continue;

Log here?

> Source/WebCore/platform/graphics/displaylists/DisplayListItems.h:174
> +    template<class Encoder> void encodeForDrawingItem(Encoder&) const;
> +    template<class Decoder> static bool decodeForDrawingItem(Decoder&,
DrawingItem&);

Names are confusing.

> Source/WebCore/platform/graphics/displaylists/DisplayListItems.h:504
> +    if (*changeFlags & GraphicsContextState::StrokeColorChange) {

stateChange.m_changeFlags.contains()

> Source/WebCore/platform/graphics/displaylists/DisplayListItems.h:513
> +    if (*changeFlags & GraphicsContextState::FillColorChange) {

.contains()

> Source/WebCore/platform/graphics/displaylists/DisplayListItems.h:522
> +    if (*changeFlags & GraphicsContextState::AlphaChange) {

.contains()