From 6d7b22f148d5aa6ad1ff845d7dd6f1768bf61486 Mon Sep 17 00:00:00 2001 From: Christian Kaiser Date: Sat, 17 Aug 2024 05:22:50 -0300 Subject: [PATCH 1/3] Improve skia text shaping/bounds calculation --- text/skia_text_blob_shaper.cpp | 35 +++++++++++++++++++++++++--------- text/text_blob.cpp | 6 ++++-- 2 files changed, 30 insertions(+), 11 deletions(-) diff --git a/text/skia_text_blob_shaper.cpp b/text/skia_text_blob_shaper.cpp index d2b3ff5c2..19bb56404 100644 --- a/text/skia_text_blob_shaper.cpp +++ b/text/skia_text_blob_shaper.cpp @@ -34,7 +34,8 @@ class ShaperRunHandler final : public SkShaper::RunHandler { const gfx::PointF& offset, TextBlob::RunHandler* subHandler) : m_builder(utf8Text, os::to_skia(offset)) - , m_subHandler(subHandler) { } + , m_subHandler(subHandler) + , m_buffer() { } sk_sp makeBlob() { return m_builder.makeBlob(); @@ -131,17 +132,33 @@ TextBlobRef SkiaTextBlob::MakeWithShaper( ASSERT(dynamic_cast(font.get())); SkFont skFont = static_cast(font.get())->skFont(); + auto skFontMgr = static_cast(fontMgr.get())->skFontMgr(); sk_sp textBlob; gfx::RectF bounds; - if (auto shaper = SkShaper::Make( - static_cast(fontMgr.get())->skFontMgr())) { + if (auto shaper = SkShaper::Make(skFontMgr)) { ShaperRunHandler shaperHandler(text.c_str(), { 0, 0 }, handler); - shaper->shape( - text.c_str(), text.size(), - skFont, - true, - std::numeric_limits::max(), - &shaperHandler); + + auto bidiRun = SkShaper::MakeBiDiRunIterator(text.c_str(), text.size(), 0xfe); + constexpr SkFourByteTag tag = SkSetFourByteTag('Z', 'y', 'y', 'y'); + auto scriptRun = SkShaper::MakeScriptRunIterator(text.c_str(), text.size(), tag); + auto languageRun = SkShaper::MakeStdLanguageRunIterator(text.c_str(), text.size()); + auto fontRun = SkShaper::MakeFontMgrRunIterator(text.c_str(), + text.size(), + skFont, + skFontMgr, + "Arial", // Fallback + SkFontStyle::Normal(), + &*languageRun); + + shaper->shape(text.c_str(), + text.size(), + *fontRun, + *bidiRun, + *scriptRun, + *languageRun, + std::numeric_limits::max(), + &shaperHandler); + textBlob = shaperHandler.makeBlob(); bounds = shaperHandler.bounds(); } diff --git a/text/text_blob.cpp b/text/text_blob.cpp index c106e5a87..2f3c43e17 100644 --- a/text/text_blob.cpp +++ b/text/text_blob.cpp @@ -69,10 +69,12 @@ gfx::RectF TextBlob::RunInfo::getGlyphBounds(const size_t i) const if (bounds.isEmpty()) { FontMetrics metrics; font->metrics(&metrics); - bounds.w = metrics.avgCharWidth; - bounds.h = -metrics.capHeight; + // avgCharWidth can be 0, so we grab the next most useful thing, the height + bounds.w = metrics.avgCharWidth > 0 ? metrics.avgCharWidth : metrics.xHeight; + bounds.h = 1.0; } + ASSERT(!bounds.isEmpty()); bounds.offset(positions[i].x, positions[i].y); if (offsets) { From 51e69d7ec1dfd98c665f7a91f9b7e5b34ecc0033 Mon Sep 17 00:00:00 2001 From: Christian Kaiser Date: Tue, 20 Aug 2024 04:08:24 -0300 Subject: [PATCH 2/3] Ceil the float spacing to avoid early cut-off of some glyphs --- text/skia_font.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/text/skia_font.cpp b/text/skia_font.cpp index b1b4a731a..0afaf13c1 100644 --- a/text/skia_font.cpp +++ b/text/skia_font.cpp @@ -71,11 +71,13 @@ int SkiaFont::height() const int SkiaFont::textLength(const std::string& str) const { - return m_skFont.measureText( - str.c_str(), - str.size(), - SkTextEncoding::kUTF8, - nullptr); + return std::ceil( + m_skFont.measureText( + str.c_str(), + str.size(), + SkTextEncoding::kUTF8, + nullptr) + ); } float SkiaFont::measureText(const std::string& str, From d1dc97df7dcdddd8fa3296224ae7d3cca20e7b7d Mon Sep 17 00:00:00 2001 From: Christian Kaiser Date: Thu, 22 Aug 2024 04:05:29 -0300 Subject: [PATCH 3/3] Whitespace bounds from space glyph --- text/skia_font.cpp | 3 +-- text/text_blob.cpp | 17 +++++++---------- text/text_blob.h | 6 +++--- 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/text/skia_font.cpp b/text/skia_font.cpp index 0afaf13c1..ee93cf87f 100644 --- a/text/skia_font.cpp +++ b/text/skia_font.cpp @@ -124,9 +124,8 @@ glyph_t SkiaFont::codePointToGlyph(codepoint_t codepoint) const gfx::RectF SkiaFont::getGlyphBounds(glyph_t glyph) const { - float widths; SkRect bounds; - m_skFont.getWidthsBounds(&glyph, 1, &widths, &bounds, nullptr); + m_skFont.getWidthsBounds(&glyph, 1, nullptr, &bounds, nullptr); return os::from_skia(bounds); } diff --git a/text/text_blob.cpp b/text/text_blob.cpp index 2f3c43e17..9e0b9672d 100644 --- a/text/text_blob.cpp +++ b/text/text_blob.cpp @@ -11,6 +11,7 @@ #include "text/text_blob.h" #include "gfx/rect.h" +#include "gfx/size.h" #include "text/font.h" #include "text/font_metrics.h" #include "text/sprite_text_blob.h" @@ -65,21 +66,17 @@ gfx::RectF TextBlob::RunInfo::getGlyphBounds(const size_t i) const gfx::RectF bounds = font->getGlyphBounds(glyphs[i]); - // Get bounds of whitespace + // Get bounds of whitespace from a space glyph. if (bounds.isEmpty()) { - FontMetrics metrics; - font->metrics(&metrics); - // avgCharWidth can be 0, so we grab the next most useful thing, the height - bounds.w = metrics.avgCharWidth > 0 ? metrics.avgCharWidth : metrics.xHeight; - bounds.h = 1.0; + auto glyph = font->getGlyphBounds(' '); + bounds.w = glyph.w - glyph.x; + bounds.h = glyph.h - glyph.y; } ASSERT(!bounds.isEmpty()); - bounds.offset(positions[i].x, - positions[i].y); + bounds.offset(positions[i]); if (offsets) { - bounds.offset(offsets[i].x, - offsets[i].y); + bounds.offset(offsets[i]); } // Add global "point" offset to the bounds. diff --git a/text/text_blob.h b/text/text_blob.h index 7bb3c6f6b..bbffd0ac3 100644 --- a/text/text_blob.h +++ b/text/text_blob.h @@ -59,12 +59,12 @@ namespace text { class RunHandler { public: - virtual ~RunHandler() { } + virtual ~RunHandler() = default; virtual void commitRunBuffer(RunInfo& info) = 0; }; - TextBlob(const gfx::RectF& bounds) : m_bounds(bounds) { } - virtual ~TextBlob() { } + explicit TextBlob(const gfx::RectF& bounds) : m_bounds(bounds) {} + virtual ~TextBlob() = default; // Returns exact bounds that are required to draw this TextBlob. gfx::RectF bounds();