[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-devel
Subject:    Re: Font hinting: suggestion to consistently restrict its usage to UI
From:       Benoit Jacob <jacob.benoit.1 () gmail ! com>
Date:       2009-09-30 15:56:03
Message-ID: d9f848520909300856g659253c6q128cd5408d9c8892 () mail ! gmail ! com
[Download RAW message or body]

2009/9/30 Benoit Jacob <jacob.benoit.1@gmail.com>:
> 2009/9/29 Benoit Jacob <jacob.benoit.1@gmail.com>:
>> 2009/9/29 Albert Astals Cid <aacid@kde.org>:
>>> You said you are not a font expert yet you claim this is because of fon=
t
>>> hinting. I see a logic problem here. I'm not a font expert so i really =
don't
>>> know if this is because of hinting or not. And no, you are not the firs=
t
>>> person to complain yet noone seems to scratch his itch.
>>
>> I can't say whether font hinting is the _only_ problem, that's true.
>> Reading this comment,
>>
>> http://slangkamp.wordpress.com/2009/09/29/kword-font-rendering/#comment-=
99
>>
>> it seems that the ideal solution would involve fixing the Qt backend
>> of poppler. What do you think of this comment?
>>
>> Suppose that I volunteer for trying to do that. Where do I start?
>
> OK, so here's where I am.
>
> - Okular uses poppler-qt4.
>
> - It seems that Poppler-qt4 always =A0uses a Splash backend, indeed I
> found this in poppler-qt4 sources:
>
> =A0 =A0 =A0 =A0case Document::ArthurBackend:
> =A0 =A0 =A0 =A0// create a splash backend even in case of the Arthur Back=
end
> =A0 =A0 =A0 =A0case Document::SplashBackend:
> =A0 =A0 =A0 =A0{
>
> - So I started looking at the Splash backend sources and I have
> comments. But i'll move this discussion to a poppler mailing list!

I've now sent a patch to poppler, as it turns out that disabling
hinting, at least in my setting, was actually the intention of the
code but wasn't actually done only because of a bug.

If anyone is interested to apply it locally, here's the patch against
poppler; but the discussion has now moved to the poppler mailing list.

Benoit

["Fix-FT_Load_Glyph-flags.patch" (text/x-patch)]

From 2f19efa83a7268b522377723e40700202537978d Mon Sep 17 00:00:00 2001
From: Benoit Jacob <jacob.benoit.1@gmail.com>
Date: Wed, 30 Sep 2009 10:57:30 -0400
Subject: [PATCH] Uniformize the flags passed to FT_Load_Glyph. Fix and simplify the way that
 they are computed.

* introduce SplashFTFont::getFTLoadFlags() returning the flags to use,
  so that the code determining them is centralized in 1 place (used to be 3,
  and one of them was inconsistent with the others)

* The new simplified logic is:

    return a ? FT_LOAD_NO_HINTING | FT_LOAD_NO_BITMAP : FT_LOAD_DEFAULT;

* Notice that this is already the logic that was meant to be used when
  the bytecode interpreter isn't enabled. However, the bytecode interpreter
  detection was completely broken: it was based on the presence of the
  TT_CONFIG_OPTION_BYTECODE_INTERPRETER symbol, but that only tells you
  whether the support was enabled in the FreeType build, that tells nothing
  about whether it is actually enabled in the user's configuration.
  Here on OpenSUSE 11.2 the symbol was defined even though the default
  configuration had it disabled.

* Also notice that, when AA is enabled, the choice

     FT_LOAD_NO_HINTING | FT_LOAD_NO_BITMAP

  is the same as what is being used in the Cairo back-end. That feels good.
  Also, it appears as if Adobe Acrobat also disables hinting, so there is
  quite some consensus here.

* I left the logic that when AA is disabled, the defaults options are used
  (so hinting may be used). I don't have an opinion on non-AA text. Just FYI,
  notice that the Cairo back-end doesn't do that, it always disables hinting.
---
 splash/SplashFTFont.cc |   60 ++++++++---------------------------------------
 splash/SplashFTFont.h  |    2 +
 2 files changed, 13 insertions(+), 49 deletions(-)

diff --git a/splash/SplashFTFont.cc b/splash/SplashFTFont.cc
index 4c62dc6..d25e183 100644
--- a/splash/SplashFTFont.cc
+++ b/splash/SplashFTFont.cc
@@ -163,6 +163,12 @@ SplashFTFont::SplashFTFont(SplashFTFontFile *fontFileA, SplashCoord *matA,
 SplashFTFont::~SplashFTFont() {
 }
 
+FT_Int32 SplashFTFont::getFTLoadFlags() const
+{
+  return aa ? FT_LOAD_NO_HINTING | FT_LOAD_NO_BITMAP
+            : FT_LOAD_DEFAULT;
+}
+
 GBool SplashFTFont::getGlyph(int c, int xFrac, int yFrac,
 			     SplashGlyphBitmap *bitmap, int x0, int y0, SplashClip *clip, SplashClipResult *clipRes) {
   return SplashFont::getGlyph(c, xFrac, 0, bitmap, x0, y0, clip, clipRes);
@@ -196,30 +202,8 @@ GBool SplashFTFont::makeGlyph(int c, int xFrac, int yFrac,
     return gFalse;
   }
 
-  if (noah) {
-    if (FT_Load_Glyph(ff->face, gid,
-                      aa ? FT_LOAD_NO_HINTING | FT_LOAD_NO_BITMAP
-                         : FT_LOAD_DEFAULT)) {
-      return gFalse;
-    }
-  } else {
-    // if we have the FT2 bytecode interpreter, autohinting won't be used
-#ifdef TT_CONFIG_OPTION_BYTECODE_INTERPRETER
-    if (FT_Load_Glyph(ff->face, gid,
-                      aa ? FT_LOAD_NO_BITMAP : FT_LOAD_DEFAULT)) {
-      return gFalse;
-    }
-#else
-    // FT2's autohinting doesn't always work very well (especially with
-    // font subsets), so turn it off if anti-aliasing is enabled; if
-    // anti-aliasing is disabled, this seems to be a tossup - some fonts
-    // look better with hinting, some without, so leave hinting on
-    if (FT_Load_Glyph(ff->face, gid,
-		      aa ? FT_LOAD_NO_HINTING | FT_LOAD_NO_BITMAP
-                         : FT_LOAD_DEFAULT)) {
-      return gFalse;
-    }
-#endif
+  if (FT_Load_Glyph(ff->face, gid, getFTLoadFlags())) {
+    return gFalse;
   }
 
   FT_Glyph_Metrics *glyphMetrics = &(ff->face->glyph->metrics);
@@ -296,30 +280,8 @@ double SplashFTFont::getGlyphAdvance(int c)
     return -1;
   }
 
-  if (noah) {
-    if (FT_Load_Glyph(ff->face, gid,
-                      aa ? FT_LOAD_NO_HINTING | FT_LOAD_NO_BITMAP
-                         : FT_LOAD_DEFAULT)) {
-      return -1;
-    }
-  } else {
-    // if we have the FT2 bytecode interpreter, autohinting won't be used
-#ifdef TT_CONFIG_OPTION_BYTECODE_INTERPRETER
-    if (FT_Load_Glyph(ff->face, gid,
-		      aa ? FT_LOAD_NO_BITMAP : FT_LOAD_DEFAULT)) {
-      return -1;
-    }
-#else
-    // FT2's autohinting doesn't always work very well (especially with
-    // font subsets), so turn it off if anti-aliasing is enabled; if
-    // anti-aliasing is disabled, this seems to be a tossup - some fonts
-    // look better with hinting, some without, so leave hinting on
-    if (FT_Load_Glyph(ff->face, gid,
-		      aa ? FT_LOAD_NO_HINTING | FT_LOAD_NO_BITMAP
-                         : FT_LOAD_DEFAULT)) {
-      return -1;
-    }
-#endif
+  if (FT_Load_Glyph(ff->face, gid, getFTLoadFlags())) {
+    return -1;
   }
 
   // 64.0 is 1 in 26.6 format
@@ -366,7 +328,7 @@ SplashPath *SplashFTFont::getGlyphPath(int c) {
     // skip the TrueType notdef glyph
     return NULL;
   }
-  if (FT_Load_Glyph(ff->face, gid, FT_LOAD_NO_BITMAP)) {
+  if (FT_Load_Glyph(ff->face, gid, getFTLoadFlags())) {
     return NULL;
   }
   if (FT_Get_Glyph(slot, &glyph)) {
diff --git a/splash/SplashFTFont.h b/splash/SplashFTFont.h
index 1881d8e..627dcc0 100644
--- a/splash/SplashFTFont.h
+++ b/splash/SplashFTFont.h
@@ -62,6 +62,8 @@ public:
   virtual double getGlyphAdvance(int c);
 
 private:
+
+  FT_Int32 getFTLoadFlags() const;
 
   FT_Size sizeObj;
   FT_Matrix matrix;
-- 
1.6.4.2



>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic