Bug 123338 - Re-enable simple line layout on non-Mac platforms
Summary: Re-enable simple line layout on non-Mac platforms
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Csaba Osztrogonác
URL:
Keywords:
Depends on: 123388 123402
Blocks:
  Show dependency treegraph
 
Reported: 2013-10-25 06:43 PDT by Antti Koivisto
Modified: 2014-02-06 09:25 PST (History)
8 users (show)

See Also:


Attachments
Patch (1.61 KB, patch)
2014-02-06 08:39 PST, Csaba Osztrogonác
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antti Koivisto 2013-10-25 06:43:05 PDT
It hits

ASSERT(run.charactersLength() >= run.length()) 

in textWidth().

for example:
http://build.webkit.org/builders/GTK%20Linux%2064-bit%20Debug%20WK1/builds/4983
http://build.webkit.org/builders/EFL%20Linux%2064-bit%20Release%20WK1/builds/2321
Comment 1 Antti Koivisto 2013-10-25 06:48:37 PDT
Some
Comment 2 Antti Koivisto 2013-10-25 06:49:30 PDT
It is not obvious from code what the difference is between Mac and other platforms.
Comment 3 Zan Dobersek 2013-10-25 12:28:28 PDT
Non-Mac platforms don't define ENABLE_8BIT_TEXTRUN to 1, so the TextRun constructor implicitly converts the character data to a WTF::String. The length parameter that's passed to the TextRun constructor is then ignored, and the length of the complete string is used.
Comment 4 Zan Dobersek 2013-10-26 05:34:16 PDT
(In reply to comment #3)
> The length parameter that's passed to the TextRun constructor is then ignored, and the length of the complete string is used.

The length parameter is actually used as the xpos value rather than the length.

Anyway, enabling the 8-bit text runs works for the GTK port. Modifying the TextRun construction in the textWidth function would likely work equally well.

The 8-bit TextRun was introduced in bug #96979, but was only enabled for the Mac port because code of other platforms needed additional modifications to properly support the introduced optimization.
https://bugs.webkit.org/show_bug.cgi?id=96979#c9

The GTK port doesn't seem affected when running the tests with the support for 8-bit text runs enabled, so simply enabling the feature define would work. I'd also like to see people test out the feature on other ports and provide the proper changes so the define can be removed entirely.
Comment 5 Antti Koivisto 2013-10-28 07:54:50 PDT
Ah ok. Thanks for looking into this. We should really make String(const char*) explicit to avoid bugs like this.
Comment 6 Csaba Osztrogonác 2014-02-06 08:37:20 PST
ENABLE_8BIT_TEXTRUN guards were removed by
https://trac.webkit.org/changeset/163478

So let's go ahead.
Comment 7 Csaba Osztrogonác 2014-02-06 08:39:13 PST
Created attachment 223334 [details]
Patch
Comment 8 Csaba Osztrogonác 2014-02-06 09:24:57 PST
Comment on attachment 223334 [details]
Patch

Clearing flags on attachment: 223334

Committed r163537: <http://trac.webkit.org/changeset/163537>
Comment 9 Csaba Osztrogonác 2014-02-06 09:25:03 PST
All reviewed patches have been landed.  Closing bug.