Support Harfbuzz in Metrics View by iorsh · Pull Request #5522 · fontforge/fontforge

iorsh · 2025-01-09T21:18:25Z

This PR introduces HarfBuzz shaping in Metrics view.

The current shaping functionality is preserved and can be invoked from View menu as "Built-in shaper". It's very complex, and I'm pretty sure I didn't manage to fully support everything it provides, so some people might find it better in some aspects.
The new shaper comes with some generic infrastructure which can be eventually expanded to support more shapers (e.g. Uniscribe under Windows)
The new code is written mostly in C++17. This version seems reasonably conservative to me.
Editing of kerning values, bearings, and width is supported.
The font is generated for HarfBuzz use every time the Metrics View is opened or shaper engine is switched (in the View menu). This could be slow for large fonts.

After:

Before:

Menu:

Fixes #3733, fixes #3837, fixes #4158, fixes #4511, fixes #5343, fixes #5513

…haper

…++ framework

khaledhosny · 2025-02-28T21:28:12Z

    context->get_enc_map = SFGetMap;
    context->get_char_width = MVCharWidth;
    context->get_kern_offset = MVGetKernOffset;
+    context->script_is_rtl = ScriptIsRightToLeft;


HarfBuzz has hb_script_get_horizontal_direction() which might be more up to date (hb_buffer_guess_segment_properties() will call it if the direction is not set or set to HB_DIRECTION_INVALID). But I still think a separate UI control to set direction is needed since a few scripts can be written horizontally in both directions.

ScriptIsRightToLeft() is passed for use in the legacy shaper, HarfBuzz won't use it. Regarding the UI, I have some enhancements in mind, but for now I prefer to keep UI changes at the necessary minimum. GDraw poses quite a few challenges in its workings.

khaledhosny · 2025-03-05T22:38:30Z

+        auto ttf_map_it = ttf_map_.find(glyph_info.codepoint);
+        SplineChar* glyph_out = (ttf_map_it != ttf_map_.end())
+                                    ? ttf_map_it->second
+                                    : get_notdef_glyph();


There is hb_buffer_set_not_found_glyph() that allow setting any arbitrary glyph index to be used instead of 0, you can, say, set it to a special value that is guaranteed not to be a valid glyph index like INT32_MAX and intercept it here. In case it makes things any simpler/faster.

Yes, I tried it, but Ubuntu 22 has HarfBuzz 2.7.4, and hb_buffer_set_not_found_glyph() was introduced in 3.1.0. Besides, I need to check the glyph index anyway, it would be a crash otherwise.

iorsh · 2025-03-07T22:37:48Z

The high-level HarfBuzz shaper structure is as follows:

When opening the "Metrics View" window, the shaper is initialized with TTF file generated on-fly.
While the window is active, it is responsive to the changes in glyph kerning using HarfBuzzShaper::initial_kerning_. The initial kerning is preserved, and once its value changes, the delta is applied internally over the metrics computed by HarfBuzz.
In a similar manner changes to glyph width are also addressed with HarfBuzzShaper::initial_width_, without the need to regenerate the TTF.
In case of more complex changes, such as in GSUB/GPOS tables, the user is expected to perform manual refresh, and I added "Refresh Shaper" menu item for that purpose. During the refresh the TTF file is fully regenerated.
In total, the shaper is regenerated whenever
- A new "Metrics View" is opened
- The user hits "Refresh Shaper"
- The uses changes the shaper ("Built-in" to "HarfBuzz" or vice versa)

The building of HarfBuzz shaper is heavy, and eventually we shall make an effort to improve its performance. Currently the TTF is written to a temporary file, and as far as I can see, it's not even guaranteed to reside in RAM. In Windows it's always on the disk (see GFileTmpfile()), and in Linux it probably depends on the mount method of /tmp.

For now I would prefer to keep the scope of this PR as is, but I can see several possibilities to improve the performance in future:

Refactoring of _WriteTTFFont(), so that it actually uses a generic buffer which can be either kept in memory or dumped onto disk.
To employ the granular approach with hb_face_create_for_tables(), it would be necessary to refactor the change tracking. Currently there is a bunch of changed_??? flags scattered across multiple structures, and it would be good to harmonize them with some unified approach.
Inherent FF deficiencies, e.g. the names hashtable struct glyphnamehash with 257 baskets, which is totally inadequate for fonts with thousands of glyphs.

skef · 2025-04-22T04:28:27Z

I'll take another look at this review-wise, but I suggest that @iorsh should start thinking about documentation.

iorsh · 2025-06-23T15:27:07Z

    int16_t cmax, clen;
    SplineChar **chars;		/* Character input stream */
    struct opentype_str *glyphs;/* after going through the various gsub/gpos transformations */
+    ShapeMetrics *metrics;      /* One for each glyph above */


Comment on difference between ShapeMetrics (metrics_core) and metricchar (metrics_ui)

iorsh · 2025-06-23T15:40:52Z

+    mv->glyphs = shaper_apply_features(mv->shaper, mv->chars, feats,
+        				script, lang, mv->pixelsize, mv->vertical);
+    if (mv->glyphs == NULL) {
+	mv->glyphs = calloc(1, sizeof(struct opentype_str));


Review this

By convention, output of ApplyTickedFeatures() is terminated by a zeroized structure and is never NULL (see source). As we move to shaper interface, this convention should hold, but we put this allocation here out of caution.

iorsh · 2025-06-23T15:44:03Z

    }
