text spacing is bad in Fennec at non-1:1 zoom levels

VERIFIED FIXED

Status

()

Core
Graphics
--
major
VERIFIED FIXED
9 years ago
8 years ago

People

(Reporter: cmtalbert, Assigned: Stuart Parmenter)

Tracking

(Depends on: 1 bug, Blocks: 1 bug)

Trunk
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.2 +

Firefox Tracking Flags

(status1.9.2 final-fixed, fennec1.0+)

Details

(Whiteboard: [needs patch][fennec-only])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

9 years ago
Load any page into fennec that uses standard variable width fonts and note the kerning (space between the letters).  You'll see that letters are oddly spaced.

For example, on bugzilla.mozilla.org, you can note that the a's have too much space after them and the t's have too little (check out t's at the ends of words).  You can see the same effect on cnn.com.

If you zoom in, the kerning is repaired and looks normal.  But at the default zoom level, this makes pages hard to read.

1. Go to site (bugzilla.mozilla.org, cnn.com, etc)

== Actual ==
Kerning is messed up.

== Expected == 
Kerning is displayed properly.  Note that if you zoom in, the kerning is displayed properly, but returning to the default zoom level, the incorrect spacing returns.

I'll try to get some screen shots.

Marking blocking ? because if you can't easily read sites, fennec isn't useful.
Flags: blocking-fennec1.0?
(Reporter)

Updated

9 years ago
Severity: normal → major
(Reporter)

Comment 1

9 years ago
Doh: version: Mozilla/5.0 (X11; U; Linux armv6l; en-US;rv:1.9.2a1pre) Gecko/20081219 Fennec/1.0a2pre

sorry for spam
Blocks: 473596
(Assignee)

Updated

9 years ago
tracking-fennec: --- → 1.0+
OS: Linux (embedded) → All
Hardware: ARM → All
Duplicate of this bug: 458458
(Assignee)

Updated

8 years ago
Flags: blocking-fennec1.0?
Blocks: 504390
(Assignee)

Updated

8 years ago
Assignee: nobody → pavlov
Duplicate of this bug: 511958
Flags: wanted-fennec1.0?
Can someone verify this behavior on the latest nightly?  I tried bugzilla and theglobeandmail with no luck.  Has it been fixed?

Since we set zoom level to <1 on websites that have overflows at 800px width (like theglobeandmail and some urls on bugzilla), I wonder if this only occurs at zoom levels less than 1.
Duplicate of this bug: 517610
I think it's still an issue.  Here's what a globeandmail.com page looks like, zoomed in, with today's nightly:
http://www.flickr.com/photos/madhava_work/4034902494/sizes/o/
You can also see it when zooming into article snippets on www.google.com/news
Just noticed this isn't a problem on Mac!
Component: General → Graphics
Product: Fennec → Core
QA Contact: general → thebes
Flags: wanted-fennec1.0? → blocking1.9.2+
Jeff asked, and I'll answer here: I marked this blocking1.9.2+ because it was marked blocking-fennec:1.0+

It may have been fixed a while back; just don't want to lose track.

Comment 10

8 years ago
Is this something specific to fennec builds or happens also on Linux?  Windows?  Not sure the platform == all makes sense.  It would really help to have a testcase and some screenshots to investigate.
(Reporter)

Comment 11

8 years ago
I'd like more detail on what the approach is to fix this.

In the platform meeting [1] we discussed that a possible solution would be forking the file that controls this code.  I really really discourage that approach because once you fork that file, we'll lose the ability to test this change as part of the automated regression testing for the general 1.9.2 platform.  The fennec tests are not up to snuff to catch any regression issues that might occur as a result of the change.

What are the options we're considering here?  Is there a WIP patch we can take a look at?

[1]: https://wiki.mozilla.org/Platform/2009-11-24

Comment 12

8 years ago
(In reply to comment #11)
Clint, please set up a testcase with a page that uses a defined set of fonts that don't display correctly in a given environment.  Please note whether zooming is required or not to produce the problem.  If the problem is on fennec, does the same problem also occur under in a Linux environment?

A couple example test pages using downloadable fonts, these can easily be reworked to use platform fonts:

http://people.mozilla.org/~jdaggett/tests/otfttfkerning.html
http://people.mozilla.org/~jdaggett/tests/waterfall.html

Kerning is usually a post-process of adjusting the advance widths based on the surrounding characters but the screen shot in comment 6 looks really off, I don't think this is just a problem with handling kerning data, something else is off, even if the end result looks like a kerning problem.

Comment 13

8 years ago
Any ETA here?  This is at this point one of three remaining non-JS core blockers.... and has had no activity at all for a week.
Stuart's under the weather, but is working on the bug.

I hate to do it, but I'd like to revisit the blocking decision on it. Specifically, from looking at the problem with Madhava, I wonder if the gain from fixing kerning will be enough, or if the font appearance problem runs deeper than just kerning. There appeared to be differences in subpixel rendering when compared side by side with another device like the iPhone
I was asked for another example of the problem, so here:

http://www.flickr.com/photos/madhava_work/4151195104/

some particularly problematic areas called out with flickr notes
That honestly doesn't look bad to me -- especially since that's zoomed in.  Why would you ever zoom in that much to read text?
BTW, is the title of this bug inaccurate? Should it be "kerning incorrect when zoomed in" or is it referring to 100% zoom?
Here is CNN on the Maemo:

Default page load:
http://people.mozilla.com/~mfinkle/fennec/screenshots/fennec-kerning-defaultzoom.png

Double tap on a paragraph so I can read it:
http://people.mozilla.com/~mfinkle/fennec/screenshots/fennec-kerning-zoomin.png
(In reply to comment #17)
> BTW, is the title of this bug inaccurate? Should it be "kerning incorrect when
> zoomed in" or is it referring to 100% zoom?

Kerning us observed when zoomed out as well, as Mark points out.  By default we zoom to fit the page on the screen.
The CNN shots are certainly more illustrative, and zoomed out it affects small text readability :/

Are we sure kerning alone will fix this?
(Assignee)

Updated

8 years ago
Summary: kerning incorrect at default zoom level in Fennec → text spacing is bad in Fennec at non-1:1 zoom levels
(Assignee)

Comment 21

8 years ago
this bug has nothing really to do with kerning
The screenshots in comment 18 are both taken with hinting enabled.
How do things look with hinting off?
(This will need to be done for subpixel positioning anyway I assume, so would at least be a cheap partial solution?)
At what size is the text reflowed?
Stuart doesn't consider this a blocker, and I tend to agree, because we can ifdef this out on non-mobile platforms.
Flags: blocking1.9.2+ → blocking1.9.2?
(Assignee)

Comment 24

8 years ago
this is a 1.9.2 blocker, not a firefox 3.6 one.
Flags: blocking1.9.2? → blocking1.9.2+
Stuart's right, though if this becomes the last blocker for Firefox desktop, then we'll declare 1.9.2 complete and Stuart will put this in a 1.9.2.1 on which Firefox for Maemo will be based. Ah, the fun of branch mechanics!
Whiteboard: [needs patch][fennec-only]
(Assignee)

Comment 26

8 years ago
Created attachment 419506 [details] [diff] [review]
disable hinting for mobile
Attachment #419506 - Flags: review?(vladimir)
Comment on attachment 419506 [details] [diff] [review]
disable hinting for mobile

In PrepareSortPattern():

>@@ -1746,6 +1746,14 @@
>     }
> #endif
> 
>+#ifdef MOZ_GFX_OPTIMIZE_MOBILE
>+    cairo_font_options_t *options = cairo_font_options_create();
>+    cairo_font_options_set_hint_style (options, CAIRO_HINT_STYLE_NONE); // not sure if this takes effect here or not

>+    cairo_font_options_set_hint_metrics (options, CAIRO_HINT_METRICS_OFF); // this doesn't take effect here

Yes, this does nothing here.  Don't bother with this line.

>+    cairo_ft_font_options_substitute(options, aPattern);

Docs for cairo_ft_font_options_substitute say "Options that are already in the
pattern, are not overridden".  In fact, in some options will get overridden
sometimes but that is a bug.
Really you want this before ApplyGdkScreenFontOptions().
Fortunately, CAIRO_HINT_STYLE_NONE will not be affected (overridden) by the
bug.

What might be most clear, though, is to only change
ApplyGdkScreenFontOptions() (instead of PrepareSortPattern()) to add
cairo_font_options_copy() and cairo_font_options_set_hint_style (options,
CAIRO_HINT_STYLE_NONE) after gdk_screen_get_font_options() (with a later
destroy).

In CreateScaledFont():

>@@ -2520,7 +2528,11 @@
>     // font will be used, but currently we don't have different gfxFonts for
>     // different surface font_options, so we'll create a font suitable for the
>     // Screen. Image and xlib surfaces default to CAIRO_HINT_METRICS_ON.
>+#ifdef MOZ_GFX_OPTIMIZE_MOBILE
>+    cairo_font_options_set_hint_metrics (fontOptions, CAIRO_HINT_METRICS_OFF);
>+#else
>     cairo_font_options_set_hint_metrics(fontOptions, CAIRO_HINT_METRICS_ON);
>+#endif

Looks good.

>+#ifndef MOZ_GFX_OPTIMIZE_MOBILE
>     FcBool hinting;
>     if (FcPatternGetBool(aPattern, FC_HINTING, 0, &hinting) != FcResultMatch) {
>         hinting = FcTrue;
>@@ -2578,6 +2591,9 @@
> #endif
>     }
>     cairo_font_options_set_hint_style(fontOptions, hint_style);
>+#else
>+    cairo_font_options_set_hint_style(fontOptions, CAIRO_HINT_STYLE_NONE);
>+#endif

This shouldn't usually be necessary as this is already set in
PrepareSortPattern()/ApplyGdkScreenFontOptions().  It will override user
fontconfig settings if users want to force something different (after
PrepareSortPattern has set the property).  I'll let you choose whether you
want to keep this or not.
(Assignee)

Comment 28

8 years ago
Created attachment 419530 [details] [diff] [review]
address review comments
Attachment #419506 - Attachment is obsolete: true
Attachment #419530 - Flags: review?(karlt)
Attachment #419506 - Flags: review?(vladimir)
Attachment #419530 - Flags: review?(karlt) → review+
(Assignee)

Comment 29

8 years ago
pushed.  this fixes this bug well enough to close it out, will do rest of subpixel work in another bug.

http://hg.mozilla.org/mozilla-central/rev/1759d1ad0ce1
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/369720551307
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
this looks a lot better with the 20091230 builds on my n810 and n900.
Status: RESOLVED → VERIFIED
status1.9.2: --- → final-fixed
Depends on: 537411
Blocks: 540788
You need to log in before you can comment on or make changes to this bug.