Skip to content

Commit

Permalink
Fix the NPR issue when Font.getTypeface() returns nullptr. (#2266)
Browse files Browse the repository at this point in the history
  • Loading branch information
domchen committed Apr 23, 2024
1 parent b6ce2a5 commit 251d408
Show file tree
Hide file tree
Showing 10 changed files with 21 additions and 19 deletions.
2 changes: 1 addition & 1 deletion DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
},
{
"url": "${PAG_GROUP}/tgfx.git",
"commit": "ab0937d87b579ebbe74028e6f8068da351a68d53",
"commit": "2d4c22ab50fe738d221025c1b68812e7b8d6f232",
"dir": "third_party/tgfx"
},
{
Expand Down
2 changes: 1 addition & 1 deletion src/platform/web/NativeTextShaper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ PositionedGlyphs NativeTextShaper::Shape(const std::string& text,
continue;
}
auto str = text.substr(infos[i].cluster, length);
auto glyphID = typeface->getGlyphID(str);
auto glyphID = typeface ? typeface->getGlyphID(str) : 0;
if (glyphID == 0) {
for (const auto& faceHolder : fallbackTypefaces) {
auto face = faceHolder->getTypeface();
Expand Down
3 changes: 2 additions & 1 deletion src/rendering/caches/TextAtlas.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,8 @@ static AtlasTextRun CreateTextRun(const GlyphHandle& glyph) {
static void ComputeStyleKey(tgfx::BytesKey* styleKey, const GlyphHandle& glyph) {
styleKey->write(static_cast<uint32_t>(glyph->getStyle()));
styleKey->write(glyph->getStrokeWidth());
styleKey->write(glyph->getFont().getTypeface()->uniqueID());
auto typeface = glyph->getFont().getTypeface();
styleKey->write(typeface ? typeface->uniqueID() : 0);
}

struct Page {
Expand Down
2 changes: 1 addition & 1 deletion src/rendering/caches/TextBlock.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ TextBlock::TextBlock(ID assetID, std::vector<std::vector<GlyphHandle>> lines, fl
for (const auto& line : this->lines()) {
for (const auto& tempGlyph : line) {
auto glyph = tempGlyph->makeHorizontalGlyph();
auto hasColor = glyph->getFont().getTypeface()->hasColor();
auto hasColor = glyph->getFont().hasColor();
auto style = glyph->getStyle();
if (hasColor && (style == TextStyle::Stroke || style == TextStyle::StrokeAndFill)) {
glyph->setStrokeWidth(0);
Expand Down
3 changes: 1 addition & 2 deletions src/rendering/caches/TextContentCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,7 @@ std::pair<std::vector<GlyphHandle>, std::vector<GlyphHandle>> GetGlyphs(
for (auto& line : glyphLines) {
for (auto& glyph : line) {
simpleGlyphs.push_back(glyph);
auto typeface = glyph->getFont().getTypeface();
if (typeface->hasColor()) {
if (glyph->getFont().hasColor()) {
colorGlyphs.push_back(glyph);
}
}
Expand Down
5 changes: 3 additions & 2 deletions src/rendering/graphics/Glyph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,8 @@ void Glyph::computeStyleKey(tgfx::BytesKey* styleKey) const {
static_cast<uint8_t>(textStyle)};
styleKey->write(strokeValues);
styleKey->write(strokeWidth);
styleKey->write(getFont().getTypeface()->uniqueID());
auto typeface = getFont().getTypeface();
styleKey->write(typeface ? typeface->uniqueID() : 0);
}

bool Glyph::isVisible() const {
Expand All @@ -129,7 +130,7 @@ tgfx::Matrix Glyph::getTotalMatrix() const {
}

void Glyph::computeAtlasKey(tgfx::BytesKey* bytesKey, TextStyle style) const {
bytesKey->write(static_cast<uint32_t>(getFont().getTypeface()->hasColor()));
bytesKey->write(static_cast<uint32_t>(getFont().hasColor()));
bytesKey->write(static_cast<uint32_t>(getGlyphID()));
bytesKey->write(static_cast<uint32_t>(style));
}
Expand Down
2 changes: 1 addition & 1 deletion src/rendering/graphics/Text.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ void Text::draw(Canvas* canvas, const TextAtlas* textAtlas) const {
}
parameters.matrices.emplace_back(matrix);
parameters.rects.emplace_back(locator.location);
if (glyph->getFont().getTypeface()->hasColor()) {
if (glyph->getFont().hasColor()) {
auto alpha = canvas->getAlpha();
canvas->setAlpha(alpha * glyph->getAlpha());
Draw(canvas, textAtlas, parameters);
Expand Down
12 changes: 9 additions & 3 deletions src/rendering/utils/shaper/TextShaperHarfbuzz.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,10 @@ hb_blob_t* HBGetTable(hb_face_t*, hb_tag_t tag, void* userData) {
[](void* ctx) { delete reinterpret_cast<PtrWrapper<tgfx::Data>*>(ctx); });
}

std::shared_ptr<hb_face_t> CreateHBFace(const std::shared_ptr<tgfx::Typeface>& typeface) {
std::shared_ptr<hb_face_t> CreateHBFace(std::shared_ptr<tgfx::Typeface> typeface) {
if (typeface == nullptr) {
return nullptr;
}
std::shared_ptr<hb_face_t> hbFace;
auto data = typeface->getBytes();
if (data && !data->empty()) {
Expand Down Expand Up @@ -142,7 +145,10 @@ static HBLockedFontCache GetHBFontCache() {
return {HBFontLRU, HBFontCache, HBFontCacheMutex};
}

static std::shared_ptr<hb_font_t> CreateHBFont(const std::shared_ptr<tgfx::Typeface>& typeface) {
static std::shared_ptr<hb_font_t> CreateHBFont(std::shared_ptr<tgfx::Typeface> typeface) {
if (typeface == nullptr) {
return nullptr;
}
auto cache = GetHBFontCache();
auto hbFont = cache.find(typeface->uniqueID());
if (hbFont == nullptr) {
Expand All @@ -157,7 +163,7 @@ static std::shared_ptr<hb_font_t> CreateHBFont(const std::shared_ptr<tgfx::Typef
}

static std::vector<std::tuple<uint32_t, uint32_t, uint32_t>> Shape(
const std::string& text, const std::shared_ptr<tgfx::Typeface>& typeface) {
const std::string& text, std::shared_ptr<tgfx::Typeface> typeface) {
auto hbFont = CreateHBFont(typeface);
if (hbFont == nullptr) {
return {};
Expand Down
7 changes: 1 addition & 6 deletions src/rendering/utils/shaper/TextShaperPrimitive.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@ PositionedGlyphs TextShaperPrimitive::Shape(const std::string& text,
tgfx::UTF::NextUTF8(&textStart, textStop);
auto length = textStart - oldPosition;
auto str = std::string(oldPosition, length);
auto glyphID = typeface->getGlyphID(str);
bool found = false;
auto glyphID = typeface ? typeface->getGlyphID(str) : 0;
if (glyphID == 0) {
for (const auto& faceHolder : fallbackTypefaces) {
auto face = faceHolder->getTypeface();
Expand All @@ -43,13 +42,9 @@ PositionedGlyphs TextShaperPrimitive::Shape(const std::string& text,
glyphID = face->getGlyphID(str);
if (glyphID != 0) {
glyphs.emplace_back(std::move(face), glyphID, oldPosition - text.data());
found = true;
break;
}
}
if (!found) {
glyphs.emplace_back(typeface, glyphID, oldPosition - text.data());
}
} else {
glyphs.emplace_back(typeface, glyphID, oldPosition - text.data());
}
Expand Down
2 changes: 1 addition & 1 deletion test/baseline/version.json
Original file line number Diff line number Diff line change
Expand Up @@ -8451,7 +8451,7 @@
"NormalEmoji": "4b7f3114",
"PositionAnimator": "7f7435d6f",
"RangeSelectorTriangleHighLow": "5c3b8bc5",
"SmallFontSizeScale": "4b7f3114",
"SmallFontSizeScale": "b2fcd22b",
"SmallFontSizeScale_LowResolution": "088c7e93",
"TextLayerScaleAnimationWithMipmap": "3bd38676",
"TextPathBox": "7f7435d6f",
Expand Down

0 comments on commit 251d408

Please sign in to comment.