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

