Bug 95812 – Add 8 bit string data path to TextRun

• Status: RESOLVED
• Resolution: FIXED
• Component: Text

[Michael Saboff]

As part of adding an 8 bit path to the text rendering code, the TextRun class and code using it need to handle 8 bit strings.

[Michael Saboff]

Created attachment 162296 [details]
Patch

[Gyuyoung Kim]

Comment on attachment 162296 [details]
Patch

Attachment 162296 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/13757692

[Early Warning System Bot]

Comment on attachment 162296 [details]
Patch

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

[Build Bot]

Comment on attachment 162296 [details]
Patch

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

[Eric Seidel (no email)]

Very exciting.

Isn't this a perf change?  Do we have perf data?

[Early Warning System Bot]

Comment on attachment 162296 [details]
Patch

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

[Michael Saboff]

(In reply to comment #5)
> Very exciting.
> 
> Isn't this a perf change?  Do we have perf data?

I don't have data yet.  I expect that tho should have little impact as it is mostly renaming for the current 16 bit paths.  The TextRun::operator[] is the big concern.  If that is an issue, I'll refactor to separate 8 and 16 bit accessors methods.

I'll be running test and posting results.  In addition to the Layout tests, do you have others you'd like to see?

Now to fix all of the platform specific code from EWS failures.

[Peter Beverloo (cr-android ews)]

Comment on attachment 162296 [details]
Patch

Attachment 162296 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/13755917

[WebKit Review Bot]

Comment on attachment 162296 [details]
Patch

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

[Michael Saboff]

Created attachment 162515 [details]
Patch with fixes for failing platforms

Fixed the non-Mac platforms by changing the calls to characters() and data() to characters16() and data16() and by disallowing the creation of 8 bit text runs via compile switches.  These other platforms will have to make the appropriate changes after the patch lands.

[Michael Saboff]

Created attachment 162516 [details]
Performance results from last posted patch

Results of Performance/Layout tests before and after the latest patch.  Ran tests 8 times and averaged the results.

[WebKit Review Bot]

Attachment 162515 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/platform/graphics/Font.cpp:269:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 17 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Michael Saboff]

Created attachment 162518 [details]
Fixed indent style issue.

[Build Bot]

Comment on attachment 162518 [details]
Fixed indent style issue.

Attachment 162518 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13772327

[Early Warning System Bot]

Comment on attachment 162518 [details]
Fixed indent style issue.

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

[Michael Saboff]

Created attachment 162527 [details]
Patch to fix truncation compile error

Reverted templated normalizeSpaces() in Font.h to UChar only as it isn't on fast path.

Comment on attachment 162527 [details]
Patch to fix truncation compile error

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

> Source/WebCore/ChangeLog:11
> +        been renamed and the creation of 8 bit tet runs has been disabled via compilation

Typo: tet.

> Source/WebCore/ChangeLog:12
> +        flags.  Someone knowledgible in those platforms will need to make corresponding changes

Typo: knowledgible.

[Michael Saboff]

Committed r127801: <http://trac.webkit.org/changeset/127801>