Bug 205001 – Add encoding and decoding support for more DisplayListItems

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Wenson Hsieh]

See: DisplayListItems.h in WebCore.

[Wenson Hsieh]

Created attachment 385132 [details]
WIP

[Tim Horton]

❤️

[Tim Horton]

Comment on attachment 385132 [details]
WIP

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

> Source/WebCore/platform/graphics/GraphicsTypes.h:199
> +    WebCore::LineCap::ButtCap,

At some point we should enum class these (and drop the suffixes). I'd mostly been doing that as I went along, but I can see why you might not want to :D

> Source/WebCore/platform/graphics/displaylists/DisplayListItems.h:533
> +void SetLineCap::encode(Encoder& encoder) const

Wonder why these aren't in the State thing

> Source/WebCore/platform/graphics/displaylists/DisplayListItems.h:1233
> +    encoder << m_point1 << m_point2;

I don't love multiple items on one line.

> Source/WebCore/platform/graphics/displaylists/DisplayListItems.h:1358
> +    encoder << m_rect << m_style;

Ditto

> Source/WebCore/platform/graphics/displaylists/DisplayListItems.h:1845
> +    encoder << m_rect;

Much better

[Wenson Hsieh]

Comment on attachment 385132 [details]
WIP

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

I’m also adding a few more image-y display list item encoder/decoders in a v2.

>> Source/WebCore/platform/graphics/GraphicsTypes.h:199
>> +    WebCore::LineCap::ButtCap,
> 
> At some point we should enum class these (and drop the suffixes). I'd mostly been doing that as I went along, but I can see why you might not want to :D

Definitely considered it! But yes, I’ll do that it a separate change (since this patch has already gotten quite large).

>> Source/WebCore/platform/graphics/displaylists/DisplayListItems.h:533
>> +void SetLineCap::encode(Encoder& encoder) const
> 
> Wonder why these aren't in the State thing

Good question. I’ll leave it like this for now (it might be something to fix when I work on getting SetState to propagate all of its bits).

>> Source/WebCore/platform/graphics/displaylists/DisplayListItems.h:1233
>> +    encoder << m_point1 << m_point2;
> 
> I don't love multiple items on one line.

Ok! Moved to one << per line.

[Tim Horton]

(In reply to Wenson Hsieh from comment #4)
> Comment on attachment 385132 [details]
> WIP
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=385132&action=review
> 
> I’m also adding a few more image-y display list item encoder/decoders in a
> v2.

Woot

> >> Source/WebCore/platform/graphics/displaylists/DisplayListItems.h:533
> >> +void SetLineCap::encode(Encoder& encoder) const
> > 
> > Wonder why these aren't in the State thing
> 
> Good question. I’ll leave it like this for now (it might be something to fix
> when I work on getting SetState to propagate all of its bits).

Yeah this was more of a smfr question

[Wenson Hsieh]

Created attachment 385133 [details]
v2 (w/ more drawing items supported)

[WebKit Commit Bot]

Comment on attachment 385133 [details]
v2 (w/ more drawing items supported)

Clearing flags on attachment: 385133

Committed r253283: <https://trac.webkit.org/changeset/253283>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/57742387>