Bug 90673

Summary:

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

Product:

WebKit

Reporter:

Pierre Rossi <pierre.rossi>

Component:

Platform

Assignee:

Pierre Rossi <pierre.rossi>

Status:

NEW

Severity:

Normal

CC:

ahmad.saleem792, ap, dglazkov, eric, fantasai.bugs, hyatt, mitz, mmaxfield, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none

Pierre Rossi

Reported 2012-07-06 03:04:59 PDT

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

Attachments
Patch (17.03 KB, patch) 2012-07-06 03:23 PDT, Pierre Rossi	no flags	Details Formatted Diff Diff
Patch (19.15 KB, patch) 2012-07-06 09:45 PDT, Pierre Rossi	no flags	Details Formatted Diff Diff
Patch (20.24 KB, patch) 2012-07-06 10:49 PDT, Pierre Rossi	no flags	Details Formatted Diff Diff
Patch (20.26 KB, patch) 2012-07-12 08:18 PDT, Pierre Rossi	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Pierre Rossi

Comment 1 2012-07-06 03:23:04 PDT

Created attachment 151046 [details] Patch

Early Warning System Bot

Comment 2 2012-07-06 03:33:16 PDT

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 3 2012-07-06 03:46:41 PDT

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 4 2012-07-06 03:47:32 PDT

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 5 2012-07-06 06:34:50 PDT

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 6 2012-07-06 08:04:00 PDT

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

Comment 7 2012-07-06 09:45:46 PDT

Created attachment 151095 [details] Patch

WebKit Review Bot

Comment 8 2012-07-06 10:02:25 PDT

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 9 2012-07-06 10:14:37 PDT

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

Pierre Rossi

Comment 10 2012-07-06 10:49:33 PDT

Created attachment 151102 [details] Patch Some more blind build fixes