Bug 72614

Summary: [GTK] Improve FontMetrics accuracy
Product: WebKit Reporter: Philippe Normand <pnormand>
Component: WebKitGTKAssignee: Martin Robinson <mrobinson>
Status: RESOLVED FIXED    
Severity: Normal CC: laszlo.gombos, mrobinson, zimmermann
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 47467    
Attachments:
Description Flags
Proposed way to get floating point metrics for Cairo
none
proposed patch ((c) WildFox) mrobinson: review+

Philippe Normand
Reported 2011-11-17 07:59:33 PST
Nikolas Zimmermann suggests to look at http://mxr.mozilla.org/mozilla-central/source/gfx/thebes/gfxFT2Utils.cpp#82 Something similar should be done in SimpleFontData::platformInit ... See also bug 47467
Attachments
Proposed way to get floating point metrics for Cairo (3.24 KB, patch)
2011-11-23 04:10 PST, Nikolas Zimmermann
no flags
proposed patch ((c) WildFox) (4.53 KB, patch)
2011-11-25 04:17 PST, Philippe Normand
mrobinson: review+
Philippe Normand
Comment 1 2011-11-18 06:43:29 PST
Some news on this one, it turns out this patch would be needed only if we used the Pango backend but by default we use Freetype. Nikolas came up with a simple fix for the Freetype backend, disabling hinting font metrics.
Martin Robinson
Comment 2 2011-11-18 06:48:50 PST
So just to be clear, disabling font hinting puts the font extents at floating point positions. From what I undertand, we already disabled font hinting long ago.
Nikolas Zimmermann
Comment 3 2011-11-18 08:23:18 PST
(In reply to comment #2) > So just to be clear, disabling font hinting puts the font extents at floating point positions. From what I undertand, we already disabled font hinting long ago. Right you've disabled hinting for bitmap glyphs, that's called "style hinting", this bug is about "metrics hinting". From cairo docs: enum cairo_hint_style_t: Specifies the type of hinting to do on font outlines. Hinting is the process of fitting outlines to the pixel grid in order to improve the appearance of the result. Since hinting outlines involves distorting them, it also reduces the faithfulness to the original outline shapes. Not all of the outline hinting styles are supported by all font backends. enum cairo_hint_metrics_t Specifies whether to hint font metrics; hinting font metrics means quantizing them so that they are integer values in device space. Doing this improves the consistency of letter and line spacing, however it also means that text will be laid out differently at different zoom factors. Currently GtkSettings sets the default of CAIRO_HINT_METRICS_ON, which gives integer values in device space, instead of precise floating point values. you're doing the rendering correctly, but measuring is still integer based, which is the underlying bug here. I forgot to leave a note here before that the mozilla link is not relevant here, as Cairo actually does respect em_units and gives precise metrics, but only if CAIRO_HINT_METRICS_ is OFF, not when its ON. Mozilla uses FT directly, and thus has to take care of em handling on its own.
Martin Robinson
Comment 4 2011-11-18 08:40:14 PST
(In reply to comment #3) > Specifies whether to hint font metrics; hinting font metrics means quantizing them so that they are integer values in device space. Doing this improves the consistency of letter and line spacing, however it also means that text will be laid out differently at different zoom factors. Thanks for clearing this up. From the documentation it seems like turning this off carries a small regression in render quality, but an improvement in consistency?
Nikolas Zimmermann
Comment 5 2011-11-23 04:10:43 PST
Created attachment 116342 [details] Proposed way to get floating point metrics for Cairo The problem is not not handling m_unitsPerEm, that should be queried from the font. The real problem is that Cairo turns on hinting metrics - the attached patch gives an idea how to fix it. I didn't test on my own on Cairo, but Phil said that we correctly get floating point metrics then, see the discussion in bug 47467! Could any of the Gtk guys take over the work and test on their own whats the impact. Might be great to run new-run-webkit-tests --rest-results -p, then apply the patch, and make a comparision run.
Martin Robinson
Comment 6 2011-11-24 07:53:50 PST
(In reply to comment #5) > Created an attachment (id=116342) [details] > Proposed way to get floating point metrics for Cairo I did some smoke testing of this patch locally. It changes almost every single DRT result, but the changes it produces seem sane enough. In the layout test, at least, it results in smaller space between lines and more space between letters.
Philippe Normand
Comment 7 2011-11-25 04:17:02 PST
Created attachment 116596 [details] proposed patch ((c) WildFox)
Philippe Normand
Comment 8 2011-11-29 01:43:12 PST
Dominik Röttsches (drott)
Comment 9 2012-11-27 01:54:46 PST
Comment on attachment 116596 [details] proposed patch ((c) WildFox) View in context: https://bugs.webkit.org/attachment.cgi?id=116596&action=review > Source/WebCore/platform/graphics/freetype/SimpleFontDataFreeType.cpp:67 > + m_fontMetrics.setLineSpacing(lroundf(ascent) + lroundf(descent) + lroundf(lineGap)); Nikolas, can you explain a bit more about the rationale of rounding here? I know CoreGraphics does it, but Qt and Skia don't do this anymore, if I checked correctly. What do you think is the benefit? With subpixel layout and zooming, I think we should get rid of rounding in the backends as much as possible. In bug 102374, I am considering removing this rounding, so it'd be good to know if you have more background. Thanks!
Nikolas Zimmermann
Comment 10 2012-11-27 12:52:39 PST
(In reply to comment #9) > (From update of attachment 116596 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=116596&action=review > > > Source/WebCore/platform/graphics/freetype/SimpleFontDataFreeType.cpp:67 > > + m_fontMetrics.setLineSpacing(lroundf(ascent) + lroundf(descent) + lroundf(lineGap)); > > Nikolas, can you explain a bit more about the rationale of rounding here? I know CoreGraphics does it, but Qt and Skia don't do this anymore, if I checked correctly. What do you think is the benefit? With subpixel layout and zooming, I think we should get rid of rounding in the backends as much as possible. In bug 102374, I am considering removing this rounding, so it'd be good to know if you have more background. Thanks! Sorry it's been a while since I looked at this the last time. I only chose the way for consistency between the ports. Someone more familiar with OSX Text rendering might answer why this was a good idea back then. Is this still in use for CoreGraphics text rendering? (the lroundf() statements)
Note You need to log in before you can comment on or make changes to this bug.