[core] Add descender, ascender in glyphMetrics. Fix mixed fonts dis-allignment

3 files changed, 55 insertions, 22 deletions

diff --git a/src/mbgl/text/glyph.hpp b/src/mbgl/text/glyph.hpp
index ba9c521f77..8977f4992a 100644
--- a/src/mbgl/text/glyph.hpp
+++ b/src/mbgl/text/glyph.hpp
@@ -30,6 +30,8 @@ struct GlyphMetrics {
     int32_t left = 0;
     int32_t top = 0;
     uint32_t advance = 0;
+    double ascender = 0.0;
+    double descender = 0.0;
 };
 
 inline bool operator==(const GlyphMetrics& lhs, const GlyphMetrics& rhs) {
diff --git a/src/mbgl/text/glyph_pbf.cpp b/src/mbgl/text/glyph_pbf.cpp
index cfaf803f75..d7b176b042 100644
--- a/src/mbgl/text/glyph_pbf.cpp
+++ b/src/mbgl/text/glyph_pbf.cpp
@@ -11,10 +11,8 @@ std::vector<Glyph> parseGlyphPBF(const GlyphRange& glyphRange, const std::string
     protozero::pbf_reader glyphs_pbf(data);
 
     while (glyphs_pbf.next(1)) {
-        auto fontstack_pbf = glyphs_pbf.get_message();
-        while (fontstack_pbf.next(3)) {
+        auto readGlyphMetrics = [glyphRange, &result](protozero::pbf_reader& fontstack_pbf) {
             auto glyph_pbf = fontstack_pbf.get_message();
-
             Glyph glyph;
             protozero::data_view glyphData;
 
@@ -64,27 +62,58 @@ std::vector<Glyph> parseGlyphPBF(const GlyphRange& glyphRange, const std::string
                 glyph.metrics.width >= 256 || glyph.metrics.height >= 256 ||
                 glyph.metrics.left < -128 || glyph.metrics.left >= 128 ||
                 glyph.metrics.top < -128 || glyph.metrics.top >= 128 ||
-                glyph.metrics.advance >= 256 ||
-                glyph.id < glyphRange.first || glyph.id > glyphRange.second) {
-                continue;
+                glyph.metrics.advance >= 256 || glyph.id < glyphRange.first ||
+                glyph.id > glyphRange.second) {
+                return;
             }
 
             // If the area of width/height is non-zero, we need to adjust the expected size
             // with the implicit border size, otherwise we expect there to be no bitmap at all.
             if (glyph.metrics.width && glyph.metrics.height) {
-                const Size size {
-                    glyph.metrics.width + 2 * Glyph::borderSize,
-                    glyph.metrics.height + 2 * Glyph::borderSize
-                };
+                const Size size{ glyph.metrics.width + 2 * Glyph::borderSize,
+                                 glyph.metrics.height + 2 * Glyph::borderSize };
 
                 if (size.area() != glyphData.size()) {
-                    continue;
+                    return;
                 }
 
-                glyph.bitmap = AlphaImage(size, reinterpret_cast<const uint8_t*>(glyphData.data()), glyphData.size());
+                glyph.bitmap = AlphaImage(size, reinterpret_cast<const uint8_t*>(glyphData.data()),
+                                          glyphData.size());
             }
 
             result.push_back(std::move(glyph));
+        };
+
+        double ascender{ 0.0 }, descender{ 0.0 };
+        uint16_t count{ 0 };
+        auto fontstack_pbf = glyphs_pbf.get_message();
+        while (fontstack_pbf.next()) {
+            switch (fontstack_pbf.tag()) {
+            case 3: {
+                readGlyphMetrics(fontstack_pbf);
+                ++count;
+                break;
+            }
+            case 4: {
+                ascender = fontstack_pbf.get_double();
+                break;
+            }
+            case 5: {
+                descender = fontstack_pbf.get_double();
+                break;
+            }
+            default: {
+                fontstack_pbf.skip();
+                break;
+            }
+            }
+        }
+        if (ascender != 0.0 || descender != 0.0) {
+            assert(count <= result.size());
+            for (uint16_t i = result.size() - count; i <= result.size() - 1; ++i) {
+                result[i].metrics.ascender = ascender;
+                result[i].metrics.descender = descender;
+            }
         }
     }
 
diff --git a/src/mbgl/text/shaping.cpp b/src/mbgl/text/shaping.cpp
index d6d9a3d34e..d62401c97e 100644
--- a/src/mbgl/text/shaping.cpp
+++ b/src/mbgl/text/shaping.cpp
@@ -326,8 +326,7 @@ void shapeLines(Shaping& shaping,
                 const GlyphMap& glyphMap,
                 bool allowVerticalPlacement) {
     float x = 0;
-    float y = Shaping::yOffset;
-    
+    float y = 0;
     float maxLineLength = 0;
 
 
@@ -359,21 +358,25 @@ void shapeLines(Shaping& shaping,
             if (it == glyphs->second.end() || !it->second) {
                 continue;
             }
-            
-            // We don't know the baseline, but since we're laying out
-            // at 24 points, we can calculate how much it will move when
-            // we scale up or down.
-            const double baselineOffset = (lineMaxScale - section.scale) * util::ONE_EM;
-            
+
             const Glyph& glyph = **it->second;
 
+            // Each glyph's baseline is starting from its acsender, which is the vertical distance
+            // from the horizontal baseline to the highest ‘character’ coordinate in a font face.
+            // Since we're laying out at 24 points, we need also calculate how much it will move
+            // when we scale up or down.
+            const double baselineOffset = -glyph.metrics.ascender * section.scale +
+                                          (lineMaxScale - section.scale) * util::ONE_EM;
+
             if (writingMode == WritingModeType::Horizontal ||
                 // Don't verticalize glyphs that have no upright orientation if vertical placement is disabled.
                 (!allowVerticalPlacement && !util::i18n::hasUprightVerticalOrientation(codePoint)) ||
                 // If vertical placement is ebabled, don't verticalize glyphs that
                 // are from complex text layout script, or whitespaces.
                 (allowVerticalPlacement && (util::i18n::isWhitespace(codePoint) || util::i18n::isCharInComplexShapingScript(codePoint)))) {
-                shaping.positionedGlyphs.emplace_back(codePoint, x, y + baselineOffset, false, section.fontStackHash, section.scale, sectionIndex);
+                shaping.positionedGlyphs.emplace_back(codePoint, x, y + baselineOffset, false,
+                                                      section.fontStackHash, section.scale,
+                                                      sectionIndex);
                 x += glyph.metrics.advance * section.scale + spacing;
             } else {
                 shaping.positionedGlyphs.emplace_back(codePoint, x, y + baselineOffset, true, section.fontStackHash, section.scale, sectionIndex);
@@ -399,7 +402,6 @@ void shapeLines(Shaping& shaping,
     align(shaping, justify, anchorAlign.horizontalAlign, anchorAlign.verticalAlign, maxLineLength,
           lineHeight, lines.size());
     const float height = y - Shaping::yOffset;
-
     // Calculate the bounding box
     shaping.top += -anchorAlign.verticalAlign * height;
     shaping.bottom = shaping.top + height;