Bug 208266 - Optimize Path::encode on platforms that support CGPathGetNumberOfElements
Summary: Optimize Path::encode on platforms that support CGPathGetNumberOfElements
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Wenson Hsieh
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-02-26 14:40 PST by Wenson Hsieh
Modified: 2020-03-05 08:14 PST (History)
8 users (show)

See Also:


Attachments
Patch (6.23 KB, patch)
2020-02-29 13:59 PST, Wenson Hsieh
no flags Details | Formatted Diff | Diff
Address comments (6.22 KB, patch)
2020-03-01 21:50 PST, Wenson Hsieh
no flags Details | Formatted Diff | Diff
Address smfr's comment (6.25 KB, patch)
2020-03-02 13:44 PST, Wenson Hsieh
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.