Last Comment Bug 683618 - synthetic bolding on Android (gfxFT2Fonts) does not adjust glyph metrics properly
: synthetic bolding on Android (gfxFT2Fonts) does not adjust glyph metrics prop...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: Trunk
: ARM Android
: -- normal (vote)
: mozilla9
Assigned To: Jonathan Kew (:jfkthame)
:
Mentors:
http://people.mozilla.com/~jkew/synbo...
Depends on:
Blocks: 634997 674909
  Show dependency treegraph
 
Reported: 2011-08-31 09:00 PDT by Jonathan Kew (:jfkthame)
Modified: 2011-09-03 04:09 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch, move AdjustAdvancesForSyntheticBold to the right place (1.05 KB, patch)
2011-08-31 09:02 PDT, Jonathan Kew (:jfkthame)
jd.bugzilla: review+
Details | Diff | Splinter Review

Description Jonathan Kew (:jfkthame) 2011-08-31 09:00:19 PDT
When we use double-striking (with an offset) to simulate bold for font families that don't have a real bold face, we're supposed to add a corresponding delta to the glyph advances, so as to maintain reasonable inter-glyph spacing.

However, the code in gfxFT2Font::InitTextRun() calls AdjustAdvancesForSyntheticBold in the wrong place, with the result that it doesn't have any effect. This means that synthetic-bold glyphs appear crowded together, as can be seen in the test case at http://people.mozilla.com/~jkew/synbold.html; this is particularly noticeable when the page is zoomed out, so that the font size is small.
Comment 1 Jonathan Kew (:jfkthame) 2011-08-31 09:02:07 PDT
Created attachment 557202 [details] [diff] [review]
patch, move AdjustAdvancesForSyntheticBold to the right place

With this patch, the synthetic-bold text in the testcase looks much better and more readable, as it has proper inter-glyph spacing.
Comment 2 John Daggett (:jtd) 2011-08-31 13:14:30 PDT
Comment on attachment 557202 [details] [diff] [review]
patch, move AdjustAdvancesForSyntheticBold to the right place

Yeah, I was wondering about this when looking at bug 674909.
Comment 3 Jonathan Kew (:jfkthame) 2011-08-31 13:52:33 PDT
Incidentally, we already have a reftest that checks this (layout/reftests/text/synthetic-bold-metrics-01.html), but it's marked as failing on android - on a tryserver run with this patch, it "unexpectedly" passes. :)
Comment 4 Jonathan Kew (:jfkthame) 2011-09-02 13:26:53 PDT
Pushed to inbound, including updating the reftest manifest because the test now passes:
http://hg.mozilla.org/integration/mozilla-inbound/rev/c1281875cfea
Comment 5 Marco Bonardo [::mak] 2011-09-03 03:01:18 PDT
http://hg.mozilla.org/mozilla-central/rev/c1281875cfea

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