Bug 208266 – Optimize Path::encode on platforms that support CGPathGetNumberOfElements

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Wenson Hsieh]

We can elide one of the calls to Path::apply() by directly asking for the number of path elements instead.

[Radar WebKit Bug Importer]

<rdar://problem/59832350>

[Wenson Hsieh]

Created attachment 392072 [details]
Patch

[Darin Adler]

Comment on attachment 392072 [details]
Patch

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

Do you know the reason for the mysterious Windows failure? It does seem to be new to the patch.

> Source/WebCore/platform/graphics/Path.h:136
> +    WEBCORE_EXPORT unsigned elementCount() const;

32-bit here

> Source/WebCore/platform/graphics/Path.h:234
> +    encoder << static_cast<uint64_t>(elementCount());

64-bit here

Why?

[Wenson Hsieh]

Comment on attachment 392072 [details]
Patch

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

>> Source/WebCore/platform/graphics/Path.h:234
>> +    encoder << static_cast<uint64_t>(elementCount());
> 
> 64-bit here
> 
> Why?

Good catch! No compelling reason for this difference.

elementCount() should return a uint64_t, and then we could just remove the static_cast here.

[Wenson Hsieh]

> Do you know the reason for the mysterious Windows failure? It does seem to
> be new to the patch.

After using the new Retry button, it looks like this was just an unrelated flaky failure (the Win bot is green). The patch should not have changed any behavior on non-Cocoa ports.

[Wenson Hsieh]

Created attachment 392116 [details]
Address comments

[Simon Fraser (smfr)]

Comment on attachment 392116 [details]
Address comments

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

> Source/WebCore/platform/graphics/Path.h:136
> +    WEBCORE_EXPORT uint64_t elementCount() const;

Why not size_t

[Wenson Hsieh]

Comment on attachment 392116 [details]
Address comments

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

>> Source/WebCore/platform/graphics/Path.h:136
>> +    WEBCORE_EXPORT uint64_t elementCount() const;
> 
> Why not size_t

Sure, I suppose that would be clearer.

[Wenson Hsieh]

Created attachment 392183 [details]
Address smfr's comment

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 392183 [details]:

editing/spelling/spellcheck-async-remove-frame.html bug 158401 (authors: [e-mail], [e-mail], and [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 392183 [details]
Address smfr's comment

Clearing flags on attachment: 392183

Committed r257918: <https://trac.webkit.org/changeset/257918>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.