Bug 90673 – Avoid querying the font cache for the zero-width space glyph

• Status: NEW
• Component: Platform

[Pierre Rossi]

Avoid querying the font cache for the zero-width space glyph

[Pierre Rossi]

Created attachment 151046 [details]
Patch

[Early Warning System Bot]

Comment on attachment 151046 [details]
Patch

Attachment 151046 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/13155238

[Build Bot]

Comment on attachment 151046 [details]
Patch

Attachment 151046 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/13152338

[WebKit Review Bot]

Comment on attachment 151046 [details]
Patch

Attachment 151046 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13152335

[Alexey Proskuryakov]

Comment on attachment 151046 [details]
Patch

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

> Source/WebCore/ChangeLog:11
> +        We can bypass a large part of this and rely directly on
> +        the character value.

Could you please explain why this is an improvement? How did you assess performance impact?

[Pierre Rossi]

Comment on attachment 151046 [details]
Patch

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

I'm going to try to clarify this ChangeLog entry in the next iteration, along with the necessary build fixes the EWS pointed out.

>> Source/WebCore/ChangeLog:11
>> +        the character value.
> 
> Could you please explain why this is an improvement? How did you assess performance impact?

The part that I believe to be an improvement is that it doesn't rely on each port's implementation of FontCache::getFontDataForCharacters() in the case of ZWSP. The example of the Qt port, where the mechanism used to get the glyph index for a given character becomes moot for ZWSP made me wonder why we need to store this particular glyph index in the first place.
I didn't bother to test things performance-wise to be honest, since it didn't seem to me this change could have a negative impact, but I can be proven wrong.

[Pierre Rossi]

Created attachment 151095 [details]
Patch

[WebKit Review Bot]

Comment on attachment 151095 [details]
Patch

Attachment 151095 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13146444

[Build Bot]

Comment on attachment 151095 [details]
Patch

Attachment 151095 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/13140540

[Pierre Rossi]

Created attachment 151102 [details]
Patch

Some more blind build fixes

[WebKit Review Bot]

Comment on attachment 151102 [details]
Patch

Attachment 151102 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13154387

[Pierre Rossi]

Created attachment 151959 [details]
Patch

Rebased patch, since the previous patch seems to have had trouble applying on the chromium EWS bot.

[Ahmad Saleem]

@Myles & @Elika - is this applicable anymore?