Details

Reviewers

Commits

rBc4b73847d31a: BLF: Remove Thread Locking For Font Metrics

Summary

This patch removes the need to lock the thread just to get to some
generic (not glyph-specific) font metrics.

For performance reasons we keep font glyphs in per-size caches in each font. For thread safety we bracket each access to GlyphCacheBLF with calls to blf_glyph_cache_acquire and blf_glyph_cache_release which set and release a spinlock.

However we do this unnecessarily when we just need font metrics that are not glyph specific. In these cases we now lock, find the cache that matches the current size, get a value we have previously saved, unlock.

But for the font metrics in this patch these values are exactly the same as can be found in the font->face directly as these are the current font metrics. There isn't a need to look in the cache and therefore no need to lock it. So for the following this is a matter of removing the saved value from GlyphCacheBLF and moving the code directly to where it is required:

blf_font_height_max
blf_font_width_max
blf_font_descender
blf_font_ascender

For blf_font_fixed_width we seem to call that a lot, so for this I added a member to FontBLF to cache this called fixed_width which is set only on blf_font_size. This is set a bit better than current code in that it gets the character advance without ever rendering it. And it uses the width of "0" rather than space, which is preferred behavior (CSS 'ch' width), falling back to 1/2 of an em if the font does not contain '0' (also CSS 'ch' recommendation). Of course this width is identical to current width for all monospaced fonts, but could come in handy if we ever want to mix in glyphs from proportional fonts (fallback).

Note that this patch also removes any use of blf_glyph_search outside of blf_glyph.c so this makes it static.

Diff Detail

Repository: rB Blender

Event Timeline

Harley Acheson (harley) requested review of this revision.Oct 23 2021, 9:32 PM

Harley Acheson (harley) created this revision.

Updating to current state of master. Also making blf_glyph_search static and cleaning it up a bit.

Harley Acheson (harley) edited the summary of this revision. (Show Details)Oct 31 2021, 5:18 AM

Removing the unrelated changes to blf_glyph.c as I can see enough potential cleanup there to warrant a separate patch.

Suggest BLF: prefix for subject instead of UI:

Otherwise only minor change requested.

source/blender/blenfont/intern/blf_font.c

1369–1380

Getting picky cast warnings here, this resolves:

/* Set fixed-width size for monospaced output. */
FT_UInt gindex = FT_Get_Char_Index(font->face, U'0');
if (gindex) {
  FT_Fixed advance = 0;
  FT_Get_Advance(font->face, gindex, FT_LOAD_NO_HINTING, &advance);
  /* Use CSS 'ch unit' width, advance of zero character. */
  font->fixed_width = (int)(advance >> 16);
}
else {
  /* Font does not contain "0" so use CSS fallback of 1/2 of em. */
  font->fixed_width = (int)((font->face->size->metrics.height / 2) >> 6);
}
if (font->fixed_width < 1) {
  font->fixed_width = 1;
}

This revision is now accepted and ready to land.Nov 2 2021, 6:18 AM

kursad k (kursadk) awarded a token.Nov 2 2021, 6:56 AM