Adds CustomTypeface and Removes GlyphFace #640

richardshan0614 · 2025-06-11T07:53:46Z

1.添加CustomTypeface, CustomTypefaceBuilder
2.删除GlyphFace

…ture/customTypeface

Copilot

Pull Request Overview

This PR removes the legacy GlyphFace implementation and replaces it with a new custom typeface API based on Font and CustomTypefaceBuilder. The changes include renaming and refactoring in core text rendering components, removal of GlyphFace-based methods, and updates to constructors and helper functions to use the new API.

Removed GlyphFace-related functionality and headers.
Updated TextBlob, GlyphRun*, Font, and Canvas to use Font-based APIs.
Added new custom typeface builder interfaces (PathTypefaceBuilder and ImageTypefaceBuilder) and extended Typeface.

Reviewed Changes

Copilot reviewed 56 out of 56 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/core/TextBlob.cpp	Refactored creation methods to use Font instead of GlyphFace; renamed enum.
src/core/ScalerContext.*	Removed legacy Make() method; updated calls to use createScalerContext.
src/core/GlyphRunList.{h,cpp}	Updated to use Font APIs for glyph rendering and bounds extraction.
src/core/Font.cpp	Updated scaler context creation with new typeface API.
src/core/Canvas.cpp	Replaced drawing functions to use Font-based glyph runs.
include/tgfx/core/{Typeface.h,GlyphRun.h}	Modified API signatures to accommodate Font instead of GlyphFace.
include/tgfx/core/{PathTypefaceBuilder.h,ImageTypefaceBuilder.h,CustomTypefaceBuilder.h}	Added new custom typeface builder interfaces.
CMakeLists.txt	Updated source file inclusion for new custom typeface builder implementations.

Comments suppressed due to low confidence (3)

src/core/TextBlob.cpp:75

[nitpick] The enum rename from GlyphFaceType to FontType is clear; please update any remaining comments or documentation referencing GlyphFaceType for consistency.

enum class FontType { Path, Color, Other };

include/tgfx/core/GlyphRun.h:37

[nitpick] Update the documentation for this constructor to mention that it now directly uses a Font instance instead of wrapping a GlyphFace, ensuring users are aware of the API change.

GlyphRun(Font font, std::vector<GlyphID> glyphIDs, std::vector<Point> positions)

src/core/GlyphRunList.cpp:42

Consider adding an assertion or guard to ensure that each GlyphRun's 'font' member is valid before calling hasColor(), to mitigate the risk of a null dereference.

[hasColor = _glyphRuns[0].font.hasColor()](const GlyphRun& glyphRun) {

domchen · 2025-06-11T09:19:01Z

include/tgfx/core/CustomTypefaceBuilder.h

+  explicit CustomTypefaceBuilder(const std::string& fontFamily = "",
+                                 const std::string& fontStyle = "",
+                                 const FontMetrics& metrics = {});


不需要这个构造函数。已经有可选的set方法了。

domchen · 2025-06-11T09:22:37Z

include/tgfx/core/ImageTypefaceBuilder.h

参考Skia，把ImageTypefaceBuilder和PathTypefaceBuilder都移动到CustomTypefaceBuilder同一个头文件里。并且把头文件命名为CustomTypeface.h

domchen · 2025-06-11T09:26:04Z

include/tgfx/core/PathTypefaceBuilder.h

+  std::shared_ptr<Typeface> detach() const override;
+
+ private:
+  std::vector<std::shared_ptr<GlyphRecord>> glyphRecords;


随手赋初始值

domchen · 2025-06-11T10:19:00Z

CMakeLists.txt

@@ -126,6 +126,7 @@ file(GLOB_RECURSE SRC_FILES
        src/core/shaders/*.*
        src/core/shapes/*.*
        src/core/utils/*.*
+        src/core/vectors/custom/*.*


这个不应该放在vectors下面，不属于任何一个后端，custom的直接放在core包下面就可以了。

domchen · 2025-06-11T10:21:22Z

include/tgfx/core/ImageTypefaceBuilder.h

+ */
+class ImageTypefaceBuilder : public CustomTypefaceBuilder {
+ public:
+  struct GlyphRecord {  // logical union


这个结构体不要放在public区域暴露。可以直接挪到Cpp里定义。头文件这边只做个struct GlyphRecord;的声明。改为ImageGlyphRecord就行了，不用内嵌。

domchen · 2025-06-11T10:32:48Z

include/tgfx/core/PathTypefaceBuilder.h

+  struct GlyphRecord {
+    Path path = {};
+    GlyphRecord(const Path& path) : path(path) {
+    }
+  };


这个结构体有两个问题：1.不应该是存储Path，这样就被迫所有的PathProvider提前实例化了。会阻塞当前线程，并且占用过多内存。应该是存储PathProvider才对，把普通Path也包装为一个PathProvider。 2.只有一个属性，是不需要定义这个GlyphRecord的，直接用PathProvider的数组存储就行。

domchen · 2025-06-11T10:33:25Z

include/tgfx/core/Typeface.h

+  /**
+   *  Returns a ScalerContext for the given size.
+   */
+  virtual std::shared_ptr<ScalerContext> createScalerContext(float size);


这个不能是public方法，改为private，然后友元访问。也不需要virtual吧？子类覆盖的是onCreateScalerContext()方法不是这个。

如果有个onCreate方法了。这个方法建议命名为getScalerContext()

domchen · 2025-06-11T10:39:17Z

include/tgfx/core/Typeface.h

+  /**
+   * Returns the active ID for this typeface. for custom typefaces, this is the builderID, or returns uniqueID.
+   */
+  virtual uint32_t getActiveID() const;


getCacheID()好一点。注释更新一下：Returns the cache ID for this typeface. Typically, this is the uniqueID of the typeface; for custom typefaces, it returns the uniqueID of the associated typeface builder.

domchen · 2025-06-11T10:45:49Z

src/core/Typeface.cpp

+std::shared_ptr<ScalerContext> Typeface::createScalerContext(float size) {
+  if (weakThis.lock() == nullptr || glyphsCount() <= 0 || size < 0.0f) {


前两个不应该判断，前两个条件是只要Typeface对象存在就不应该无效。第三个条件要改成 size <= 0.0f.

domchen · 2025-06-11T10:50:25Z

src/core/vectors/custom/CustomTypeFaceBuilder.cpp

+  if (firstTime) {
+    _fontMetrics.top = bounds.top;
+    _fontMetrics.bottom = bounds.bottom;
+    _fontMetrics.xMin = bounds.left;
+    _fontMetrics.xMax = bounds.right;
+  } else {
+    _fontMetrics.top = std::min(_fontMetrics.top, bounds.top);
+    _fontMetrics.bottom = std::max(_fontMetrics.bottom, bounds.bottom);
+    _fontMetrics.xMin = std::min(_fontMetrics.xMin, bounds.left);
+    _fontMetrics.xMax = std::max(_fontMetrics.xMax, bounds.right);
+  }


这个方法是没有必要的，用户不设置metrics我们不用自己计算。

domchen · 2025-06-11T10:54:47Z

src/core/vectors/custom/ImageTypeface.h

+#include "tgfx/core/Typeface.h"
+
+namespace tgfx {
+class ImageTypeface final : public Typeface {


这个命名有点歧义，改为ImageUserTypeface吧，对应把PathTypeface改一下，以及其他相关的类。ImageTypefaceBuilder因为存在TypefaceBuilder的标识，还是能比较好区分出来是自定义字体的类，可以不改。

domchen · 2025-06-11T10:55:38Z

src/core/vectors/custom/ImageTypeface.h

+  uint32_t uniqueID() const override {
+    return _uniqueID;
+  }
+
+  std::string fontFamily() const override {
+    return _fontFamily;
+  }
+
+  std::string fontStyle() const override {
+    return _fontStyle;
+  }
+
+  const FontMetrics& fontMetrics() const {
+    return _fontMetrics;
+  }


建议是加个UserTypeface的基类，把公共的属性都实现到一起。避免代码存在重复。对应的ScalerContext也是。

domchen · 2025-06-11T10:57:57Z

src/core/vectors/custom/PathScalerContext.cpp

+static constexpr float StdFakeBoldInterpKeys[] = {
+    9.f,
+    36.f,
+};
+static constexpr float StdFakeBoldInterpValues[] = {
+    1.f / 24.f,
+    1.f / 32.f,
+};
+
+inline float Interpolate(const float& a, const float& b, const float& t) {
+  return a + (b - a) * t;
+}
+
+/**
+ * Interpolate along the function described by (keys[length], values[length])
+ * for the passed searchKey. SearchKeys outside the range keys[0]-keys[Length]
+ * clamp to the min or max value. This function assumes the number of pairs
+ * (length) will be small and a linear search is used.
+ *
+ * Repeated keys are allowed for discontinuous functions (so long as keys is
+ * monotonically increasing). If key is the value of a repeated scalar in
+ * keys the first one will be used.
+ */
+static float FloatInterpFunc(float searchKey, const float keys[], const float values[],


这部分计算粗体的代码不要重复，提取一个工具类出来。任何时候都不应该写重复的代码。不好维护容易出bug。

domchen · 2025-06-11T10:58:43Z

src/core/vectors/custom/PathScalerContext.cpp

+  if (record == nullptr || record->path.isEmpty()) {
+    return false;
+  }
+  auto bounds = PathScalerContext::getImageTransform(glyphID, fauxBold, stroke, nullptr);


这个getImageTransform的前缀PathScalerContext::是没必要的

domchen · 2025-06-11T11:02:50Z

src/core/vectors/web/WebScalerContext.cpp

@@ -75,7 +61,7 @@ Rect WebScalerContext::getImageTransform(GlyphID glyphID, bool fauxBold, const S
    return {};
  }
  if (!hasColor() && stroke != nullptr) {
-    stroke->applyToBounds(&bounds);
+    ApplyStrokeToBounds(*stroke, &bounds, true);


这些都是流水线应该检查出来的，应该是npm那边的编译命令默认改成了编译多线程的原因，这块问题无法暴露了。到.github/workflows/build.xml 里找下web平台的编译命令，把 npm run build:debug 改为 npm run build:st:debug

domchen · 2025-06-11T11:06:02Z

test/src/TypefaceTest.cpp

+#include "utils/TestUtils.h"
+
+namespace tgfx {
+TGFX_TEST(TypefaceTest, CustomPathTypeface) {


应该主要构造继承PathProvider的测试用例，写个自定义的PathProvider子类，里面就是直接用那些lineTo之类的临时创建Path的代码。另外可以给PathProvider加个静态的Wrap方法，用来快速包装Path。

richardshan0614 added 30 commits June 9, 2025 16:53

Adds custom typeface

76024df

Merge remote-tracking branch 'origin/main' into feature/customTypeface

b1b81f7

Code tidy

44ab4e3

Code tidy

f45211a

Merge remote-tracking branch 'origin/feature/customTypeface' into fea…

31093c7

…ture/customTypeface

Remove Glyphface

461d964

Fix compile error

0242cc5

Removes Glyphface test, Adds TypefaceTest

008f4a4

Merge remote-tracking branch 'origin/main' into feature/customTypeface

26123e9

Fix customFont check error

92f3f13

Add ImageTypeface test

e3cadf9

src

2d8815e

ImageScalerContext support scale

68353f1

Code tidy

528b50b

Fix nullptr

94f3693

Code tidy

3689f13

Fix fauxItalic error

14df037

Code tidy

93e32f3

Code tidy

ce969eb

Code tidy

998bc34

Code format

fd4f356

Remove unused code

4d9296a

Code tidy

0c21115

Init builderID for imageTypeface

3bbdfc7

Code tidy

7f3ec92

Code tidy

1ceac08

Code tidy

8939a00

Make pathScalerContext final

65c3cba

Code tidy

8e8433c

Code tidy

7a4228b

richardshan0614 added 3 commits June 11, 2025 15:22

Code format

d296bed

Code format

8f1efb8

PathScalerContext::readpixels path should scale

e0180b3

Copilot AI review requested due to automatic review settings June 11, 2025 07:53

richardshan0614 requested review from kevingpqi123, domchen, Hparty and shlzxjp as code owners June 11, 2025 07:53

Copilot AI reviewed Jun 11, 2025

View reviewed changes

domchen reviewed Jun 11, 2025

View reviewed changes

richardshan0614 added 2 commits June 11, 2025 17:55

Remove unnecessary parameters

91489ce

Initialize member variables

e8cd5e4

domchen reviewed Jun 11, 2025

View reviewed changes

richardshan0614 added 3 commits June 11, 2025 21:31

Code tidy

ebc0e4e

Code tidy

8fd3f24

Merge main'

f0e406b

		std::shared_ptr<ScalerContext> Typeface::createScalerContext(float size) {
		if (weakThis.lock() == nullptr \|\| glyphsCount() <= 0 \|\| size < 0.0f) {

Adds CustomTypeface and Removes GlyphFace #640

Are you sure you want to change the base?

Adds CustomTypeface and Removes GlyphFace #640

Uh oh!

Conversation

richardshan0614 commented Jun 11, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

domchen Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

domchen Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

domchen Jun 11, 2025 •

edited

Loading

domchen Jun 11, 2025 •

edited

Loading