Bug 32794 - With text-rendering: optimizelegibility, sometimes lines run too long
Summary: With text-rendering: optimizelegibility, sometimes lines run too long
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Text (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh OS X 10.6
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2009-12-20 08:55 PST by mitz
Modified: 2010-01-06 17:42 PST (History)
2 users (show)

See Also:


Attachments
Test case (559 bytes, text/html)
2009-12-20 08:56 PST, mitz
no flags Details
Allow trailing space to affect kerning when measuring a word (60.58 KB, patch)
2010-01-06 17:17 PST, mitz
sam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description mitz 2009-12-20 08:55:46 PST
The attached test case shows the using text-rendering: optimizelegibility can cause lines to break too late, and the effect that it has on left-aligned and on justified text. Perhaps there is a discrepancy between how text is measured for the purpose of line breaking and how it’s actually rendered afterwards.
Comment 1 mitz 2009-12-20 08:56:38 PST
Created attachment 45282 [details]
Test case
Comment 2 mitz 2009-12-20 08:57:03 PST
<rdar://problem/7488126>
Comment 3 mitz 2010-01-06 17:17:47 PST
Created attachment 46010 [details]
Allow trailing space to affect kerning when measuring a word
Comment 4 WebKit Review Bot 2010-01-06 17:19:13 PST
style-queue ran check-webkit-style on attachment 46010 [details] without any errors.
Comment 5 Darin Adler 2010-01-06 17:29:22 PST
Comment on attachment 46010 [details]
Allow trailing space to affect kerning when measuring a word

> +            TextRenderingMode textRenderingMode = f.fontDescription().textRenderingMode();
> +            // Non-zero only when kerning is enabled, in which case we measure words with their trailing
> +            // space, then subtract its width.
> +            int wordTrailingSpaceWidth = textRenderingMode == OptimizeLegibility || textRenderingMode == GeometricPrecision ? f.spaceWidth() + wordSpacing : 0;

I think it would be clearer to have a function named allowsKerning or something like that. It could contain:

    TextRenderingMode textRenderingMode = f.fontDescription().textRenderingMode();
    textRenderingMode == OptimizeLegibility || textRenderingMode == GeometricPrecision

Then you could saw:

    wordTrailingSpaceWidth = allowsKerning(f) ? f.spaceWidth() + wordSpacing : 0;

Maybe there's a better name than "allows kerning", though.
Comment 6 mitz 2010-01-06 17:39:28 PST
(In reply to comment #5)
> (From update of attachment 46010 [details])
> > +            TextRenderingMode textRenderingMode = f.fontDescription().textRenderingMode();
> > +            // Non-zero only when kerning is enabled, in which case we measure words with their trailing
> > +            // space, then subtract its width.
> > +            int wordTrailingSpaceWidth = textRenderingMode == OptimizeLegibility || textRenderingMode == GeometricPrecision ? f.spaceWidth() + wordSpacing : 0;
> 
> I think it would be clearer to have a function named allowsKerning or something
> like that. It could contain:
> 
>     TextRenderingMode textRenderingMode =
> f.fontDescription().textRenderingMode();
>     textRenderingMode == OptimizeLegibility || textRenderingMode ==
> GeometricPrecision
> 
> Then you could saw:
> 
>     wordTrailingSpaceWidth = allowsKerning(f) ? f.spaceWidth() + wordSpacing :
> 0;
> 
> Maybe there's a better name than "allows kerning", though.

I intend to add two Font methods (one for kerning and one for ligatures) and deploy them here and in the many other places that currently access the TextRenderingMode.
Comment 7 mitz 2010-01-06 17:42:15 PST
Fixed in <http://trac.webkit.org/projects/webkit/changeset/52889>.