Summary: | [MotionMark] Add an inline path data type to represent a circular sector | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Wenson Hsieh <wenson_hsieh> | ||||||
Component: | WebCore Misc. | Assignee: | Wenson Hsieh <wenson_hsieh> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | darin, sabouhallawa, simon.fraser, thorton, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=217563 | ||||||||
Attachments: |
|
Description
Wenson Hsieh
2020-10-10 22:42:58 PDT
> > Still need to measure projected perf impact... Some early testing shows roughly 2-3%, on top of webkit.org/b/217563 (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 :) Created attachment 411081 [details]
Work in progress
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 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! Created attachment 411093 [details]
For EWS
commit-queue failed to commit attachment 411093 [details] to WebKit repository.
Committed r268352: <https://trac.webkit.org/changeset/268352> |