+
+    // TODO: Move metrics calculations completely into shapers, and make MetricsView::metric const.
+    mv->metrics = (ShapeMetrics*)shaper_metrics(mv->shaper);


Return this from shaper_apply_features()

iorsh · 2025-06-23T16:22:52Z

+    return shaper_defs;
+}
+
+const char* get_default_shaper() {


Add user preference for default shaper.
Retain current shaper per session, which would be initialized from preference, but would not affect it.

iorsh · 2025-06-23T16:24:41Z

+    }
+}
+
+std::set<Tag> BuiltInShaper::default_features(Tag script, Tag /*lang*/,


Why this was not deleted in legacy code? If still in use, put the reference in comment

Duplicates StdFeaturesOfScript(), which is still in use

iorsh · 2025-06-23T16:38:37Z

+        // features list, but if it is unselected it should be in the features
+        // list with value set to 0 (disable).
+        const std::set<Tag> default_feats =
+            default_features(script, lang, vertical);


Move outside the cycle

iorsh · 2025-06-23T16:47:33Z

+    }
+
+    // No special glyph found, create it.
+    notdef_glyph_ = context_->get_or_make_char(context_->sf, -1, ".notdef");


What happens in legacy shaper?

In the legacy code we act similarly, calling
MVTextChanged()
WordlistEscapedInputStringToParsedDataComplex()
SFGetOrMakeChar()

frank-trampe

This all makes sense to me.

brigazvi · 2025-07-02T20:58:14Z

THIS IS HUGE!!!!
@iorsh thank you so much for all the hard work you put into this. for RTL designers like me it's makes such a difference.

in my opinion it justifies a new release.

iorsh added 30 commits November 8, 2024 12:43

Add HarfBuzz to CMake with PkgConfig

4883503

Make HarfBuzz dependency optional

0289358

shapers subdir; sample HB library call

8a06d1d

clang-format

6709139

Add struct ShaperDef, func get_shaper_defs()

444ce0c

Add get_default_shaper()

1d3c499

Add menu items for shapers

7b687cb

IShaper interface with dummy implementations HarfBuzzShaper, BuiltInS…

25fe717

…haper

C functions for shaper objects

fbbde28

Add and activate dummy shapers in Metrics view

3cd8b9d

Add class Tag

90c5aec

Add C struct ShaperContext

76b37d0

Refresh in MVSetShaper()

3db1b81

Legacy call ApplyTickedFeatures() from shaper_apply_features() with C…

0b2ffb7

…++ framework

Extract splinechar.h

50a8fed

Build font and invoke HarfBuzz shaper

b472af1

Extract ShapeMetrics from struct metricchar

f71578e

Extract ShapeMetrics to metrics.h

737c477

Manage MetricsView::metrics buffer memory inside shaper

824f189

Extract MVCharWidth()

bd3f99f

Move legacy metrics scaling code into BuiltInShaper::scale_metrics()

c09b7c8

Fix shadowed var

6ec2540

Calculate metrics from HarfBuzz output

f796a07

Fix crash

f360469

Fix TTF generation for HarfBuzz

2ad2169

Support RTL shaping in HarfBuzz

00f493c

Extract HarfBuzzShaper::extract_shaped_data()

2b7bb9d

Extract HarfBuzzShaper::reverse_rtl_metrics()

42a668e

Fix vertical metrics in RTL shaping

f663eab

Allow ApplyTickedFeatures() without GSUB

aa6b4db

khaledhosny reviewed Feb 28, 2025

View reviewed changes

iorsh added 4 commits March 2, 2025 00:40

Valgrind issues

020290c

Fix HarfBuzz crash on missing characters

183a2a2

Add "Refresh Shaper" button

fd68bdf

Incomplete C type cpp_IShaper

c073f2c

khaledhosny reviewed Mar 5, 2025

View reviewed changes

Show "waiting" cursor while loading shaper

2750ec7

Merge branch 'master' into harfbuzz_metrics

69c2241

iorsh mentioned this pull request Mar 31, 2025

Performance fixes for GSUB/GPOS dumps #5547

Merged

iorsh added 3 commits April 28, 2025 21:01

Documentation

007b839

Move .clang-format

f84a028

Format new files

d62a528

iorsh commented Jun 23, 2025

View reviewed changes

iorsh added 7 commits June 23, 2025 22:35

Rename structs

4a37aae

Minor fix

0542476

More comments

cd61e99

Fix comment

d765362

Remove redundant MVRefreshMetric() call

bf58b1d

Return kerning data and metrics from shaper_apply_features()

c8eb0f9

Add UI-based preference for default shaper

ccffe36

frank-trampe approved these changes Jun 29, 2025

View reviewed changes

Merge branch 'master' into harfbuzz_metrics

16accf2

iorsh merged commit 387146a into fontforge:master Jul 1, 2025
11 of 13 checks passed

iorsh mentioned this pull request Jul 9, 2025

RTL kerns don't show right in metric window #5581

Open

iorsh mentioned this pull request Nov 21, 2025

It's annoying that the MetricsView creates glyphs if you type one that doesn't exist, and it shouldn't do that #4655

Closed

This was referenced Apr 4, 2026

Metrics feature 'rand' only shows first #5001

Closed

Metrics window ignores default language system in OpenType features #214

Closed

Conversation

iorsh commented Jan 9, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iorsh Feb 28, 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!

iorsh commented Mar 7, 2025

Uh oh!

skef commented Apr 22, 2025

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!

frank-trampe left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

brigazvi commented Jul 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

iorsh Feb 28, 2025 •

edited

Loading