Bug 217568 – [MotionMark] Add an inline path data type to represent a circular sector

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

[Wenson Hsieh]

Similar to the extant ArcData, except it represents the following path recipe:

- Move to (x, y)
- Arc with radius r centered at (x, y)
- Line segment back to (x, y)

This should allow us to encode the remaining ~20% or so of “complex” paths in the Canvas Arcs subtest as inline data, which should reduce memory overhead when computing bounding rects, encoding paths, etc. Since the vast majority of the remaining 80% of paths in this subtest are simple arcs (represented by ArcData), this should, in theory, allow us to avoid CGPath allocation in the web process during this subtest.

Still need to measure projected perf impact...

[Wenson Hsieh]

> 
> Still need to measure projected perf impact...

Some early testing shows roughly 2-3%, on top of webkit.org/b/217563

[Wenson Hsieh]

(In reply to Wenson Hsieh from comment #1)
> > 
> > Still need to measure projected perf impact...
> 
> Some early testing shows roughly 2-3%, on top of webkit.org/b/217563

...2-3% better, that is :)

[Wenson Hsieh]

Created attachment 411081 [details]
Work in progress

[Darin Adler]

Comment on attachment 411081 [details]
Work in progress

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

> Source/WebCore/platform/graphics/InlinePathData.h:52
>      bool hasStart { false };
> +    bool endsAtStart { false };

Not great to have these be separate booleans. These two booleans together have only 3 valid states, not 4.

> Source/WebCore/platform/graphics/Path.cpp:270
> +            arc.center.x() + arc.radius * acosf(arc.endAngle),
> +            arc.center.y() + arc.radius * asinf(arc.endAngle)

I suggest using std::acos and std::asin instead of acosf and asinf

[Wenson Hsieh]

Comment on attachment 411081 [details]
Work in progress

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

>> Source/WebCore/platform/graphics/InlinePathData.h:52
>> +    bool endsAtStart { false };
> 
> Not great to have these be separate booleans. These two booleans together have only 3 valid states, not 4.

Good catch! `endsAtStart && !hasStart` should never be a possibility.

I'll fix this by adding an enum class like:

    enum class Type : uint8_t {
        ArcOnly,
        LineAndArc,
        ClosedLineAndArc
    };

…and then replacing `hasStart` and `endsAtStart` with:

    Type type { Type::ArcOnly };

>> Source/WebCore/platform/graphics/Path.cpp:270
>> +            arc.center.y() + arc.radius * asinf(arc.endAngle)
> 
> I suggest using std::acos and std::asin instead of acosf and asinf

Done!

[Wenson Hsieh]

Created attachment 411093 [details]
For EWS

[EWS]

commit-queue failed to commit attachment 411093 [details] to WebKit repository.

[Wenson Hsieh]

Committed r268352: <https://trac.webkit.org/changeset/268352>

[Radar WebKit Bug Importer]

<rdar://problem/70209839>