Improved vertical alignment of text in FbTk::TextButton

The old formula for vertical align text inside FbTk::TextButton ('height/2 +
font_ascent/2 - 1') produced not always good looking results, escpecially
when different fonts are involved (eg, ClockTool and WorkspaceName have
different fonts and font-sizes).

'(height - font_ascent) / 2 - 1' produces better results.

Additional changes:

* added ASCII-Art to document the involved entities when calculating the
  baseline
* rewritten tests/testFont.cc to accept multiples texts and multiple
  fonts
* removed some internal parts of FbTk::Font from the public interface

4 files changed, 109 insertions, 65 deletions

diff --git a/src/FbTk/Font.cc b/src/FbTk/Font.cc
index 37621a8..544ddc2 100644
--- a/src/FbTk/Font.cc
+++ b/src/FbTk/Font.cc
@@ -66,6 +66,7 @@
 #include <map>
 #include <typeinfo>
 #include <langinfo.h>
+#include <cstdio>
 
 #include <errno.h>
 
@@ -83,12 +84,10 @@ namespace {
 // use to map <font1>|<font2>|<font3> => <fontthatworks>
 typedef map<string, string> StringMap;
 typedef StringMap::iterator StringMapIt;
-StringMap lookup_map;
 
 // stores <fontthatworks and the fontimp
 typedef map<string, FbTk::FontImp* > FontCache;
 typedef FontCache::iterator FontCacheIt;
-FontCache font_cache;
 
 
 void resetEffects(FbTk::Font& font) {
@@ -100,24 +99,30 @@ void resetEffects(FbTk::Font& font) {
     font.setShadowOffX(2);
 }
 
+
+StringMap s_lookup_map;
+FontCache s_font_cache;
+bool s_multibyte = false; // if the fontimp should be a multibyte font
+bool s_utf8mode = false; // should the font use utf8 font imp
+
+
 } // end nameless namespace
 
 
 
 namespace FbTk {
 
-bool Font::s_multibyte = false;
-bool Font::s_utf8mode = false;
+const char Font::DEFAULT_FONT[] = "__DEFAULT__";
 
 
 void Font::shutdown() {
 
     FontCacheIt fit;
-    for (fit = font_cache.begin(); fit != font_cache.end(); fit++) {
+    for (fit = s_font_cache.begin(); fit != s_font_cache.end(); ++fit) {
         FontImp* font = fit->second;
         if (font) {
             FontCacheIt it;
-            for (it = fit; it != font_cache.end(); ++it)
+            for (it = fit; it != s_font_cache.end(); ++it)
                 if (it->second == font)
                     it->second = 0;
             delete font;
@@ -125,6 +130,15 @@ void Font::shutdown() {
     }
 }
 
+bool Font::multibyte() {
+    return s_multibyte;
+}
+
+bool Font::utf8() {
+    return s_utf8mode;
+}
+
+
 Font::Font(const char *name):
     m_fontimp(0),
     m_shadow(false), m_shadow_color("black", DefaultScreen(App::instance()->display())),
@@ -135,13 +149,15 @@ Font::Font(const char *name):
     if (MB_CUR_MAX > 1) // more than one byte, then we're multibyte
         s_multibyte = true;
 
-    // check for utf-8 mode
-#if defined(CODESET) && !defined(_WIN32)
-    char *locale_codeset = nl_langinfo(CODESET);
-#else // openbsd doesnt have this (yet?)
+
     char *locale_codeset = 0;
-#endif // defined(CODESET) && !defined(_WIN32)
 
+    // openbsd doesnt have this (yet?)
+#if defined(CODESET) && !defined(_WIN32)
+    locale_codeset = nl_langinfo(CODESET);
+#endif
+
+    // check for utf-8 mode
     if (locale_codeset && strcmp("UTF-8", locale_codeset) == 0) {
         s_utf8mode = true;
     } else if (locale_codeset != 0) {
@@ -159,15 +175,15 @@ Font::~Font() {
 
 bool Font::load(const string &name) {
 
-    if (name.size() == 0)
+    if (name.empty())
         return false;
 
     StringMapIt lookup_entry;
     FontCacheIt cache_entry;
 
     // check if one of <font1>|<font2>|<font3> is already there
-    if ((lookup_entry = lookup_map.find(name)) != lookup_map.end() &&
-            (cache_entry = font_cache.find(lookup_entry->second)) != font_cache.end()) {
+    if ((lookup_entry = s_lookup_map.find(name)) != s_lookup_map.end() &&
+            (cache_entry = s_font_cache.find(lookup_entry->second)) != s_font_cache.end()) {
         m_fontstr = cache_entry->first;
         m_fontimp = cache_entry->second;
         resetEffects(*this);
@@ -185,10 +201,10 @@ bool Font::load(const string &name) {
         FbTk::StringUtil::removeTrailingWhitespace(*name_it);
         FbTk::StringUtil::removeFirstWhitespace(*name_it);
 
-        if ((cache_entry = font_cache.find(*name_it)) != font_cache.end()) {
+        if ((cache_entry = s_font_cache.find(*name_it)) != s_font_cache.end()) {
             m_fontstr = cache_entry->first;
             m_fontimp = cache_entry->second;
-            lookup_map[name] = m_fontstr;
+            s_lookup_map[name] = m_fontstr;
             resetEffects(*this);
             return true;
         }
@@ -210,32 +226,33 @@ bool Font::load(const string &name) {
 #ifdef USE_XFT
         if ((*name_it)[0] != '-') {
 
-            if (*name_it == "__DEFAULT__")
+            if (*name_it == Font::DEFAULT_FONT)
                 realname = "monospace";
 
-            tmp_font = new XftFontImp(0, s_utf8mode);
+            tmp_font = new XftFontImp(0, utf8());
         }
 #endif // USE_XFT
 
         if (!tmp_font) {
-            if (*name_it == "__DEFAULT__")
+            if (*name_it == Font::DEFAULT_FONT)
                 realname = "fixed";
 
 #ifdef USE_XMB
 
-            if (s_multibyte || s_utf8mode) {
-                tmp_font = new XmbFontImp(0, s_utf8mode);
-            } else // basic font implementation
+            if (multibyte() || utf8()) {
+                tmp_font = new XmbFontImp(0, utf8());
+            }
 #endif // USE_XMB
-    {
-               tmp_font = new XFontImp();
-    }
+        }
+
+        if (!tmp_font) {
+           tmp_font = new XFontImp();
         }
 
         if (tmp_font && tmp_font->load(realname.c_str())) {
-            lookup_map[name] = (*name_it);
+            s_lookup_map[name] = (*name_it);
             m_fontimp = tmp_font;
-            font_cache[(*name_it)] = tmp_font;
+            s_font_cache[(*name_it)] = tmp_font;
             m_fontstr = name;
             resetEffects(*this);
             return true;
@@ -251,6 +268,10 @@ unsigned int Font::textWidth(const char* text, unsigned int size) const {
     return m_fontimp->textWidth(text, size);
 }
 
+unsigned int Font::textWidth(const BiDiString &text) const {
+    return textWidth(text.visual().c_str(), text.visual().size());
+}
+
 unsigned int Font::height() const {
     return m_fontimp->height();
 }
diff --git a/src/FbTk/Font.hh b/src/FbTk/Font.hh
index fb399d9..6ea2571 100644
--- a/src/FbTk/Font.hh
+++ b/src/FbTk/Font.hh
@@ -43,17 +43,19 @@ class FbDrawable;
 class Font {
 public:
 
+    static const char DEFAULT_FONT[];
+
+
     /// called at FbTk::App destruction time, cleans up cache
     static void shutdown();
 
     /// @return true if multibyte is enabled, else false
-    static bool multibyte() { return s_multibyte; }
+    static bool multibyte();
     /// @return true if utf-8 mode is enabled, else false
-    static bool utf8() { return s_utf8mode; }
-
+    static bool utf8();
 
 
-    explicit Font(const char *name = "__DEFAULT__");
+    explicit Font(const char* name = DEFAULT_FONT);
     virtual ~Font();
     /**
         Load a font
@@ -76,9 +78,7 @@ public:
        @return size of text in pixels
     */
     unsigned int textWidth(const char* text, unsigned int size) const;
-    unsigned int textWidth(const BiDiString &text) const {
-        return textWidth(text.visual().c_str(), text.visual().size());
-    }
+    unsigned int textWidth(const BiDiString &text) const;
 
     unsigned int height() const;
     int ascent() const;
@@ -119,9 +119,6 @@ private:
     FbTk::FontImp* m_fontimp; ///< font implementation
     std::string m_fontstr; ///< font name
 
-    static bool s_multibyte; ///< if the fontimp should be a multibyte font
-    static bool s_utf8mode; ///< should the font use utf8 font imp
-
     int m_angle; ///< rotation angle
     bool m_shadow; ///< shadow text
     Color m_shadow_color; ///< shadow color
diff --git a/src/FbTk/TextButton.cc b/src/FbTk/TextButton.cc
index c31e3d6..33a2b44 100644
--- a/src/FbTk/TextButton.cc
+++ b/src/FbTk/TextButton.cc
@@ -23,6 +23,7 @@
 #include "TextUtils.hh"
 #include "Font.hh"
 #include "GContext.hh"
+#include <cstdio>
 
 namespace FbTk {
 
@@ -62,16 +63,18 @@ void TextButton::setJustify(FbTk::Justify just) {
 }
 
 bool TextButton::setOrientation(FbTk::Orientation orient) {
+
     if (orient == m_orientation
         || !m_font->validOrientation(orient))
         return false;
+
     invalidateBackground();
 
     if (((m_orientation == FbTk::ROT0 || m_orientation == FbTk::ROT180) &&
          (orient == FbTk::ROT90 || orient == FbTk::ROT270)) ||
         ((m_orientation == FbTk::ROT90 || m_orientation == FbTk::ROT270) &&
          (orient == FbTk::ROT0 || orient == FbTk::ROT180))) {
-        // flip width and height
+
         m_orientation = orient;
         resize(height(), width());
     } else {
@@ -112,8 +115,7 @@ void TextButton::setTextPadding(unsigned int padding) {
 
 /// clear window and redraw text
 void TextButton::clear() {
-    TextButton::clearArea(0, 0,
-                          width(), height());
+    TextButton::clearArea(0, 0, width(), height());
 }
 
 void TextButton::clearArea(int x, int y,
@@ -135,34 +137,58 @@ void TextButton::renderForeground(FbWindow &win, FbDrawable &drawable) {
 }
 
 void TextButton::drawText(int x_offset, int y_offset, FbDrawable *drawable) {
+
+    if (drawable == 0)
+        drawable = this;
+
     const FbString& visual = text().visual();
     unsigned int textlen = visual.size();
-    unsigned int textw = width();
-    unsigned int texth = height();
-    translateSize(m_orientation, textw, texth);
+    unsigned int button_width = width();
+    unsigned int button_height = height();
+
+    translateSize(m_orientation, button_width, button_height);
 
-    int align_x = FbTk::doAlignment(textw - x_offset - m_left_padding - m_right_padding,
+    // horizontal alignment, cut off text if needed
+    int align_x = FbTk::doAlignment(button_width - x_offset - m_left_padding - m_right_padding,
                                     bevel(), justify(), font(),
                                     visual.data(), visual.size(),
                                     textlen); // return new text len
 
-    // center text by default
-    int center_pos = texth/2 + font().ascent()/2 - 1;
-
-    int textx = align_x + x_offset + m_left_padding;
-    int texty = center_pos + y_offset;
-
-    if (drawable == 0)
-        drawable = this;
+    //
+    //  we center the text vertically. to do this we have to nudge the
+    //  baseline a little bit down so the "middle" of the glyph is placed
+    //  on the middle of the button. we "assume", that ascent/2 is roughly
+    //  the middle of the glyph. example:
+    //
+    //   +== ascent <--------->==                       +=====================
+    //   |          |         |                         |
+    //   |          |  ggggg  |                         |   ascent <--------->
+    //   |          |  g  gg  |                         |          |         |
+    //   | baseline <  glyph  |                         |          |  ggggg  |
+    //   |          |      g  |  -- middle of button -- |          |  g  gg  |
+    //   |  descent <  ggggg  |                         | baseline <  glyph  |
+    //   |  height  |_________|                         |          |      g  |
+    //   |                                              |  descent <  ggggg  |
+    //   |                                              |   height |_________|
+    //   |                                              |
+    //   +=======================                       +=====================
+    //
+    //    ascent = 4
+    //    button_height = 11
+    //    baseline = (11 + 4) / 2 - 1 = 6
+    //
+
+    int baseline_x = align_x + x_offset + m_left_padding;
+    int baseline_y = ((button_height + font().ascent()) / 2) - 1 + y_offset;
+
+    // TODO: remove debug output fprintf(stderr, "%d | %d %d %d\n", height(), font().height(), font().ascent(), font().descent());
 
     // give it ROT0 style coords
-    translateCoords(m_orientation, textx, texty, textw, texth);
+    translateCoords(m_orientation, baseline_x, baseline_y, button_width, button_height);
 
-    font().drawText(*drawable,
-                    screenNumber(),
-                    gc(), // graphic context
-                    visual.c_str(), textlen, // string and string size
-                    textx, texty, m_orientation); // position
+    font().drawText(*drawable, screenNumber(), gc(),
+                    visual.c_str(), textlen,
+                    baseline_x, baseline_y, m_orientation);
 }
 
 
@@ -170,15 +196,15 @@ bool TextButton::textExceeds(int x_offset) {
 
     const FbString& visual = text().visual();
     unsigned int textlen = visual.size();
-    unsigned int textw = width();
-    unsigned int texth = height();
-    translateSize(m_orientation, textw, texth);
+    unsigned int button_width = width();
+    unsigned int button_height = height();
+    translateSize(m_orientation, button_width, button_height);
 
-    FbTk::doAlignment(textw - x_offset - m_left_padding - m_right_padding,
+    FbTk::doAlignment(button_width - x_offset - m_left_padding - m_right_padding,
                       bevel(), justify(), font(), visual.data(), visual.size(),
                       textlen); // return new text len
 
-    return visual.size()>textlen;
+    return visual.size() > textlen;
 }
 
 void TextButton::exposeEvent(XExposeEvent &event) {
diff --git a/src/FbTk/TextButton.hh b/src/FbTk/TextButton.hh
index a1c8b0e..eb0beca 100644
--- a/src/FbTk/TextButton.hh
+++ b/src/FbTk/TextButton.hh
@@ -56,7 +56,8 @@ public:
 
     void exposeEvent(XExposeEvent &event);
 
-    void renderForeground(FbDrawable &drawable);
+    //void renderForeground(FbDrawable &drawable);
+    void renderForeground(FbWindow &win, FbDrawable &drawable);
 
     FbTk::Justify justify() const { return m_justify; }
     const BiDiString &text() const { return m_text; }
@@ -65,7 +66,6 @@ public:
     unsigned int textWidth() const;
     int bevel() const { return m_bevel; }
 
-    void renderForeground(FbWindow &win, FbDrawable &drawable);
 
 protected:
     virtual void drawText(int x_offset, int y_offset, FbDrawable *drawable_override);