Bug 215052 – Make GlyphBuffers required in the fast text codepath

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Myles C. Maxfield]

Make GlyphBuffers required in the fast text codepath

[Myles C. Maxfield]

Created attachment 405771 [details]
Patch

[Myles C. Maxfield]

Comment on attachment 405771 [details]
Patch

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

> Source/WebCore/platform/graphics/WidthIterator.cpp:84
>      if (m_run.length() <= 1 || !(m_enableKerning || m_requiresShaping))

if (m_run.length() <= 1)

[Myles C. Maxfield]

Created attachment 405789 [details]
Patch

[Myles C. Maxfield]

Created attachment 405790 [details]
Patch

[Simon Fraser (smfr)]

Is this going to be a perf regression?

[Myles C. Maxfield]

Created attachment 405977 [details]
Patch

[Myles C. Maxfield]

Created attachment 405981 [details]
Patch

[Myles C. Maxfield]

(In reply to Simon Fraser (smfr) from comment #5)
> Is this going to be a perf regression?

I'm still testing. Will comment here when I know more.

[Myles C. Maxfield]

% compare-results -a plt-results-Vanilla -b plt-results-RequiredGlyphBuffers 
a mean = 740.41311
b mean = 743.70648
pValue = 0.7209862553
(Smaller means are better.)
1.004 times worse
Results ARE NOT significant

[Myles C. Maxfield]

The design test of MotionMark shows this to be a progression

With patch: 156pt
Without patch: 153pt

[Radar WebKit Bug Importer]

<rdar://problem/66718097>

[Darin Adler]

Comment on attachment 405981 [details]
Patch

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

> Source/WebCore/platform/graphics/FontCascade.cpp:450
> +    // This is needed only to match the result of the slow path. Same glyph widths but different floating point arithmentics can
> +    // produce different run width.

Existing comment just moving but:

- Would look nicer if line break was at sentence break.
- "arithmentics"

> LayoutTests/ChangeLog:8
> +        * platform/win/fast/events/selectstart-by-drag-expected.txt: Test progressed on Windows.

Do you know why?

[Myles C. Maxfield]

Comment on attachment 405981 [details]
Patch

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

>> LayoutTests/ChangeLog:8
>> +        * platform/win/fast/events/selectstart-by-drag-expected.txt: Test progressed on Windows.
> 
> Do you know why?

I don't have a Windows development environment, so I can't say for sure, but requiring GlyphBuffers is a slight behavior change. When GlyphBuffers aren't specified, shaping doesn't occur at all, so making them mandatory performs shaping when shaping didn't always used to occur.  (This is why I made sure to check performance numbers before making this patch as r?.)

Because this is a progression, and aligns Windows with Mac and iOS, I didn't look too hard at it.

[Myles C. Maxfield]

Committed r265413: <https://trac.webkit.org/changeset/265413>