RESOLVED FIXED 66498
Typesetting features need not force complex for single-character runs
https://bugs.webkit.org/show_bug.cgi?id=66498
Summary Typesetting features need not force complex for single-character runs
Ned Holbrook
Reported 2011-08-18 14:33:56 PDT
The only typesetting features are kerning and ligatures, neither of which applies for a single character. As such, there's no need to spool up the complex text machinery for a single-character run just because the font has typesetting features. This is particularly interesting when hyphenation is enabled.
Attachments
Proposed changes. (1.32 KB, patch)
2011-08-18 14:55 PDT, Ned Holbrook
mitz: review+
Diffs from failing test. (1.28 KB, text/plain)
2011-08-18 17:46 PDT, Ned Holbrook
no flags
Updated ChangeLog. (1.35 KB, patch)
2011-09-01 09:28 PDT, Ned Holbrook
no flags
Ned Holbrook
Comment 1 2011-08-18 14:55:34 PDT
Created attachment 104401 [details] Proposed changes. In most cases this isn't much of a problem (a percent or less) but every little bit helps.
Ned Holbrook
Comment 2 2011-08-18 17:19:51 PDT
Running the full set of tests shows that fast/writing-mode/text-orientation-basic.html is failing with this patch, I'm investigating now.
Ned Holbrook
Comment 3 2011-08-18 17:46:21 PDT
Created attachment 104428 [details] Diffs from failing test.
Ned Holbrook
Comment 4 2011-08-23 11:10:16 PDT
(In reply to comment #2) > Running the full set of tests shows that fast/writing-mode/text-orientation-basic.html is failing with this patch, I'm investigating now. This should be fixed by 66733.
Ned Holbrook
Comment 5 2011-08-31 14:27:40 PDT
As of r94090, I see no fast or svg pixel test failures due to the proposed changes. I also ran the entire suite of tests without pixel tests and saw no failures.
mitz
Comment 6 2011-09-01 00:59:47 PDT
Comment on attachment 104401 [details] Proposed changes. View in context: https://bugs.webkit.org/attachment.cgi?id=104401&action=review > Source/WebCore/ChangeLog:11 > + No new tests. (OOPS!) I think this line will stop the commit bot from landing this patch
Ned Holbrook
Comment 7 2011-09-01 09:06:12 PDT
(In reply to comment #6) > I think this line will stop the commit bot from landing this patch Oops, indeed! I will change it to note that the changes are for performance and not expected to change layout.
Ned Holbrook
Comment 8 2011-09-01 09:28:12 PDT
Created attachment 105977 [details] Updated ChangeLog.
WebKit Review Bot
Comment 9 2011-09-01 10:19:34 PDT
Comment on attachment 105977 [details] Updated ChangeLog. Clearing flags on attachment: 105977 Committed r94303: <http://trac.webkit.org/changeset/94303>
WebKit Review Bot
Comment 10 2011-09-01 10:19:38 PDT
All reviewed patches have been landed. Closing bug.
James Robinson
Comment 11 2011-09-01 11:50:10 PDT
On Chromium Mac on Snow Leopard this patch caused the text metrics on fast/text/international/vertical-text-glyph-test.html to change very, very slightly. Here's the text diff: --- /b/build/slave/Webkit_Mac10_6__CG__dbg_/build/layout-test-results/fast/text/international/vertical-text-glyph-test-expected.txt +++ /b/build/slave/Webkit_Mac10_6__CG__dbg_/build/layout-test-results/fast/text/international/vertical-text-glyph-test-actual.txt @@ -1,8 +1,8 @@ -layer at (0,0) size 785x634 +layer at (0,0) size 785x635 RenderView at (0,0) size 785x600 -layer at (0,0) size 785x634 - RenderBlock {HTML} at (0,0) size 785x634 - RenderBody {BODY} at (8,21) size 769x605 +layer at (0,0) size 785x635 + RenderBlock {HTML} at (0,0) size 785x635 + RenderBody {BODY} at (8,21) size 769x606 RenderBlock {P} at (0,0) size 769x32 RenderText {#text} at (0,5) size 161x21 text run at (0,5) width 161: "Simple text path" @@ -15,8 +15,8 @@ RenderBlock {P} at (0,320) size 769x32 RenderText {#text} at (0,5) size 181x21 text run at (0,5) width 181: "Complex text path" - RenderBlock (anonymous) at (0,373) size 769x232 - RenderBlock {SPAN} at (0,0) size 32x223 - RenderText {#text} at (5,0) size 21x223 - text run at (5,0) width 223: "string\x{300C}\x{3042}\x{3001}\x{5909}\x{3063}\x{FF01}\x{300D}\x{3002}" + RenderBlock (anonymous) at (0,373) size 769x233 + RenderBlock {SPAN} at (0,0) size 32x224 + RenderText {#text} at (5,0) size 21x224 + text run at (5,0) width 224: "string\x{300C}\x{3042}\x{3001}\x{5909}\x{3063}\x{FF01}\x{300D}\x{3002}" RenderText {#text} at (0,0) size 0x0 This also changes the pixel output slightly - the text all renders the same, but the scrollbar looks different because the scrollable area is 1px taller. I'm planning to rebaseline since this looks harmless, but since the comments indicate that this should not have changed any layout posting as an FYI.
Ned Holbrook
Comment 12 2011-09-01 11:52:37 PDT
Interesting. I wonder if there aren't some minute differences in vertical metrics on SL.
Note You need to log in before you can comment on or make changes to this bug.