403513 - bad kerning in print output if hinting is set to "medium" or "full"

Status: RESOLVED FIXED

(Core :: Graphics, defect, P1)

Description

[Ilja Sekler]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b2pre) Gecko/2007111211 Firefox/3.0b2pre
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b2pre) Gecko/2007111211 Firefox/3.0b2pre

The current trunk builds of Firefox print some widely used fonts like Arial and especially Times New Roman with obviously wrong kerning. For example, the gaps after the capital letter "M" are way too small with Arial and way too large with Times New Roman. Other oddities: a too big gap after "p" and after "y", a too small gap after "g" with Times New Roman and after the cyrillic letter "ж" with Verdana.

Firefox 2 builds are OK.

Reproducible: Always

Steps to Reproduce:
1. install Microsoft truetype fonts and set Times New Roman as the default font for serif and Arial for sans-serif

2. open a web page that specifies these fonts (or none) for printing and print it, preferably to a PDF file
Actual Results:  
Bad kerning in the printed page

Expected Results:  
Good kerning or something similar to the nice printing output of Firefox 2.0

I never saw better printing after the switch to cairo, so I don't classify it as a regression on my own.

The problem doesn't exist on Windows.

Comment 1

[Ilja Sekler]

Attachment: a small illustration of the problem with different fonts

Comment 2

[Ilja Sekler]

Attachment: a PDF print of the illustration above

Comment 3

[Ilja Sekler]

Attachment: the same with Firefox 2.0.0.8 (Ubuntu Gutsy)

Comment 4

[Ilja Sekler]

I've missed an important point. It is mandatory to set font rendering to "monochrome" to disable anti-aliasing in order to be able to reproduce this bug. The summary changed accordingly.

Modifying local font rendering options really shouldn't damage printing.

Comment 5

[Ilja Sekler]

The third iteration to describe this bug properly, my apologies.

The anti-aliasing method doesn't matter, Microsoft fonts don't matter, but hinting matters. Adjusting the summary once again.

Better steps to reproduce:

================================================================================

1. Set hinting with gnome-appearance-properties to "Medium" or "Full"

2. Start firefox, open http://www.berlinonline.de/berliner-zeitung/print/feuilleton/709619.html?_=print and print the page to a file, close Firefox

3. Only for better visibility: convert the file to PDF using ps2pdf command

4. Disable hinting or set it to "Slight"

5. Repeat the steps 2. and 3.

6. Open the PDF files and compare the kerning.

================================================================================

Actual results: bad kerning in the first print, good kerning in the second one.

Expected results: good kerning in both prints.

Tested with current trunk builds and with 3.0b2 RC1 on an installed Ubuntu 7.10, on Ubuntu 8.04 alpha live CD and on Fedora 8 live CD.

Comment 6

[Matthew Middleton (:zzxc)]

I can confirm this bug and this workaround in the latest trunk build with Ubuntu Gutsy.  I will test this later on my Gentoo machine, to see if this is tied to Ubuntu's version of freetype.

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

This is not related to the print dialog. It is almost certainly cairo/Pango/gfx stuff.

Comment 8

[Karl Tomlinson (:karlt)]

Looks like a consequence of positioning characters based on the display resolution (and hint settings) but drawing glyphs hinted (or maybe not hinted) to printer (or some other) resolution.

Reflowing with the printer resolution would produce nice looking text (but might produce some position-change surprises).

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

For printing we shouldn't be using display settings, we definitely should be using print metrics. I can see if the print DPI is the same as the screen DPI we might accidentally get hits for screen textruns in the textrun cache, but otherwise we shouldn't. Not sure what's going on here.

Comment 10

[Karl Tomlinson (:karlt)]

I wonder whether using gdk_pango_context_get() is a problem here.

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/gfx/thebes/src/gfxPangoFonts.cpp&rev=1.128&mark=436,1227#426

Comment 11

[Karl Tomlinson (:karlt)]

pango_cairo_update_context() may be what is needed.

http://library.gnome.org/devel/pango/unstable/pango-Cairo-Rendering.html#pango-cairo-update-context

Comment 12

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

roc/karl, do we want to block on this?  Any idea what a fix would be?

Comment 13

[Karl Tomlinson (:karlt)]

