Bug 67996 - WebFont followed tiny monospace text displays weird
: WebFont followed tiny monospace text displays weird
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: Text
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To: Kenichi Ishibashi
http://crbug.com/59033
: InRadar
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-09-13 03:50 PDT by Kenichi Ishibashi
Modified: 2011-10-18 16:34 PDT (History)
6 users (show)

See Also:


Attachments
Patch (11.50 KB, patch)
2011-09-13 04:10 PDT, Kenichi Ishibashi
no flags Details | Formatted Diff | Diff
Patch V1 (11.35 KB, patch)
2011-09-13 17:13 PDT, Kenichi Ishibashi
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kenichi Ishibashi 2011-09-13 03:50:21 PDT
Text using webfonts sometimes looks weird. See http://crbug.com/59033 in details.  I confirmed this could occur with Safari and Chromium mac.
Comment 1 Kenichi Ishibashi 2011-09-13 04:10:27 PDT
Created attachment 107162 [details]
Patch
Comment 2 mitz@webkit.org 2011-09-13 07:34:20 PDT
I was told that saving and restoring the graphics state is relatively expensive. Is it possible to save and restore the specific attribute that’s changing here?
Comment 3 Kenichi Ishibashi 2011-09-13 08:21:51 PDT
Thank you for taking a look.

(In reply to comment #2)
> I was told that saving and restoring the graphics state is relatively expensive. Is it possible to save and restore the specific attribute that’s changing here?

I'm not sure which attributes should be saved and restored because wkSetCGFontRenderingMode() is black-box to me.. I'd appreciate if someone could give advice on this. (I might well have to brute force investigation, though).
Comment 4 Darin Adler 2011-09-13 09:46:46 PDT
Comment on attachment 107162 [details]
Patch

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

> Source/WebCore/platform/graphics/mac/FontMac.mm:212
> +        CGContextSaveGState(cgContext);

We need to know more before we make this code change.

Doing a complete CGContextSaveGState can be an expensive way to save and restore state. While complete, it has a high performance cost. I believe that’s why we don’t use it to preserve the setting of CGContextSetShouldSmoothFonts.

An alternative is to come up with a different way of dealing with the state in question. For each piece of state we can decide to either:

    1) Set it each time, or
    2) Restore it to the previous state with an explicit function call, or
    3) Restore it to a “neutral” state with an explicit function call.

For font size we might choose to do (1). Each piece of code drawing any text would have to set the font size and not assume that it’s set to any particular value beforehand.

For font rendering mode we might choose to do either (1) or (3).

I am not going to say review- because I am not certain that a complete save/restore is too slow, but I suspect it is.
Comment 5 mitz@webkit.org 2011-09-13 09:56:41 PDT
I think the best approach here would be to always set the rendering mode. wkSetCGFontRenderingMode() will behave correctly if passed a nil font.
Comment 6 Darin Adler 2011-09-13 09:58:03 PDT
Comment on attachment 107162 [details]
Patch

review-; please try Mitz’s suggestion of calling wkSetCGFontRenderingMode every time
Comment 7 Kenichi Ishibashi 2011-09-13 17:13:43 PDT
Created attachment 107263 [details]
Patch V1
Comment 8 Kenichi Ishibashi 2011-09-13 17:16:18 PDT
Hi Darin, mitz,

Thank you so much for your help.  Calling wkSetCGFontRenderingMode() always solves the problem.  Revised the patch.
Comment 9 WebKit Review Bot 2011-09-13 18:57:33 PDT
Comment on attachment 107263 [details]
Patch V1

Clearing flags on attachment: 107263

Committed r95070: <http://trac.webkit.org/changeset/95070>
Comment 10 WebKit Review Bot 2011-09-13 18:57:38 PDT
All reviewed patches have been landed.  Closing bug.
Comment 11 mitz@webkit.org 2011-09-27 11:23:11 PDT
*** Bug 28915 has been marked as a duplicate of this bug. ***
Comment 12 David Harrison 2011-10-18 16:34:38 PDT
rdar://10259221