Fix rounding error that can lead to clipped glyphs (closes #1978)

The key thing is to to trust the metrics in slot->bitmap, which
are always correctly rounded up to integral values, rather than
also looking at the fixed-point values in slot->metrics and
rounding them manually.

Previously, CanLoadAllGlyphs was pre-flighting the texture atlas
allocation without rendering the glyphs. However, slot->bitmap
isn't populated until the glyph is rendered. I've therefore
removed CanLoadAllGlyphs entirely, and tweaked the preflight
process so that it calls LoadCharGlyph directly.

This should be slightly faster, as we don't have to load the
glyphs twice. It also reduces the total code size a bit, and
guarantees that the "preflight" is always consistent.

The new preflight behavior is *slightly* different, in that it
will expand the first texture atlas up to its maximum size (but
won't move on to a second atlas). Previously, if CanLoadAllGlyphs
failed, we would render only as far as char code 255 (probably
leaving some of the initial texture atlas free).

Source/Urho3D/UI/FontFaceFreeType.cpp

@@ -160,25 +160,24 @@ bool FontFaceFreeType::Load(const unsigned char* fontData, unsigned fontDataSize
     // Load each of the glyphs to see the sizes & store other information
     loadMode_ = (int)(ui->GetForceAutoHint() ? FT_LOAD_FORCE_AUTOHINT : FT_LOAD_DEFAULT);
     ascender_ = RoundToPixels(face->size->metrics.ascender);
-    int descender = RoundToPixels(face->size->metrics.descender);
+    rowHeight_ = RoundToPixels(face->size->metrics.height);
+    pointSize_ = pointSize;
 
     // Check if the font's OS/2 info gives different (larger) values for ascender & descender
     TT_OS2* os2Info = (TT_OS2*)FT_Get_Sfnt_Table(face, ft_sfnt_os2);
     if (os2Info)
     {
+        int descender = RoundToPixels(face->size->metrics.descender);
         ascender_ = Max(ascender_, os2Info->usWinAscent * face->size->metrics.y_ppem / face->units_per_EM);
         ascender_ = Max(ascender_, os2Info->sTypoAscender * face->size->metrics.y_ppem / face->units_per_EM);
         descender = Max(descender, os2Info->usWinDescent * face->size->metrics.y_ppem / face->units_per_EM);
         descender = Max(descender, os2Info->sTypoDescender * face->size->metrics.y_ppem / face->units_per_EM);
+        rowHeight_ = Max(rowHeight_, ascender_ + descender);
     }
 
-    // Store point size and row height. Use the maximum of ascender + descender, or the face's stored default row height
-    pointSize_ = pointSize;
-    rowHeight_ = (int)Max(ascender_ + descender, RoundToPixels(face->size->metrics.height));
-
     int textureWidth = maxTextureSize;
     int textureHeight = maxTextureSize;
-    hasMutableGlyph_ = !CanLoadAllGlyphs(charCodes, textureWidth, textureHeight);
+    hasMutableGlyph_ = false;
 
     SharedPtr<Image> image(new Image(font_->GetContext()));
     image->SetSize(textureWidth, textureHeight, 1);
@@ -192,11 +191,11 @@ bool FontFaceFreeType::Load(const unsigned char* fontData, unsigned fontDataSize
         if (charCode == 0)
             continue;
 
-        if (hasMutableGlyph_ && (charCode > 0xff))
-            break;
-
         if (!LoadCharGlyph(charCode, image))
-            return false;
+        {
+            hasMutableGlyph_ = true;
+            break;
+        }
     }
 
     SharedPtr<Texture2D> texture = LoadFaceTexture(image);
@@ -308,38 +307,6 @@ const FontGlyph* FontFaceFreeType::GetGlyph(unsigned c)
     return 0;
 }
 
-bool FontFaceFreeType::CanLoadAllGlyphs(const PODVector<unsigned>& charCodes, int& textureWidth, int& textureHeight) const
-{
-    FT_Face face = (FT_Face)face_;
-    FT_GlyphSlot slot = face->glyph;
-    AreaAllocator allocator(FONT_TEXTURE_MIN_SIZE, FONT_TEXTURE_MIN_SIZE, textureWidth, textureHeight);
-
-    unsigned numGlyphs = charCodes.Size();
-    for (unsigned i = 0; i < numGlyphs; ++i)
-    {
-        unsigned charCode = charCodes[i];
-        if (charCode == 0)
-            continue;
-
-        FT_Error error = FT_Load_Char(face, charCode, loadMode_);
-        if (!error)
-        {
-            int width = Max(RoundToPixels(slot->metrics.width), (int)slot->bitmap.width);
-            int height = Max(RoundToPixels(slot->metrics.height), (int)slot->bitmap.rows);
-            if (width > 0 && height > 0)
-            {
-                int x, y;
-                if (!allocator.Allocate(width + 1, height + 1, x, y))
-                    return false;
-            }
-        }
-    }
-
-    textureWidth = allocator.GetWidth();
-    textureHeight = allocator.GetHeight();
-    return true;
-}
-
 bool FontFaceFreeType::SetupNextTexture(int textureWidth, int textureHeight)
 {
     SharedPtr<Image> image(new Image(font_->GetContext()));
@@ -368,96 +335,105 @@ bool FontFaceFreeType::LoadCharGlyph(unsigned charCode, Image* image)
     FT_GlyphSlot slot = face->glyph;
 
     FontGlyph fontGlyph;
-    FT_Error error = FT_Load_Char(face, charCode, loadMode_);
-    if (!error)
+    FT_Error error = FT_Load_Char(face, charCode, loadMode_ | FT_LOAD_RENDER);
+    if (error)
+    {
+        const char* family = face->family_name ? face->family_name : "NULL";
+        URHO3D_LOGERRORF("FT_Load_Char failed (family: %s, char code: %u)", family, charCode);
+        fontGlyph.width_ = 0;
+        fontGlyph.height_ = 0;
+        fontGlyph.offsetX_ = 0;
+        fontGlyph.offsetY_ = 0;
+        fontGlyph.advanceX_ = 0;
+        fontGlyph.page_ = 0;
+    }
+    else
     {
         // Note: position within texture will be filled later
-        fontGlyph.width_ = (short)Max(RoundToPixels(slot->metrics.width), (int)slot->bitmap.width);
-        fontGlyph.height_ = (short)Max(RoundToPixels(slot->metrics.height), (int)slot->bitmap.rows);
-        fontGlyph.offsetX_ = (short)(RoundToPixels(slot->metrics.horiBearingX));
-        fontGlyph.offsetY_ = (short)(ascender_ - RoundToPixels(slot->metrics.horiBearingY));
-        fontGlyph.advanceX_ = (short)(slot->metrics.horiAdvance >> 6);
+        fontGlyph.width_ = slot->bitmap.width;
+        fontGlyph.height_ = slot->bitmap.rows;
+        fontGlyph.offsetX_ = slot->bitmap_left;
+        fontGlyph.offsetY_ = ascender_ - slot->bitmap_top;
+        fontGlyph.advanceX_ = (short)RoundToPixels(slot->metrics.horiAdvance);
+    }
 
-        if (fontGlyph.width_ > 0 && fontGlyph.height_ > 0)
+    int x = 0, y = 0;
+    if (fontGlyph.width_ > 0 && fontGlyph.height_ > 0)
+    {
+        if (!allocator_.Allocate(fontGlyph.width_ + 1, fontGlyph.height_ + 1, x, y))
         {
-            int x, y;
-            if (!allocator_.Allocate(fontGlyph.width_ + 1, fontGlyph.height_ + 1, x, y))
+            if (image)
             {
-                if (!HasMutableGlyphs())
-                {
-                    // Should never happen if CanLoadAllGlyphs was used correctly
-                    URHO3D_LOGERROR("Unexpectedly ran out of space in LoadCharGlyph");
-                }
-
-                if (!SetupNextTexture(allocator_.GetWidth(), allocator_.GetHeight()))
-                    return false;
-
-                if (!allocator_.Allocate(fontGlyph.width_ + 1, fontGlyph.height_ + 1, x, y))
-                    return false;
+                // We're rendering into a fixed image and we ran out of room.
+                return false;
             }
 
-            fontGlyph.x_ = (short)x;
-            fontGlyph.y_ = (short)y;
-
-            unsigned char* dest = 0;
-            unsigned pitch = 0;
-            if (image)
+            int w = allocator_.GetWidth();
+            int h = allocator_.GetHeight();
+            if (!SetupNextTexture(w, h))
             {
-                fontGlyph.page_ = 0;
-                dest = image->GetData() + fontGlyph.y_ * image->GetWidth() + fontGlyph.x_;
-                pitch = (unsigned)image->GetWidth();
+                URHO3D_LOGWARNINGF("FontFaceFreeType::LoadCharGlyph: failed to allocate new %dx%d texture", w, h);
+                return false;
             }
-            else
+
+            if (!allocator_.Allocate(fontGlyph.width_ + 1, fontGlyph.height_ + 1, x, y))
             {
-                fontGlyph.page_ = textures_.Size() - 1;
-                dest = new unsigned char[fontGlyph.width_ * fontGlyph.height_];
-                pitch = (unsigned)fontGlyph.width_;
+                URHO3D_LOGWARNINGF("FontFaceFreeType::LoadCharGlyph: failed to position char code %u in blank page", charCode);
+                return false;
             }
+        }
 
-            FT_Render_Glyph(slot, FT_RENDER_MODE_NORMAL);
-            if (slot->bitmap.pixel_mode == FT_PIXEL_MODE_MONO)
-            {
-                for (unsigned y = 0; y < (unsigned)slot->bitmap.rows; ++y)
-                {
-                    unsigned char* src = slot->bitmap.buffer + slot->bitmap.pitch * y;
-                    unsigned char* rowDest = dest + y * pitch;
+        fontGlyph.x_ = (short)x;
+        fontGlyph.y_ = (short)y;
 
-                    for (unsigned x = 0; x < (unsigned)slot->bitmap.width; ++x)
-                        rowDest[x] = (unsigned char)((src[x >> 3] & (0x80 >> (x & 7))) ? 255 : 0);
-                }
-            }
-            else
+        unsigned char* dest = 0;
+        unsigned pitch = 0;
+        if (image)
+        {
+            fontGlyph.page_ = 0;
+            dest = image->GetData() + fontGlyph.y_ * image->GetWidth() + fontGlyph.x_;
+            pitch = (unsigned)image->GetWidth();
+        }
+        else
+        {
+            fontGlyph.page_ = textures_.Size() - 1;
+            dest = new unsigned char[fontGlyph.width_ * fontGlyph.height_];
+            pitch = (unsigned)fontGlyph.width_;
+        }
+
+        if (slot->bitmap.pixel_mode == FT_PIXEL_MODE_MONO)
+        {
+            for (unsigned y = 0; y < (unsigned)slot->bitmap.rows; ++y)
             {
-                for (unsigned y = 0; y < (unsigned)slot->bitmap.rows; ++y)
-                {
-                    unsigned char* src = slot->bitmap.buffer + slot->bitmap.pitch * y;
-                    unsigned char* rowDest = dest + y * pitch;
+                unsigned char* src = slot->bitmap.buffer + slot->bitmap.pitch * y;
+                unsigned char* rowDest = dest + y * pitch;
 
-                    for (unsigned x = 0; x < (unsigned)slot->bitmap.width; ++x)
-                        rowDest[x] = src[x];
-                }
+                for (unsigned x = 0; x < (unsigned)slot->bitmap.width; ++x)
+                    rowDest[x] = (unsigned char)((src[x >> 3] & (0x80 >> (x & 7))) ? 255 : 0);
             }
-
-            if (!image)
+        }
+        else
+        {
+            for (unsigned y = 0; y < (unsigned)slot->bitmap.rows; ++y)
             {
-                textures_.Back()->SetData(0, fontGlyph.x_, fontGlyph.y_, fontGlyph.width_, fontGlyph.height_, dest);
-                delete[] dest;
+                unsigned char* src = slot->bitmap.buffer + slot->bitmap.pitch * y;
+                unsigned char* rowDest = dest + y * pitch;
+
+                for (unsigned x = 0; x < (unsigned)slot->bitmap.width; ++x)
+                    rowDest[x] = src[x];
             }
         }
-        else
+
+        if (!image)
         {
-            fontGlyph.x_ = 0;
-            fontGlyph.y_ = 0;
-            fontGlyph.page_ = 0;
+            textures_.Back()->SetData(0, fontGlyph.x_, fontGlyph.y_, fontGlyph.width_, fontGlyph.height_, dest);
+            delete[] dest;
         }
     }
     else
     {
-        fontGlyph.width_ = 0;
-        fontGlyph.height_ = 0;
-        fontGlyph.offsetX_ = 0;
-        fontGlyph.offsetY_ = 0;
-        fontGlyph.advanceX_ = 0;
+        fontGlyph.x_ = 0;
+        fontGlyph.y_ = 0;
         fontGlyph.page_ = 0;
     }
 

Source/Urho3D/UI/FontFaceFreeType.h

@@ -48,8 +48,6 @@ public:
     virtual bool HasMutableGlyphs() const { return hasMutableGlyph_; }
 
 private:
-    /// Check can load all glyph in one texture, return true and texture size if can load.
-    bool CanLoadAllGlyphs(const PODVector<unsigned>& charCodes, int& textureWidth, int& textureHeight) const;
     /// Setup next texture.
     bool SetupNextTexture(int textureWidth, int textureHeight);
     /// Load char glyph.