The default bug view has changed. See this FAQ.

no synthetic bolding on Android

RESOLVED FIXED in Firefox 8

Status

()

Core
Graphics
P3
normal
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: tarend, Assigned: padenot)

Tracking

Trunk
mozilla8
ARM
All
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox8 fixed, blocking2.0 -, fennec8+)

Details

(URL)

Attachments

(4 attachments, 1 obsolete attachment)

(Reporter)

Description

6 years ago
See attached screenshot

(1) go to m.spiegel.de (and other sites like GNews, etc.) on a Samsung Galaxy Tab and compare it to other mobile devices and our desktop build

Result:
Font rendering is different, not as smooth, bold face is missing 
Expected: rendering should be the same

Also: comparing Fennec with stock browser shows that our image (and font) rendering is sub-par: we show far more artifacts.
(Reporter)

Comment 1

6 years ago
Created attachment 513207 [details]
Font rendering on Galaxy Tab vs. Nexus 1
other than the lack of bold, the rendering differences aren't immediately obvious to me with these screenshots. Perhaps some better shots would be useful?

http://blog.lassey.us/2010/09/13/android-screen-shots-without-rooting-your-phone/
(Reporter)

Comment 3

6 years ago
Font type and kerning seem slightly different, the visual appearance of the pages are definitely different. I will *try* to capture better screenshots. Or you can try it on any Galaxy Tab.
If different fonts are being used, than some differences are to be expected. Let's see if we can get the fonts list. We might send that to logcat.
(In reply to comment #3)
> Font type and kerning seem slightly different, the visual appearance of the
> pages are definitely different. I will *try* to capture better screenshots. Or
> you can try it on any Galaxy Tab.

I don't have a Galaxy Tab, nor do any of the "font people" at mozilla as far as I know. If you follow the instructions I linked to, you'll get more useful screen shots.

Also, as Mark said, if the two devices has a different set of fonts, this is to be expected. Also, differences between the fonts the stock browser uses and the fonts we use may be explained by bug 633109, but that's pure conjecture.
This may be relevant: In the about config, doing a search under font.name.* , I found that it lists Droid Sans for some languages on my droid 2.  Perhaps we can confirm that there are different fonts being used on the two devices by knowing what font names are being used in the fennec preferences?
On start up we echo what font files we load to the logcat, as Mark said if you can post that, we'll know what fonts are being loaded
Here's is the log from my Verizon Galaxy Tab (Android 2.2, not rooted or modified):

I/Gecko   ( 3449): Font: /system/fonts/DroidSerif-Bold.ttf
I/Gecko   ( 3449): font family: Droid Serif
I/Gecko   ( 3449): Font: /system/fonts/DroidSansFallback.ttf
I/Gecko   ( 3449): font family: Droid Sans Fallback
I/Gecko   ( 3449): Font: /system/fonts/Clockopia.ttf
I/Gecko   ( 3449): font family: Clockopia
I/Gecko   ( 3449): Font: /system/fonts/DroidSans-Bold.ttf
I/Gecko   ( 3449): font family: Droid Sans
I/Gecko   ( 3449): Font: /system/fonts/Georgia.ttf
I/Gecko   ( 3449): font family: Georgia
I/Gecko   ( 3449): Font: /system/fonts/DroidSansMono.ttf
I/Gecko   ( 3449): font family: Droid Sans Mono
I/Gecko   ( 3449): Font: /system/fonts/DroidSans_Subset.ttf
I/Gecko   ( 3449): font family: Droid Sans
I/Gecko   ( 3449): Font: /system/fonts/DroidSansHebrew.ttf
I/Gecko   ( 3449): font family: Droid Sans Hebrew
I/Gecko   ( 3449): Font: /system/fonts/DroidSansArabic.ttf
I/Gecko   ( 3449): font family: Droid Sans Arabic
I/Gecko   ( 3449): Font: /system/fonts/Verdana.ttf
I/Gecko   ( 3449): font family: Verdana
I/Gecko   ( 3449): Font: /system/fonts/Comic.ttf
I/Gecko   ( 3449): font family: Comic Sans MS
I/Gecko   ( 3449): Font: /system/fonts/DroidSerif-Italic.ttf
I/Gecko   ( 3449): font family: Droid Serif
I/Gecko   ( 3449): Font: /system/fonts/DroidSerif-Regular.ttf
I/Gecko   ( 3449): font family: Droid Serif
I/Gecko   ( 3449): Font: /system/fonts/DroidSansThai.ttf
I/Gecko   ( 3449): font family: Droid Sans Thai
I/Gecko   ( 3449): Font: /system/fonts/DroidSans.ttf
I/Gecko   ( 3449): font family: Droid Sans
I/Gecko   ( 3449): Font: /system/fonts/DroidSerif-BoldItalic.ttf
I/Gecko   ( 3449): font family: Droid Serif
I/Gecko   ( 3449): Font: /system/fonts/DroidSansMono_EBook.ttf
I/Gecko   ( 3449): font family: Droid Sans Mono
Nexus One (Android 2.3.3) , raw output from logcat

I/GeckoFonts( 8254): got: /system/fonts/DroidSansMono.ttf;droid sans mono,;1266978337;117072;0,;/system/fonts/DroidSerif-Regular.ttf;droid serif,;1266978337;172532;0,;/system/fonts/DroidSerif-Bold.ttf;droid serif,;1266978337;184836;0,;/system/fonts/DroidSansHebrew.ttf;droid sans hebrew,;1217592000;23076;0,;/system/fonts/DroidSerif-BoldItalic.ttf;droid serif,;1266978337;189916;0,;/system/fonts/DroidSansThai.ttf;droid sans thai,;1217592000;36028;0,;/system/fonts/DroidSansArabic.ttf;droid sans arabic,;1217592000;35908;0,;/system/fonts/DroidSerif-Italic.ttf;droid serif,;1266978337;177176;0,;/system/fonts/DroidSans.ttf;droid sans,;1266978337;190044;0,;/system/fonts/DroidSans-Bold.ttf;droid sans,;1266978337;191032;0,;/system/fonts/Clockopia.ttf;clockopia,;1266978337;6880;0,;/system/fonts/DroidSansFallback.ttf;droid sans fallback,;1279142112;3640264;0,; from the cache
OS: Android → All
Created attachment 515683 [details]
test case
It looks like the difference is that the Galaxy Tab includes a version of the Verdana font family, but Fennec fails to render Verdana in bold or italic styles.

On the Nexus One, there are no Verdana fonts, so Fennec correctly uses a fallback font instead.

Opera Mobile appears to render Verdana correctly (including bold and italic), while the stock browser on the Galaxy Tab appears to fall back and use Droid Sans instead of Verdana.
Summary: Different font rendering on Nexus One and Galaxy Tab → Incorrect rendering of Verdana bold/italic fonts on Galaxy Tab
It seems the problem is that the Galaxy Tab ships Verdana, but no Verdana Bold.  (The Nexus One and most other Android phones do not ship Verdana at all.)

Can we detect this situation, and refuse to use the font if there is no bold variant?  If not, should we just blacklist Verdana on Android?  (The default Droid Sans which ships on all Android devices is a reasonable substitute for Verdana.)
Summary: Incorrect rendering of Verdana bold/italic fonts on Galaxy Tab → Incorrect rendering of Verdana bold font on Galaxy Tab
as far as I know we should double strike the normal font for bold
(Reporter)

Updated

6 years ago
blocking2.0: --- → ?
tracking-fennec: --- → ?
Component: General → Graphics
Product: Fennec → Core
QA Contact: general → thebes
jdaggett, any idea why we're not double striking these fonts?
Not blocking Macaw
blocking2.0: ? → -
tracking-fennec: ? → 2.0-
Whiteboard: [fennec-6]
tracking-fennec: - → 6+
Whiteboard: [fennec-6]
jdaggett?

Comment 17

6 years ago
(In reply to comment #14)
> jdaggett, any idea why we're not double striking these fonts?

Argh, more Samsung font wackiness.  What's in the water they're drinking? ;)

The double-striking behavior for bolding isn't automatic, you need to explicitly enable it in the gfxFont subclass.  We only do this on OSX currently, with GDI/DirectWrite it's done natively.  So the FT2 font subclass of gfxFont needs to mimic the gfxMacFont code:

http://mxr.mozilla.org/mozilla-central/source/gfx/thebes/gfxMacFont.cpp#61

Looks like you need to pass in the 'aNeedsBold' boolean in the constructor for that to work.  We should also verify that synthetic italics is also working (again, use the Mac font code as a guide).

As Matt notes in comment 11, this problem is not specific to either Verdana or the Tab per se, it's simply that we're seeing this problem because Samsung (unwisely) chose to ship font families that lack bold/italic faces (Georgia will probably have the same problem).
Summary: Incorrect rendering of Verdana bold font on Galaxy Tab → no synthetic bolding on Android

Comment 18

6 years ago
One thing I don't understand is why the reftest below passes on Android (reftests/font-face):

http://people.mozilla.org/~jdaggett/font-face/synthetic-variations.html

This test should catch the problem this bug details.
(Assignee)

Comment 19

6 years ago
Created attachment 537570 [details]
jdaggett test case on Motorola XOOM

Here is a screenshot of jdaggett's test case, on a Motorola XOOM running Fennec Nightlies. It doesn't show any bold. If this is related, I guess I could work on this, having a device that reproduce the bug.
Paul - It would be great if you could start to work on this bug.
tracking-fennec: 6+ → 8+
(Assignee)

Updated

6 years ago
Assignee: nobody → paul
(Assignee)

Comment 21

6 years ago
Created attachment 546543 [details] [diff] [review]
Patch v0 -- Adds synthetic bolding support on Android

Here is a patch, which show a correct behavior with mbrubeck attached test case, as well as the test case provided by jdaggett in comment 18. I've tested m.spiegel.de too, and it seems to work.

I've used a Samsung Galaxy S (stock, not rooted nor modified) to test, and it was failings all the aforementioned test cases.

The patch basically ports the behavior of the Mac version, as mentioned in comment 17.
Attachment #546543 - Flags: review?(jdaggett)
(Assignee)

Updated

6 years ago
Status: NEW → ASSIGNED

Comment 22

6 years ago
Comment on attachment 546543 [details] [diff] [review]
Patch v0 -- Adds synthetic bolding support on Android

Looks good, the only problem I see is that your patch has a bunch of tabs in it.  Please remove all tabs and verify the spacing before landing.
Attachment #546543 - Flags: review?(jdaggett) → review+
(Assignee)

Comment 23

6 years ago
Created attachment 548422 [details] [diff] [review]
Patch v1 -- Replaced tabs by spaces

Here is the patch with spacing fixed.

I took the liberty to remove an XXX comment saying that this bug should be fixed.
Attachment #546543 - Attachment is obsolete: true
(Assignee)

Updated

6 years ago
Keywords: checkin-needed
I tried to apply this patch and got:
applying attachment.cgi?id=548422
patching file gfx/thebes/gfxAndroidPlatform.cpp
Hunk #1 FAILED at 636
1 out of 1 hunks FAILED -- saving rejects to file gfx/thebes/gfxAndroidPlatform.cpp.rej
patching file gfx/thebes/gfxFT2Fonts.cpp
Hunk #1 FAILED at 105
Hunk #2 FAILED at 691
Hunk #3 FAILED at 797
Hunk #4 FAILED at 871
4 out of 4 hunks FAILED -- saving rejects to file gfx/thebes/gfxFT2Fonts.cpp.rej
patching file gfx/thebes/gfxFT2Fonts.h
Hunk #1 FAILED at 120
1 out of 1 hunks FAILED -- saving rejects to file gfx/thebes/gfxFT2Fonts.h.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir
errors during apply, please fix and refresh attachment.cgi?id=548422
(Assignee)

Comment 25

6 years ago
Blassey, this patch applies fine on my clean, updated tree.
http://hg.mozilla.org/mozilla-central/rev/17835995c2d2
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
(Reporter)

Updated

6 years ago
Blocks: 674909
(Reporter)

Comment 27

6 years ago
I see bold font now, but it doesn't render consistently - I filed new Bug 634997 for the issue.
status-firefox8: --- → fixed

Comment 28

6 years ago
This patch broke a tier 1 platform. Why was it not backed out? Please do so.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(In reply to Andreas Gal :gal from comment #28)
> This patch broke a tier 1 platform. Why was it not backed out? Please do so.

what did it break?

Comment 30

6 years ago
"I see bold font now, but it doesn't render consistently - I filed new Bug 634997 for the issue."

Comment 31

6 years ago
Actually the wrong bug number is quoted there. Its 674909. Bold fonts look completely messed up on Samsung devices and I think italic fonts are broken too.
(In reply to Andreas Gal :gal from comment #30)
> "I see bold font now, but it doesn't render consistently - I filed new Bug
> 634997 for the issue."

that's not a regression. Before this landed there were no bold fonts for these devices. Now there are. You're seeing the limits on how good synthetic bold can be, or at least the limits of our implementation of it. If there are things we can do to improve that, we should look at them, but that's a different bug.
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED

Comment 33

6 years ago
We went from a visual glitch to a barely readable state. I strongly disagree with taking this patch as is, but I am not driving here so your call.
That being said, I wonder if the synthetic bolding is required anymore since bug 636042 landed. Was this just one bad font that was causing the issue on these devices?

Can someone with an effected device do a build with this backed out?
It doesn't matter if a change exposed latent bugs that then bit hard, or directly regressed something -- regression is as regression does. I'm with Andreas: if we are serious about tier 1 status, nightly usability bugs like this should lead to a back-out.

Backing out is done all the time on tier-1 plaform regression. Hg makes it easy enough. It's the right thing.

/be

Comment 36

6 years ago
I will try to test #34 and report back.
Brendan, from what I've been told until today the current behavior of synthetically bolding the fonts is preferable to not bolding them at all. Andreas seems to disagree, we should figure out what we can do.

Comment 38

6 years ago
Due to the problems noted on bug 674909, I think we should back this one out and rework the patch to solve the underlying problem, namely that the current double-stamping approach to fake bolding doesn't correctly account for transforms applied to the current gfxContext.  This is a problem that exists anywhere we use the double-stamping logic (e.g. OSX) for synthetic bolding.  See bug 674909, comment 18 for more details.

The reason this only shows up on some devices is that fonts are usually packaged in regular-bold-italic-bold-italic combinations where synthetic bolding isn't required.  Samsung, for whatever reason, decided to ship just the regular face of Verdana with no bold or italic faces, that's why the problem shows up on these devices.

Bug 636042 is unrelated to this issue.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Depends on: 683618
Comment on attachment 548422 [details] [diff] [review]
Patch v1 -- Replaced tabs by spaces

Review of attachment 548422 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/thebes/gfxFT2Fonts.cpp
@@ +696,5 @@
>                                            aRunStart, aRunLength, aRunScript);
>      }
>  
>      if (!ok) {
> +        aTextRun->AdjustAdvancesForSyntheticBold(aRunStart, aRunLength);

This is the wrong place - we need to call AdjustAdvancesForSyntheticBold *after* the glyphs for the run have been set up with their normal metrics. And we need to apply it in the harfbuzz-shaped case (which is the usual codepath) as well. So it should go at the end of the function, right before return PR_TRUE.

Filed bug 683618 as a followup to fix this.
Closing this, as we didn't end up backing it out, and the followup issues (bug 674909, bug 683618) have now been fixed as well.
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.