Page MenuHome

BLF: Cleanup blf_glyph_cache_find & blf_font_size
ClosedPublic

Authored by Harley Acheson (harley) on Nov 25 2021, 7:19 PM.

Details

Summary

Removes unnecessary calls to blf_glyph_cache_find, simplifies
blf_font_size, and reduces calls to it.


A fairly simple refactor that probably needs some detailed explanation.

All uses of our glyph caches are bracketed by calls to blf_glyph_cache_acquire and blf_glyph_cache_release, which set and clear a spinlock and also ensures that a cache of the needed attributes exists. This check is done with a call to blf_glyph_cache_find

However there are two places where blf_glyph_cache_find is called directly without good reason. Fixing this results in some nice cleanup.

BLF_thumb_preview calls this function just to ensure that a prior call to blf_font_size was successful, This seems like an odd way to check for this. Instead this patch changes blf_font_size so that it returns true if it worked as advertised. It is therefore simplified by the removal of the cache check.

The other problematic use of blf_glyph_cache_find is inside blf_font_size itself. This function first calls blf_glyph_cache_acquire for the spin lock, but that also does an unneeded check that we have a glyph cache for the old size. Then we check to see if a cache exists for the new size and then we only change the size if that exists and the size is actually different.

But there is no need for any creation or manipulation of glyph caches here, since it is actual uses of the font that ensures it exists. The real reason we have the aquire and release here at all is because we used to preload the cache. But now we only cache glyphs as they are used.

By removing these unneeded locks, checks, and unlocks, this function gets much simpler, doing only what it needs to do. And gets even smaller with D13226: BLF: Save fixed_width value in cache.

With these functions no longer calling blf_glyph_cache_find it can be made static as it is called only in blf_glyph_cache_acquire. This removes the potential mistake of using find when acquire should be called instead with proper locking.

Keeping in mind that setting font size no longer preloads the glyph caches, this patch also removes unneeded calls to it. interface_style.c calls it a number of times for the only purpose of cache preloading.

Diff Detail

Repository
rB Blender

Event Timeline

Harley Acheson (harley) requested review of this revision.Nov 25 2021, 7:19 PM
Harley Acheson (harley) created this revision.

Updated to current state of master.

Updated to current state of master.

Harley Acheson (harley) edited the summary of this revision. (Show Details)

Updated to the current state of master.

Campbell Barton (campbellbarton) accepted this revision.EditedFeb 4 2022, 2:56 AM

Nice cleanup, minor changes requested inline (accepting, no need for additional review).

source/blender/blenfont/intern/blf_internal.h
59

Doc-string should be added noting what the return value represents.

143

This should be marked static (getting -Wmissing-prototypes warning).

This revision is now accepted and ready to land.Feb 4 2022, 2:56 AM

Updating to current state of master and to incorporate changes requested in review.