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

RESOLVED FIXED in mozilla1.9.1b3

Status

()

defect
P1
normal
RESOLVED FIXED
12 years ago
10 years ago

People

(Reporter: bugzilla.i.sekler, Assigned: stransky)

Tracking

(Blocks 1 bug, {fixed1.9.1})

Trunk
mozilla1.9.1b3
x86
Linux
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.1 -
wanted1.9.1 +
blocking1.9 -
wanted1.9 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 4 obsolete attachments)

(Reporter)

Description

12 years ago
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.
(Reporter)

Comment 2

12 years ago
(Reporter)

Updated

12 years ago
Component: General → Printing
Product: Firefox → Core
Version: unspecified → Trunk
QA Contact: general → printing
(Reporter)

Comment 4

12 years ago
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.
Summary: printing: bad kerning with some fonts → bad kerning in print output if anti-aliasing is turned off
(Reporter)

Comment 5

12 years ago
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.
Summary: bad kerning in print output if anti-aliasing is turned off → bad kerning in print output if hinting is set to "medium" or "full"
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
This is not related to the print dialog. It is almost certainly cairo/Pango/gfx stuff.
Component: Printing → GFX: Thebes
QA Contact: printing → thebes
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).
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.
roc/karl, do we want to block on this?  Any idea what a fix would be?
Flags: blocking1.9?
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.
Nice to have for 1.9 but we can't block on this.
Flags: wanted1.9+
Flags: blocking1.9?
Flags: blocking1.9-
I can confirm this bug as well in my Ubuntu 8.04 using Firefox 3.0rc1.
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.
Flags: wanted1.9.1?
Flags: blocking1.9.1?
Flags: wanted1.9.1?
Flags: wanted1.9.1+
Flags: blocking1.9.1?
Flags: blocking1.9.1-
Priority: -- → P1
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.
(Assignee)

Updated

11 years ago
Duplicate of this bug: 464684
(Assignee)

Comment 19

11 years ago
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)
(Assignee)

Comment 20

11 years ago
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()).
(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.
Flags: blocking1.9.2?
(Assignee)

Comment 22

11 years ago
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...
(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);
(Assignee)

Comment 24

11 years ago
Posted patch an updated patch (obsolete) — Splinter Review
Thanks, here is the updated patch.
Attachment #351180 - Attachment is obsolete: true
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().
(Assignee)

Comment 26

11 years ago
Posted patch the updated one (obsolete) — Splinter Review
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...
Attachment #351366 - Attachment is obsolete: true
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.
Attachment #352118 - Flags: review+
(Assignee)

Comment 28

11 years ago
This one should address the changes...
Attachment #352118 - Attachment is obsolete: true
(Reporter)

Comment 29

11 years ago
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+).
Attachment #352345 - Flags: superreview?(vladimir)
Attachment #352345 - Flags: review+
Attachment #352345 - Flags: superreview?(vladimir) → superreview+
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 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.
Attachment #353328 - Flags: approval1.9.1?
Fixed on mozilla-central.
Assignee: nobody → stransky
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b3
No longer blocks: 459748
Duplicate of this bug: 459748
Comment on attachment 353328 [details] [diff] [review]
with necessary changes to gfx/thebes/test

a191=beltzner
Attachment #353328 - Flags: approval1.9.1? → approval1.9.1+
(Assignee)

Updated

10 years ago
Duplicate of this bug: 431427
(Assignee)

Updated

10 years ago
Duplicate of this bug: 429638
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?
(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

Updated

10 years ago
Duplicate of this bug: 490475

Comment 42

10 years ago
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

10 years ago
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.
(Reporter)

Comment 44

10 years ago
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Reporter)

Comment 45

10 years ago
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.
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.)
Status: REOPENED → RESOLVED
Last Resolved: 11 years ago10 years ago
Resolution: --- → FIXED

Comment 47

10 years ago
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

10 years ago
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.
(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.

Updated

10 years ago
Flags: blocking1.9.2?
You need to log in before you can comment on or make changes to this bug.