Bug 67996 - WebFont followed tiny monospace text displays weird
: WebFont followed tiny monospace text displays weird
Status: RESOLVED FIXED
: WebKit
Text
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To:
: http://crbug.com/59033
: InRadar
:
:
  Show dependency treegraph
 
Reported: 2011-09-13 03:50 PST by
Modified: 2011-10-18 16:34 PST (History)


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


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2011-09-13 03:50:21 PST
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 From 2011-09-13 04:10:27 PST -------
Created an attachment (id=107162) [details]
Patch
------- Comment #2 From 2011-09-13 07:34:20 PST -------
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 From 2011-09-13 08:21:51 PST -------
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 From 2011-09-13 09:46:46 PST -------
(From update of attachment 107162 [details])
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 From 2011-09-13 09:56:41 PST -------
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 From 2011-09-13 09:58:03 PST -------
(From update of attachment 107162 [details])
review-; please try Mitz’s suggestion of calling wkSetCGFontRenderingMode every time
------- Comment #7 From 2011-09-13 17:13:43 PST -------
Created an attachment (id=107263) [details]
Patch V1
------- Comment #8 From 2011-09-13 17:16:18 PST -------
Hi Darin, mitz,

Thank you so much for your help.  Calling wkSetCGFontRenderingMode() always solves the problem.  Revised the patch.
------- Comment #9 From 2011-09-13 18:57:33 PST -------
(From update of attachment 107263 [details])
Clearing flags on attachment: 107263

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