Bug 208266

Summary: Optimize Path::encode on platforms that support CGPathGetNumberOfElements
Product: WebKit Reporter: Wenson Hsieh <wenson_hsieh>
Component: WebCore Misc.Assignee: Wenson Hsieh <wenson_hsieh>
Status: RESOLVED FIXED    
Severity: Normal CC: bdakin, commit-queue, darin, jonlee, sabouhallawa, simon.fraser, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Address comments
none
Address smfr's comment none

Description Wenson Hsieh 2020-02-26 14:40:07 PST
We can elide one of the calls to Path::apply() by directly asking for the number of path elements instead.
Comment 1 Radar WebKit Bug Importer 2020-02-26 19:01:40 PST
<rdar://problem/59832350>
Comment 2 Wenson Hsieh 2020-02-29 13:59:24 PST
Created attachment 392072 [details]
Patch
Comment 3 Darin Adler 2020-03-01 18:35:41 PST
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?
Comment 4 Wenson Hsieh 2020-03-01 18:50:57 PST
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.
Comment 5 Wenson Hsieh 2020-03-01 21:49:54 PST
> 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.
Comment 6 Wenson Hsieh 2020-03-01 21:50:50 PST
Created attachment 392116 [details]
Address comments
Comment 7 Simon Fraser (smfr) 2020-03-02 13:18:17 PST
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
Comment 8 Wenson Hsieh 2020-03-02 13:31:33 PST
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.
Comment 9 Wenson Hsieh 2020-03-02 13:44:02 PST
Created attachment 392183 [details]
Address smfr's comment
Comment 10 WebKit Commit Bot 2020-03-05 08:13:24 PST
The commit-queue encountered the following flaky tests while processing attachment 392183 [details]:

editing/spelling/spellcheck-async-remove-frame.html bug 158401 (authors: morrita@google.com, rniwa@webkit.org, and tony@chromium.org)
The commit-queue is continuing to process your patch.
Comment 11 WebKit Commit Bot 2020-03-05 08:14:02 PST
Comment on attachment 392183 [details]
Address smfr's comment

Clearing flags on attachment: 392183

Committed r257918: <https://trac.webkit.org/changeset/257918>
Comment 12 WebKit Commit Bot 2020-03-05 08:14:03 PST
All reviewed patches have been landed.  Closing bug.