Performance problems with hb_ft_* functions

[tiffany352]

I'm currently implementing HarfBuzz-based shaping and I'm very close to finishing a shippable version, but there's a few performance issues I'd like to address before doing so. I have HarfBuzz built with only the OpenType shaper, and only the UCDN Unicode functions. It is used alongside Freetype. As such, I am using the Freetype face/font functions.

Originally, I was using hb_ft_font_create, but I found that this was very expensive because of it creating and then destroying an hb_face_t every time it's called. It doesn't appear to cache it at all. This was a big problem because it also meant that shape plans would not be cached, they would be discarded with the hb_font_t reference.

Now, I am using the following code:

FT_Face face;
hb_face_t* hbFace = hb_ft_face_create(face, NULL);
...
hb_font_t* hb_font = hb_font_create(hbFace);
// this line is lifted directly from the HarfBuzz source for hb_ft_font_create
hb_font_set_scale(hb_font,
    (int)(((uint64_t)face->size->metrics.x_scale * (uint64_t)face->units_per_EM + (1u << 15)) >> 16),
    (int)(((uint64_t)face->size->metrics.y_scale * (uint64_t)face->units_per_EM + (1u << 15)) >> 16));
hb_ft_font_set_funcs(hb_font);

Which is about 10 times faster (because I store the hbFace, resulting in shape plans being cached), and I haven't noticed any incorrect behavior. (I could be doing it wrong, though.) However, hb_ft_font_set_funcs still calls FT_New_Memory_Face seemingly unnecessarily, which is now 12% of the CPU usage of the entire application (actually calling hb_shape is only 6%).

I believe I could make this faster still by doing the same thing that hb_ft_font_set_funcs does, but without creating a new FT_Face (using my existing handle). However, this function relies on static functions in HarfBuzz like _hb_ft_font_set_funcs. I'm wondering whether I should add a new function that does the same thing as hb_ft_font_set_funcs but takes an FT_Face reference instead of creating a new one.

[behdad]

Thanks for the detailed report. I'll get to it soonish hopefully.

[roozbehp]

/cc @nona-google

[tiffany352]

I added a new function hb_ft_font_set_funcs_with_ft_face(hb_font_t*, FT_Face) which uses the passed in face instead of creating a new one. It assumes that the FT_Face will outlive the hb_font_t, which is easy for me to guarantee caller side. The performance is now much better - the HarfBuzz implementation is only marginally slower than the original text rendering pipeline.

I just looked into why Chrome doesn't have performance issues like this. Apparently it's because it doesn't use these hb_ft_* functions at all - Skia implements its own hb_font_funcs_t. I (so far at least) have no reasons why I would do that other than to resolve the performance issues I had with hb_ft_*, so it seems reasonable to me to add this function. Should I make a pull request?

[jfkthame]

Maybe I'm misunderstanding your situation, but it sounds to me as though you're frequently creating and discarding hb_font_t instances; can't you just create this once, and hold on to it as long as you need to shape text with a given font? That should avoid the performance issue you describe, AFAICS.

[tiffany352]

@jfkthame The documentation says that one should use FT_Request_Size before creating an hb_font_t, so I just assumed I had to create a new one for each font size. It looks like I'm wrong, and I can just store a single hb_font_t per face, calling hb_font_set_scale before using it. Thanks for pointing that out.

[behdad]

@jfkthame The documentation says that one should use FT_Request_Size before creating an hb_font_t, so I just assumed I had to create a new one for each font size. It looks like I'm wrong, and I can just store a single hb_font_t per face, calling hb_font_set_scale before using it. Thanks for pointing that out.

Right. I'll add a hb_ft_font_changed() that one can call after size or other aspects of FT_Face changed.

[behdad]

Fixed. Please test!