Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement GlyphFace rendering capabilities #371

Merged
merged 22 commits into from
Dec 10, 2024
Merged

Conversation

shlzxjp
Copy link
Collaborator

@shlzxjp shlzxjp commented Dec 9, 2024

实现GlyphFace渲染能力,用于渲染外部提供的文字path和emoji。通过GlyphFace隔离Font渲染和排版,统一内外部文本渲染流程。

@@ -19,6 +19,7 @@
#include "CustomLayer.h"
#include "base/LayerTreeDrawers.h"
#include "drawers/AppHost.h"
#include "tgfx/core/FontGlyphFace.h"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个接口我们不公开,属于内部实现。

@@ -66,7 +67,8 @@ std::unique_ptr<tgfx::LayerContent> CustomLayer::onUpdateContent() {
xOffset += emptyAdvance;
}
}
tgfx::GlyphRun glyphRun(_font, std::move(glyphs), std::move(positions));
tgfx::GlyphRun glyphRun(std::make_shared<tgfx::FontGlyphFace>(_font), std::move(glyphs),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

还是让它正常通过font可以构造,我们有两个构造函数,内部转成FontGlyphFace就行。

@@ -279,6 +280,18 @@ class Canvas {
void drawGlyphs(const GlyphID glyphs[], const Point positions[], size_t glyphCount,
const Font& font, const Paint& paint);

/**
* Draws an array of glyphIDs using GlyphFace, with paths and images provided by GlyphFace and
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

注释修改一下:Draws an array of glyphs from glyphIDs at positions using clip, matrix, glyphFace, and paint. GlyphFace的类注释上可以详细再写它是提供Path和Image的这些相关信息

*/
GlyphRun(Font font, std::vector<GlyphID> glyphIDs, std::vector<Point> positions)
: font(font), glyphs(glyphIDs), positions(positions) {
GlyphRun(std::shared_ptr<GlyphFace> glyphFace, std::vector<GlyphID> glyphIDs,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

不要删除原本的Font构造函数,提供两个版本,和Canvas上一样。

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

重命名一下文件吧,glyph1, glyph2, glyph3

* @param paint Blend, color, and so on, used to draw.
* @note The lengths of `glyphIDs` and `positions` must be the same.
*/
void drawGlyphs(const std::vector<GlyphID>& glyphIDs, const std::vector<Point>& positions,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

除了GlyphFace参数不一样,其他都保持和另外一个drawGlyphs方法的参数一致,这里不用std::vector<>是为了减少拷贝,不然上层业务如果不是用std:vector的数据结构,还得拷贝构造一个,然后内部又多拷贝一次。用指针的方式传进来比较通用,只有内部拷贝一次。

/**
* Returns the Font if this GlyphFace is a FontGlyphFace, otherwise returns std::nullopt.
*/
virtual std::optional<Font> asFont() const = 0;
Copy link
Collaborator

@domchen domchen Dec 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

可以定义成 virtual bool asFont(Font* font) const = 0; 这样方便一个if同时取出Font以及失败的return。

@@ -333,12 +332,31 @@ void Canvas::drawGlyphs(const GlyphID glyphs[], const Point positions[], size_t
if (glyphCount == 0 || paint.nothingToDraw()) {
return;
}
GlyphRun glyphRun(font, {glyphs, glyphs + glyphCount}, {positions, positions + glyphCount});
GlyphRun glyphRun(std::make_shared<FontGlyphFace>(font), {glyphs, glyphs + glyphCount},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个原版的drawGlyphs不用单独实现了,就直接创建一下FontGlyphFace,然后转发给另一个函数统一处理。

if (hasScale) {
// Scale the glyphs before measuring to prevent precision loss with small font sizes.
font = font.makeWithSize(font.getSize() * resolutionScale);
glyphFace = glyphFace->makeScaled(resolutionScale);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这后面补充一个DEBUG_ASSERT(), 防止外部继承的返回nullptr。

Comment on lines 54 to 56
if (_glyphRuns.empty()) {
return false;
}
Copy link
Collaborator

@domchen domchen Dec 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里不用再判空了,GlyphRunList是内部类,在所有创建它的实例之前都已经做过了判空,这里直接访问就好了。包括glyphFace也不用在判空了。

Comment on lines 89 to 91
if (glyphFace == nullptr) {
continue;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个也没有必要再判空了。内部类,应该在构造它之前就做好判空,另外我们还有构造函数里的DEBUG_ASSERT二次验证。

Comment on lines 65 to 69
if (_glyphRuns.empty()) {
return false;
}
const auto& glyphFace = _glyphRuns[0].glyphFace;
return glyphFace ? glyphFace->hasOutlines() : false;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这边同上,不需要多余的判空。

@@ -71,13 +73,18 @@ std::shared_ptr<TextBlob> TextBlob::MakeFrom(GlyphRun glyphRun) {

enum class FontType { Path, Color, Other };

static FontType GetFontType(const Font& font) {
if (font.hasColor()) {
static FontType GetFontType(const std::shared_ptr<GlyphFace>& font) {
Copy link
Collaborator

@domchen domchen Dec 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

变量名叫face吧。返回值可以重命名为GlyphFaceType

Comment on lines 191 to 197
auto glyphFace = glyphRun.glyphFace;
if (glyphFace == nullptr || glyphFace->asFont() == std::nullopt) {
continue;
}

const auto& font = glyphFace->asFont();
if (font->isFauxBold() || font->getTypeface() == nullptr) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

你应该在构造FontGlyphFace的时候就不允许Font->getTypeface()==nullptr的Font对象能够成功构造一个GlyphFace,这样可以简化后续的判断。所以FontGlyphFace那边加个静态方法来构造吧,隐藏掉构造函数。这样确保只要GlyphFace不为空,它就应该是有效的。

Comment on lines 218 to 221
auto glyphFace = glyphRun.glyphFace;
if (glyphFace == nullptr || glyphFace->asFont() == std::nullopt) {
continue;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

前面已经做过判空了,这里不用重复判空,应该是直接asFont,然后加个DEBUG_ASSERT不允结果失败。因为会走到里的只有CoreGraphics生成的Font。

Comment on lines 268 to 270
if (glyphFace == nullptr) {
continue;
}
Copy link
Collaborator

@domchen domchen Dec 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

能被我们内部类GlyphRunList包装进来的GlyphRun就是都判空过了,这里不需要判空。有需要可以考虑再加个DEBUG_ASSERT确保提醒到位。

@@ -422,7 +423,7 @@ void TextLayer::buildGlyphRunList(const std::vector<std::shared_ptr<GlyphInfo>>&
if (glyphRunMap.find(typefaceID) == glyphRunMap.end()) {
auto font = _font;
font.setTypeface(typeface);
glyphRunMap[typefaceID] = GlyphRun(font, {}, {});
glyphRunMap[typefaceID] = GlyphRun(std::make_shared<FontGlyphFace>(font), {}, {});
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里还原一下,用原来的GlyphRun构造函数就行。

Comment on lines 28 to 30
/**
* GlyphFace is a provider for glyphs. It provides the glyph path, image, and bounds, etc.
*/
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

修改一下注释: GlyphFace is a render-only font that contains only the necessary information to render glyphs. It can be implemented externally to render glyphs from a custom font or used as a wrapper around a Font object.


namespace tgfx {
/**
* GlyphFace is a render-only font that contains only the necessary information to render glyphs. It can be implemented externally to render glyphs from a custom font or used as a wrapper around a Font object.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

忘记换行了

Comment on lines 36 to 41
* Returns true if the font has color glyphs, for example, color emojis.
*/
virtual bool hasColor() const = 0;

/**
* Returns true if the font has outline glyphs, meaning it can generate paths.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

更新注释里的 the font 为 the glyph face

Comment on lines 69 to 71
* Returns the font object represented by this GlyphFace.
* If the GlyphFace is not a font object, returns false and sets the font pointer to null.
* If the GlyphFace is a font object, returns true and sets the font pointer to the font object.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

修改注释: Checks if the GlyphFace is backed by a Font object. If so, sets the font pointer to the backing Font object and returns true. Otherwise, returns false and leaves the font pointer unchanged.

namespace tgfx {
class FontGlyphFace final : public GlyphFace {
public:
static std::shared_ptr<FontGlyphFace> Make(const Font& font);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个方法重命名为Wrap,移动到父类GlyphFace上作为公开API,之前GlyphRun的修改可以还原回去。

Comment on lines 58 to 60
if (glyphFace == nullptr) {
continue;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里不需要判空,已经判空过了。只有后面那句makeScaled之后防止意外加个DEBUG_ASSERT判空。

for (auto& run : glyphRuns) {
if (run.glyphs.size() != run.positions.size()) {
return nullptr;
}
if (run.glyphs.empty()) {
continue;
}
auto currentFontType = GetFontType(run.font);
if (currentFontType != fontType) {
auto currentFontType = GetGlyphFaceType(run.glyphFace);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里要补充上glyphFace==nullptr的 return判断。我们之前GlyphRunList里并没有判断Font是否为空。现在内部到处都用到了这个判断,所以所有创建GlyphRunList的地方都要检查一遍,补上内部glyphFace属性的判空。GlyphRunList构造函数里的DEBUG_ASSERT只是最后的提醒措施,外部构造的地方还是要都检查一遍。

if (font.isFauxBold() || font.getTypeface() == nullptr) {
Font font;
bool success = glyphRun.glyphFace->asFont(&font);
if (!success || font.isFauxBold() || font.getTypeface() == nullptr) {
Copy link
Collaborator

@domchen domchen Dec 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这句里的font.getTypeface()可以不用判断了,因为Wrap的结果是如果typeface为空,不会得到一个有效的GlyphFace。
但这里的success确实是需要判断的,因为可能有外部的自定义GlyphFace进入到这里,这里asFont失败了return,上层的Mask就会调用getPath方法再去尝试绘制。

Comment on lines 216 to 222
Font font;
bool success = glyphRun.glyphFace->asFont(&font);
if (!success) {
CGContextRestoreGState(cgContext);
continue;
}
DEBUG_ASSERT(font.getTypeface() != nullptr);
Copy link
Collaborator

@domchen domchen Dec 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这边的判断不需要,上面已经for循环判断过,只要存在一个asFont失败就会return。所以直接asFont就行了,不用声明返回值success。后面的DEBUG_ASSERT也不需要。typeface永远不会为空。只单独给success加DEBUG_ASSERT会导致release版本里出现变量没有使用的报错。所以这边success变量最好不要声明。

@shlzxjp shlzxjp requested a review from domchen December 10, 2024 08:12
@domchen domchen force-pushed the feature/xjp_glyph_face branch from 7eec5cb to f5f46a2 Compare December 10, 2024 09:09
@domchen domchen merged commit 6fecb4a into main Dec 10, 2024
8 checks passed
@domchen domchen deleted the feature/xjp_glyph_face branch December 10, 2024 09:20
Fjie added a commit that referenced this pull request Dec 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants