[80_7] Using OpenType glyph variants for rubber character #2119

KeShih · 2024-09-30T11:43:40Z

What?

Utilizing the glyph variant table of OpenType fonts to improve typesetting, specifically, it primarily enhances the typesetting of large mathematical operators. However, some glyph are not included in the table, so they will not be improved.

OpenType glyph variant table provides the glyph IDs for the variants, so we introduced <@xxxx>, which translates the glyph ID into a 4-digit hexadecimal number for representation. This mechanism is only used in rubber_unicode_font.cpp and unicode_font.cpp.

Why?

For many OpenType fonts, such as Asana Math, certain large operators use incorrect fonts or glyphs.

How to test it?

Open devel/80_7.tmu and run xmake run parse_variant_test to test parse_variant(string, string) function.

Before:

After:
The first two are OpenType fonts affected by this change, while the latter two are hardcoded OpenType fonts that remain unchanged.

da-liii · 2024-10-20T13:20:38Z

src/Graphics/Fonts/smart_font.cpp

    if (fn[nr]->supports (c)) return sm->add_char (key, c);
  }
  return -1;
 }

+font
+smart_font_rep::make_rubber_font (font base) {
+  if (occurs ("mathlarge=", res_name) || occurs ("mathrubber=", res_name))


nit: use contains instead of occurs for better readability

da-liii · 2024-10-20T14:04:12Z

src/Plugins/Freetype/rubber_unicode_font.cpp

@@ -71,10 +89,16 @@ rubber_unicode_font_rep::rubber_unicode_font_rep (string name, font base2)
    //<< ((double) (ex->y2-ex->y1)) / base->yx << LF;
    if ((((double) (ex->y2 - ex->y1)) / base->yx) >= 1.55) big_sums= true;
  }