gfxFontGroups (or gfxFontStyles) should depend on the cairo_t context to include information about whether hinting is enabled (and maybe non-scale transformations).

Might be a bit much of a change for 1.9.

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Nice to have for 1.9 but we can't block on this.

Comment 15

[Ricardo Pérez López]

I can confirm this bug as well in my Ubuntu 8.04 using Firefox 3.0rc1.

Comment 16

[Ricardo Pérez López]

This bug is not fixed yet in 3.0.1. Any news about it? It's a very annoying issue which does printing on Linux almost unusable in many situations.

Comment 17

[Karl Tomlinson (:karlt)]

I was wondering about fixing this by measuring at a resolution suitable for the printer.  That would be a bit of a hack but perhaps simple.

Apparently the reason why we don't do that is that widgets with GTK themes don't look right.  But I'm not sure that it makes a lot of sense to use the screen's theme for printing documents, and wonder whether disabling native themes for printing would be better anyway.

Comment 19

[Martin Stránský [:stransky] (ni? me)]

PangoContext for printing can be obtained by this code (it's from firefox-pango-printing.patch)

static void
default_substitute (FcPattern *pattern,
                    gpointer   data)
{
  FcPatternDel (pattern, FC_HINTING);
  FcPatternAddBool (pattern, FC_HINTING, 0);
}

static PangoFontMap *
get_fontmap (void)
{
  static PangoFontMap               *fontmap = NULL;

  if (!fontmap) {
    fontmap = pango_ft2_font_map_new ();
    pango_ft2_font_map_set_resolution ((PangoFT2FontMap *)fontmap, 72., 72.);
    pango_ft2_font_map_set_default_substitute ((PangoFT2FontMap *)fontmap, default_substitute, NULL, NULL);
  }

  return fontmap;
}

static PangoContext *
get_context (void)
{
  return pango_ft2_font_map_create_context ((PangoFT2FontMap *) get_fontmap ());
}

Another solution is to use GtkPrintOperation and derive the PangoContext for printing from draw-page signal (by gtk_print_context_create_pango_context())
(http://library.gnome.org/devel/gtk/stable/gtk-High-level-Printing-API.html)

Comment 20

[Martin Stránský [:stransky] (ni? me)]

After some digging it looks like the information about different font spacing (screen versus printing) could be stored in gfxFontStyle. Otherwise the screen/print fonts can be mixed in gfxFontCache which is global for all fonts and we can get printer spaced fonts for screen device (via. GetOrMakeFont()).

Comment 21

[Karl Tomlinson (:karlt)]

(In reply to comment #20)
> After some digging it looks like the information about different font spacing
> (screen versus printing) could be stored in gfxFontStyle.

Yes, I think the gfxFontStyle is a sensible place to add such information.
The main issue is getting this information to the gfxFontGroup.

Once the information is on the font group, it's just a matter of setting the font rendering options according to that information.  The existing PangoFontMap can be used for both screen/print fonts, provided that it is given this information.

Comment 22

[Martin Stránský [:stransky] (ni? me)]

Attachment: printerFont options added to gfxFontStyle

This patch adds printerFont to gfxFontStyle. But the pango_cairo_context_set_resolution() in GetPangoContext() doesn't work well here and the output is still broken, no idea why...

Comment 23

[Karl Tomlinson (:karlt)]

(In reply to comment #22)
> This patch adds printerFont to gfxFontStyle.

Thanks.

> But the pango_cairo_context_set_resolution() in GetPangoContext()
> doesn't work well here

pango_cairo_context_set_resolution() only works when the PangoContext is a PangoCairoContext, which this one is not.

But I don't think the resolution will have any effect here.

> and the output is still broken, no idea why...

I think you just need to pass this information to PrepareSortPattern() and then, for printing, replace gdk_screen_get_font_options with something appropriate: 

    cairo_font_options_t *options = cairo_font_options_create()
    cairo_font_options_set_hint_style (options, CAIRO_HINT_STYLE_NONE);
    cairo_font_options_set_antialias (options, CAIRO_ANTIALIAS_GRAY);

You may want this in CreateScaledFont(), but it shouldn't be essential, and I don't know if it makes any difference.

    cairo_font_options_set_hint_metrics (fontOptions, CAIRO_HINT_METRICS_OFF);

Comment 24

[Martin Stránský [:stransky] (ni? me)]

Attachment: an updated patch

Thanks, here is the updated patch.

Comment 25

[Karl Tomlinson (:karlt)]

Comment on attachment 351366 [details] [diff] [review]
an updated patch

This looks very close to ready, and it would be great to get this into 1.9.1.
 
+PRBool nsThebesDeviceContext::IsPrinterSurface(void)
+{
+  return(mPrintingSurface &&
+        (mPrintingSurface->GetType() == gfxASurface::SurfaceTypePDF ||
+         mPrintingSurface->GetType() == gfxASurface::SurfaceTypePS ||
+         mPrintingSurface->GetType() == gfxASurface::SurfaceTypeQuartz));
+}
+
 nsresult
 nsThebesDeviceContext::SetDPI()

Just testing mPrintingSurface would be more appropriate here.
There are printing surfaces that are not one of these three types.

File style is to place the return value type "PRBool" on its own line,
and to leave out the "void" from the parameter list.

 PrepareSortPattern(FcPattern *aPattern, double aFallbackSize,
-                   double aSizeAdjustFactor)
+                   double aSizeAdjustFactor, PRBool printerFont)

File style is to have parameter names starting with a (argument), and names
for boolean variables typically include a verb so that the name has a boolean
context.  e.g. aIsPrinterFont or aUsePrinterFont.

-    const cairo_font_options_t *options =
-        gdk_screen_get_font_options(gdk_screen_get_default());
+    
+    cairo_font_options_t *options = NULL;
+
+    if(!printerFont) {
+        options = (cairo_font_options_t *)gdk_screen_get_font_options(gdk_screen_get_default());
+    } else {
+    	options = cairo_font_options_create();
+    	cairo_font_options_set_hint_style (options, CAIRO_HINT_STYLE_NONE);
+    	cairo_font_options_set_antialias (options, CAIRO_ANTIALIAS_GRAY);
+    }
 
     cairo_ft_font_options_substitute(options, aPattern);

A cairo_font_options_destroy is required on the options for printing.

The cairo_ft_font_options_substitute from Mozilla's cairo that I had here for
the screen options wasn't quite right and has been fixed up in bug 462798.
However, using Mozilla's cairo_ft_font_options_substitute is right for the
printing options as the cairo_font_options_t is created by Mozilla's
cairo_font_options_create.

What this means is that PrepareSortPattern only needs to handle printing
options and ApplyGdkScreenFontOptions will take care of screen options.
(This also removes any need to care about the const/non-const issue.)

-    PrepareSortPattern(pattern, size, 1.0);
+    PrepareSortPattern(pattern, size, 1.0, IsPrinterFont(context));

+#define PRINTER_FONT_KEY "PrinterFontKey"
+
+static void
+SetPrinterFont(PangoContext *context, PRBool printerFont)
+{
+    g_object_set_data(G_OBJECT(context),PRINTER_FONT_KEY,(void *)printerFont);
+}
+

+    SetPrinterFont(context,mStyle.printerFont);
+
     // we should set this to null if we don't have a text language from the page...
     // except that we almost always have something...
     pango_context_set_language(context, mPangoLanguage);
     SetFontGroup(context, this);

The SetFontGroup here means that printerFont is available (without setting any
further data) through something like:

    gfxPangoFontGroup *fontGroup = GetFontGroup(context);
    PRBool usePrinterFont = fontGroup && fontGroup->GetStyle()->printerFont;

(Using g_object_set_data may be necessary if back-porting to 1.9.0 though.)

+++ mozilla-central/layout/svg/base/src/nsSVGGlyphFrame.cpp	2008-12-03 16:57:43.000000000 +0100
     gfxFontStyle fontStyle(font.style, font.weight, textRunSize, langGroup,
                            font.sizeAdjust, font.systemFont,
-                           font.familyNameQuirks);
+                           font.familyNameQuirks,
+                           PR_FALSE);

+++ mozilla-central/content/canvas/src/nsCanvasRenderingContext2D.cpp	2008-12-03 16:57:43.000000000 +0100
     gfxFontStyle style(fontStyle->mFont.style,
                        fontStyle->mFont.weight,
                        NSAppUnitsToFloatPixels(fontSize, aupcp),
                        langGroup,
                        fontStyle->mFont.sizeAdjust,
                        fontStyle->mFont.systemFont,
-                       fontStyle->mFont.familyNameQuirks);
+                       fontStyle->mFont.familyNameQuirks,
+                       PR_FALSE);

printerFont should be set according to the device context which is available
from presContext->DeviceContext().

Comment 26

[Martin Stránský [:stransky] (ni? me)]

Attachment: the updated one

Thanks for the review, this one should address the remarks.

Note: I've derived the printFont options from nsPresContext:

PRBool printerFont = (presContext->Type() == nsPresContext::eContext_PrintPreview || presContext->Type() == nsPresContext::eContext_Print);

because presContext->DeviceContext() returns generic device context and I don't see any simple way how to get the print options from it. Retype it to ThebesContext or derive it from DPI/Scale looks like an ugly hack...

Comment 27

[Karl Tomlinson (:karlt)]

Comment on attachment 352118 [details] [diff] [review]
the updated one

(In reply to comment #26)
> Note: I've derived the printFont options from nsPresContext

That's better than what I suggested, thanks.

-    if (mPrintingSurface &&
+    if (IsPrinterSurface() && 
         (mPrintingSurface->GetType() == gfxASurface::SurfaceTypePDF ||

I'd prefer testing mPrintingSurface directly because it makes it clearer that
GetType won't be called on a NULL pointer.

-    const cairo_font_options_t *options =
-        gdk_screen_get_font_options(gdk_screen_get_default());
-
-    cairo_ft_font_options_substitute(options, aPattern);
+    if(aIsPrinterFont) {
+    	cairo_font_options_t *options = cairo_font_options_create();
+    	cairo_font_options_set_hint_style (options, CAIRO_HINT_STYLE_NONE);
+    	cairo_font_options_set_antialias (options, CAIRO_ANTIALIAS_GRAY);
+       cairo_ft_font_options_substitute(options, aPattern);
+       cairo_font_options_destroy(options);
+    }

There are tabs in here that need to be replaced with 4 space indentation (and check there are no tabs elsewhere too).

This will make more sense when merged with the changes from bug 462798.
Then the following will be needed for screen options:

#ifdef MOZ_WIDGET_GTK2
    else {
        ApplyGdkScreenFontOptions(aPattern);
    }
#endif

r+=me with those changes.

Comment 28

[Martin Stránský [:stransky] (ni? me)]

Attachment: here is the patch against latest mozilla-central

This one should address the changes...

Comment 29

[Ilja Sekler]

Sorry for bugspam, but I just can't help saying "Thank you!". The patch works fine in Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2a1pre) Gecko/20081210 Firefox/3.2a1pre (SourceStamp=1eae54a0f48c+).

Comment 30

[Karl Tomlinson (:karlt)]

Comment on attachment 352345 [details] [diff] [review]
here is the patch against latest mozilla-central

I pushed this but needed to back out due to a compilation error in gfxFontSelectionTest.cpp.  Looks like a gfxFontStyle constructor needs updating:

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1229474897.1229475100.9701.gz#err0

Comment 31

[Karl Tomlinson (:karlt)]

Attachment: with necessary changes to gfx/thebes/test

Comment 32

[Karl Tomlinson (:karlt)]

Comment on attachment 353328 [details] [diff] [review]
with necessary changes to gfx/thebes/test

http://hg.mozilla.org/mozilla-central/rev/02d13fa61d8b

Would be nice to have this on 1.9.1.

Comment 33

[Karl Tomlinson (:karlt)]

Fixed on mozilla-central.

Comment 35

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 353328 [details] [diff] [review]
with necessary changes to gfx/thebes/test

a191=beltzner

Comment 36

[Karl Tomlinson (:karlt)]

Fixed on 1.9.1, thank you:
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/71ab93d0d75c

Comment 39

[Ricardo Pérez López]

The bug is fixed for xulrunner 1.9.1. Any chances of backporting the fix to the current 1.9.0.1, in order to get the bug fixed for Firefox 3.0.x?

Comment 40

[Karl Tomlinson (:karlt)]

(In reply to comment #39)
> Any chances of backporting the fix to the
> current 1.9.0.1, in order to get the bug fixed for Firefox 3.0.x?

The font code is sufficiently different between 1.9.0 and 1.9.1 that parts of the fix would need to be very different.  Porting fixes like this to 1.9.0 tends to be very rare, due to issues like:
http://www.computerworld.com/action/article.do?command=viewArticleBasic&articleId=9132208

Comment 42

[Tom Alsberg]

In a newly installed Firefox 3.5, I still notice the same problems.  Except, that now it no longer seems to be affected by the GNOME font settings, but by the fontconfig settings instead (also for the on-screen effects, not only for the print kerning problem).

Shouldn't Firefox 3.5 have this fixed - as it is using xulrunner 1.9.1 (which has the patch in)?

Comment 43

[Tom Alsberg]

This actually makes this even worse for me:

Previously (in Firefox 3.0) I would work around this problem by briefly setting the GNOME hinting (which affects running programs immediately) to "slight" just for the time of printing, while now in Firefox 3.5 (using fontconfig for those settings) I know of no easy way to change the font hinting temporarily on a running Firefox.

With hinting set to slight, fonts look really bad (very blurred) and are disturbing to read on screen, but print OK.  With hinting set to full, fonts look good on screen, but really bad (to the extent of being difficult to read) in print.

The GNOME hinting setting (used by Firefox 3.0) could be set from the command line, so I had aliases for:
gconftool --type string --set /desktop/gnome/font_rendering/hinting "slight"
and
gconftool --type string --set /desktop/gnome/font_rendering/hinting "full"
which I used whenever I wanted to print.

To change the fontconfig hinting setting, on the other hand, I do not know of a better way than to change the "hintfull" to "hintslight" in the following block in my .fonts.conf:
        <match target="font">
                <edit mode="assign" name="hintstyle">
                        <const>hintfull</const>
                </edit>
        </match>
then run fc-cache, and then restart Firefox 3.5 for the change to take effect.  Obviously not a practical to do whenever I need to print.

This is not an argument for using GNOME settings instead of fontconfig -- normally being able to change such font rendering settings on the fly does not generally sound all that necessary -- but it happens to significantly affect the usability impact of this bug.

Comment 44

[Ilja Sekler]

Confirming that this ugly bug has raised its head again. Reopening.

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2a1pre) Gecko/20090703 Firefox/3.6a1pre

SourceStamp=9493dd67dc31

See this with the latest official nightly build (SourceStamp=b4fee3813956) too. Will look for the regression window.

Comment 45

[Ilja Sekler]

This is very embarrassing, but I can't find any evidence that this bug has ever been fixed. Went back to 20081219020508 (SourceStamp=eb8abab83036) and the kerning in the printing output was still good with hintslight and bad with hintfull.

Tested with m-c (not 1.9.1 branch) builds on Ubuntu 9.04 and on a Fedora 10 live CD.

Comment 46

[Karl Tomlinson (:karlt)]

The bug fixed here was that gnome (and Xft) font properties were incorrectly
affecting print output (as reported in comment 5).

The issue with fontconfig settings can be tracked in bug 490475.

I recommend that fontconfig settings do not override what is already in the pattern without good reason.  (See bug 458612 comment 21.)

Comment 47

[Tom Alsberg]

Alright, so as I understand it:

* The issue that was (reported here as) fixed was Gnome hinting settings affecting (negatively) the printed output.

* The new issue now is fontconfig hinting settings affecting (negatively) the printed output, in the same way.

Presumably, this is because now (for on-screen font rendering) Firefox obeys fontconfig settings instead of Gnome settings.

This may be a separate problem, but the two feel suspiciously related.  Why do font hinting settings affect printing at all?  Font hinting should only be relevant to on-screen / (low resolution) raster rendering.

Comment 48

[Tom Alsberg]

Now that I think of it, the other common settings in fontconfig or in the Gnome font properties dialog mostly do not seem relevant to printing either.  Hinting, RGBA, anti-aliasing, resolution...  Perhaps font substitution (e.g. whenever Helvetica appears, use Arial instead) is meaningful for printing, though I am not sure if a user setting this would typically want (or expect) it also in print.

Comment 49

[Karl Tomlinson (:karlt)]

(In reply to comment #47)
> Presumably, this is because now (for on-screen font rendering) Firefox obeys
> fontconfig settings instead of Gnome settings.

Firefox still considers Gnome/Xft Screen settings (for screen rendering only now), but if fontconfig rules replace the Screen settings with other values, then you won't notice the effects of the Screen sttings.