-  for (int i= 0; i < 5; i++) {
+
+  for (int i= 0; i < 6; i++) {


use const and add comment on the magic number 6

da-liii · 2024-10-20T14:05:24Z

src/Plugins/Freetype/rubber_unicode_font.cpp

+parse_variant (string s, string& r) {
+  // cout << "parse_variant for " << s << LF;
+  int var  = 0;
+  int n    = N (s);


nit: typically, we use s_N for N(s), it is ok to use n here, because there only one N() invoke

da-liii · 2024-10-20T14:09:23Z

src/Plugins/Freetype/rubber_unicode_font.cpp

+
+  int          j= 0;
+  unsigned int u= decode_from_utf8 (uu, j);
+  cout << "unicode " << uu << " -> " << lolly::data::to_hex (u) << LF;


should be guarded by debug switch

da-liii · 2024-10-20T14:09:38Z

src/Plugins/Freetype/rubber_unicode_font.cpp

+
+  unsigned int glyphID= ft_get_char_index (math_face->ft_face, u);
+
+  cout << "search_font_sub_opentype for " << u << " -> " << glyphID << LF;


should be guarded by debug switch

da-liii · 2024-10-20T14:10:03Z

src/Plugins/Freetype/unicode_font.hpp

+  int    metric_to_design_unit (SI m);
+};
+
+font rubber_unicode_font (font base, tt_face face);


add the last empty line

da-liii · 2024-10-20T14:14:07Z

src/Plugins/Freetype/rubber_unicode_font.cpp

+    if (var < N (gv)) {
+      int res= gv[var];
+      // use <@XXXX> for native glyph id
+      rew= "<@" * as_hexadecimal (res, 4) * ">";


a very important design, needs to be documented carefully

da-liii · 2024-10-20T14:19:57Z

Suggested title change: Using OpenType glyph variants for rubber character

da-liii · 2024-10-20T14:23:18Z

You may add the following description for your What section:

Defined in smart_font.cpp:

static bool
is_rubber (string c) {
  return (starts (c, "<large-") || starts (c, "<left-") ||
          starts (c, "<right-") || starts (c, "<mid-")) &&
         ends (c, ">");
}

A rubber character is represented as <xxxx> and the xxxx part must starts from large-, left-, right- and mid-.

This pull request introduced a mechanism to render the rubber character (rubber symbol) using <@xxxx>. bla bla bla

da-liii · 2024-10-20T14:24:04Z

Is this issue #1682 fixed by this pull request?

da-liii · 2024-10-20T14:27:35Z

src/Plugins/Freetype/rubber_unicode_font.cpp

+rubber_unicode_font_rep::search_font_sub_opentype (string s, string& rew) {
+  string r;
+  int    var= 0;
+  bool   ver= true; // verizontal or vertical, default is vertical


verizontal -> horizontal

use a longer variable name like using_vertical instead of ver, ver is usually short for version

da-liii · 2024-10-20T14:29:10Z

src/Plugins/Freetype/rubber_unicode_font.cpp

+  string uu= N (r) > 1 ? strict_cork_to_utf8 ("<" * r * ">") : r;
+
+  int          j= 0;
+  unsigned int u= decode_from_utf8 (uu, j);


use uint32_t here, unsigned int is not in fixed bytes

src/Plugins/Freetype/rubber_unicode_font.cpp

da-liii · 2024-10-20T14:38:08Z

src/Plugins/Freetype/rubber_unicode_font.cpp

@@ -105,6 +133,77 @@ rubber_unicode_font_rep::get_font (int nr) {
 * Find the font
 ******************************************************************************/

+int
+parse_variant (string s, string& r) {


unit tests should be added for parse_variant, if you add unit tests for it, you do not need to add the static modifier

what is r, what is the return code, they should be documented

da-liii · 2024-10-20T14:40:21Z

src/Plugins/Freetype/rubber_unicode_font.cpp

+  int var  = 0;
+  int n    = N (s);
+  int start= search_forwards ("-", 0, s);
+  int end  = search_backwards ("-", n, s);


tokenize provided in analyze.hpp is a better choice to impl this routine

Here is the function definition of tokenize:

array<string> tokenize (string s, string sep);

da-liii · 2024-10-20T14:42:07Z

src/Plugins/Freetype/rubber_unicode_font.cpp

+  }
+  else if (starts (s, "<wide-")) {
+    ver= false;
+  }


TODO might be added here for <left-, <right-, <mid-

KeShih · 2024-10-21T07:30:52Z

Is this issue #1682 fixed by this pull request?

This PR does not fix this issue.

da-liii

LGTM

KeShih force-pushed the glyph-variant branch 2 times, most recently from 2cbef38 to ae96369 Compare October 15, 2024 07:59

Use opentype glyph variant table

3749abe

KeShih force-pushed the glyph-variant branch from c4222fb to 3749abe Compare October 20, 2024 12:33

update

35ee7e1

KeShih marked this pull request as ready for review October 20, 2024 12:44

da-liii reviewed Oct 20, 2024

View reviewed changes

src/Plugins/Freetype/rubber_unicode_font.cpp Show resolved Hide resolved

da-liii reviewed Oct 20, 2024

View reviewed changes

update

4e61ff9

KeShih changed the title ~~[80_7] Using OpenType glyph variants~~ [80_7] Using OpenType glyph variants for rubber character Oct 21, 2024

update

d480386

da-liii approved these changes Oct 21, 2024

View reviewed changes

KeShih merged commit e2d5c20 into XmacsLabs:branch-1.2 Oct 21, 2024
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[80_7] Using OpenType glyph variants for rubber character #2119

[80_7] Using OpenType glyph variants for rubber character #2119

KeShih commented Sep 30, 2024 •

edited

Loading

da-liii Oct 20, 2024 •

edited

Loading

da-liii Oct 20, 2024

da-liii Oct 20, 2024

da-liii Oct 20, 2024

da-liii Oct 20, 2024

da-liii Oct 20, 2024

da-liii Oct 20, 2024

da-liii commented Oct 20, 2024

da-liii commented Oct 20, 2024

da-liii commented Oct 20, 2024 •

edited

Loading

da-liii Oct 20, 2024

da-liii Oct 20, 2024

da-liii Oct 20, 2024

da-liii Oct 20, 2024

da-liii Oct 20, 2024

da-liii Oct 20, 2024

KeShih commented Oct 21, 2024

da-liii left a comment


		unsigned int glyphID= ft_get_char_index (math_face->ft_face, u);

		cout << "search_font_sub_opentype for " << u << " -> " << glyphID << LF;

[80_7] Using OpenType glyph variants for rubber character #2119

[80_7] Using OpenType glyph variants for rubber character #2119

Conversation

KeShih commented Sep 30, 2024 • edited Loading

What?

Why?

How to test it?

da-liii Oct 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

da-liii commented Oct 20, 2024

da-liii commented Oct 20, 2024

da-liii commented Oct 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KeShih commented Oct 21, 2024

da-liii left a comment

Choose a reason for hiding this comment

KeShih commented Sep 30, 2024 •

edited

Loading

da-liii Oct 20, 2024 •

edited

Loading

da-liii commented Oct 20, 2024 •

edited

Loading