Fix #130463, #114592: VSE prefetching uses wrong font for text strips

Prefetching happens on a background thread by design, and so it was not loading
any fonts and using the default monospace font.

Address this by making all font usage within VSE use "unique" BLF font objects,
and protecting concurrent access to them or their state within VSE itself:

- SeqFontMap structure to hold a mutex, file path -> font ID map (for file
  based fonts), name -> font ID map (for datablock fonts).
- seq_load_font_file, seq_load_font_mem, seq_unload_font that use the above
  instead of calling into BLF directly.
- Text effect rendering part guards with a mutex, so that font render state and
  glyph cache does not get trashed.
- SeqFontMap is global instead of some Scene-specific runtime data (e.g.
  scene->ed), because right now there's a hard max limit of total number of
  fonts (64), and if each copy of the VSE data would start to  load their own
  unique fonts, that limit could get easily reached.
- BLF font loading/unloading code is now thread safe. The rest of BLF drawing
  is still not!

If at some point in the future BLF font usage becomes thread safe, and font
drawing becomes "stateless", this whole machinery can be removed.

Pull Request: https://projects.blender.org/blender/blender/pulls/130542

source/blender/blenfont/BLF_api.hh

@@ -55,6 +55,10 @@ void BLF_cache_flush_set_fn(void (*cache_flush_fn)());
 
 /**
  * Loads a font, or returns an already loaded font and increments its reference count.
+ *
+ * Note that while loading fonts is thread-safe, most of font usage via BLF
+ * state modification functions is not. If you need to use fonts from multiple threads,
+ * use unique font instances for threaded parts, see #BLF_load_unique.
  */
 int BLF_load(const char *filepath) ATTR_WARN_UNUSED_RESULT ATTR_NONNULL(1);
 int BLF_load_mem(const char *name, const unsigned char *mem, int mem_size) ATTR_WARN_UNUSED_RESULT
@@ -62,19 +66,40 @@ int BLF_load_mem(const char *name, const unsigned char *mem, int mem_size) ATTR_
 
 bool BLF_is_loaded(const char *filepath) ATTR_WARN_UNUSED_RESULT ATTR_NONNULL(1);
 bool BLF_is_loaded_mem(const char *name) ATTR_WARN_UNUSED_RESULT ATTR_NONNULL(1);
+bool BLF_is_loaded_id(int fontid) ATTR_WARN_UNUSED_RESULT;
 
+/**
+ * Loads a font into a new font object.
+ *
+ * Unlike #BLF_load, it does not look whether a font with the same
+ * path or name is already loaded. Primary use case is when using BLF
+ * functions from a non-main thread.
+ */
 int BLF_load_unique(const char *filepath) ATTR_WARN_UNUSED_RESULT ATTR_NONNULL(1);
 int BLF_load_mem_unique(const char *name, const unsigned char *mem, int mem_size)
     ATTR_NONNULL(1, 2);
 
+/**
+ * Decreases font reference count, if it reaches zero the font is unloaded.
+ */
 void BLF_unload(const char *filepath) ATTR_NONNULL(1);
 #if 0 /* Not needed at the moment. */
 void BLF_unload_mem(const char *name) ATTR_NONNULL(1);
 #endif
 
-void BLF_unload_id(int fontid);
+/**
+ * Decreases font reference count, if it reaches zero the font is unloaded.
+ * Returns true if font got unloaded.
+ */
+bool BLF_unload_id(int fontid);
+
 void BLF_unload_all();
 
+/**
+ * Increases font reference count.
+ */
+void BLF_addref_id(int fontid);
+
 char *BLF_display_name_from_file(const char *filepath) ATTR_WARN_UNUSED_RESULT ATTR_NONNULL(1);
 
 char *BLF_display_name_from_id(int fontid);

source/blender/blenfont/intern/blf.cc

@@ -1,4 +1,4 @@
-/* SPDX-FileCopyrightText: 2009 Blender Authors
+/* SPDX-FileCopyrightText: 2009-2024 Blender Authors
  *
  * SPDX-License-Identifier: GPL-2.0-or-later */
 
@@ -14,6 +14,7 @@
 #include <cstdio>
 #include <cstdlib>
 #include <cstring>
+#include <mutex>
 
 #include <ft2build.h>
 
@@ -26,7 +27,6 @@
 #include "BLI_math_rotation.h"
 #include "BLI_path_utils.hh"
 #include "BLI_string.h"
-#include "BLI_threads.h"
 
 #include "BLF_api.hh"
 
@@ -51,6 +51,8 @@ FontBLF *global_font[BLF_MAX_FONT] = {nullptr};
 int blf_mono_font = -1;
 int blf_mono_font_render = -1;
 
+static std::mutex g_blf_load_mutex;
+
 static FontBLF *blf_get(int fontid)
 {
   if (fontid >= 0 && fontid < BLF_MAX_FONT) {
@@ -106,13 +108,9 @@ void BLF_cache_clear()
   }
 }
 
-bool blf_font_id_is_valid(int fontid)
-{
-  return blf_get(fontid) != nullptr;
-}
-
 static int blf_search_by_mem_name(const char *mem_name)
 {
+  std::lock_guard lock(g_blf_load_mutex);
   for (int i = 0; i < BLF_MAX_FONT; i++) {
     const FontBLF *font = global_font[i];
     if ((font == nullptr) || (font->mem_name == nullptr)) {
@@ -128,6 +126,7 @@ static int blf_search_by_mem_name(const char *mem_name)
 
 static int blf_search_by_filepath(const char *filepath)
 {
+  std::lock_guard lock(g_blf_load_mutex);
   for (int i = 0; i < BLF_MAX_FONT; i++) {
     const FontBLF *font = global_font[i];
     if ((font == nullptr) || (font->filepath == nullptr)) {
@@ -171,6 +170,11 @@ bool BLF_is_loaded_mem(const char *name)
   return blf_search_by_mem_name(name) >= 0;
 }
 
+bool BLF_is_loaded_id(int fontid)
+{
+  return blf_get(fontid) != nullptr;
+}
+
 int BLF_load(const char *filepath)
 {
   /* check if we already load this font. */
@@ -186,9 +190,7 @@ int BLF_load(const char *filepath)
 
 int BLF_load_unique(const char *filepath)
 {
-  /* Don't search in the cache!! make a new
-   * object font, this is for keep fonts threads safe.
-   */
+  std::lock_guard lock(g_blf_load_mutex);
   int i = blf_search_available();
   if (i == -1) {
     printf("Too many fonts!!!\n");
@@ -227,7 +229,6 @@ int BLF_load_mem(const char *name, const uchar *mem, int mem_size)
 {
   int i = blf_search_by_mem_name(name);
   if (i >= 0) {
-    // font = global_font[i]; /* UNUSED */
     return i;
   }
   return BLF_load_mem_unique(name, mem, mem_size);
@@ -235,10 +236,7 @@ int BLF_load_mem(const char *name, const uchar *mem, int mem_size)
 
 int BLF_load_mem_unique(const char *name, const uchar *mem, int mem_size)
 {
-  /*
-   * Don't search in the cache, make a new object font!
-   * this is to keep the font thread safe.
-   */
+  std::lock_guard lock(g_blf_load_mutex);
   int i = blf_search_available();
   if (i == -1) {
     printf("Too many fonts!!!\n");
@@ -263,6 +261,7 @@ int BLF_load_mem_unique(const char *name, const uchar *mem, int mem_size)
 
 void BLF_unload(const char *filepath)
 {
+  std::lock_guard lock(g_blf_load_mutex);
   for (int i = 0; i < BLF_MAX_FONT; i++) {
     FontBLF *font = global_font[i];
     if (font == nullptr || font->filepath == nullptr) {
@@ -281,8 +280,9 @@ void BLF_unload(const char *filepath)
   }
 }
 
-void BLF_unload_id(int fontid)
+bool BLF_unload_id(int fontid)
 {
+  std::lock_guard lock(g_blf_load_mutex);
   FontBLF *font = blf_get(fontid);
   if (font) {
     BLI_assert(font->reference_count > 0);
@@ -291,8 +291,10 @@ void BLF_unload_id(int fontid)
     if (font->reference_count == 0) {
       blf_font_free(font);
       global_font[fontid] = nullptr;
+      return true;
     }
   }
+  return false;
 }
 
 void BLF_unload_all()
@@ -309,6 +311,15 @@ void BLF_unload_all()
   BLF_default_set(-1);
 }
 
+void BLF_addref_id(int fontid)
+{
+  std::lock_guard lock(g_blf_load_mutex);
+  FontBLF *font = blf_get(fontid);
+  if (font) {
+    font->reference_count++;
+  }
+}
+
 void BLF_enable(int fontid, int option)
 {
   FontBLF *font = blf_get(fontid);

source/blender/blenfont/intern/blf_default.cc

@@ -32,7 +32,7 @@ void BLF_default_size(float size)
 
 void BLF_default_set(int fontid)
 {
-  if ((fontid == -1) || blf_font_id_is_valid(fontid)) {
+  if ((fontid == -1) || BLF_is_loaded_id(fontid)) {
     global_font_default = fontid;
   }
 }

source/blender/blenfont/intern/blf_internal.hh

@@ -73,8 +73,6 @@ char *blf_dir_metrics_search(const char *filepath);
 int blf_font_init();
 void blf_font_exit();
 
-bool blf_font_id_is_valid(int fontid);
-
 /**
  * Return glyph id from char-code.
  */

source/blender/blenfont/intern/blf_internal_types.hh

@@ -8,6 +8,7 @@
 
 #pragma once
 
+#include <atomic>
 #include <mutex>
 
 #include "BLF_api.hh"
@@ -310,8 +311,8 @@ struct FontBLF {
    */
   uint unicode_ranges[4];
 
-  /** Number of times this font was loaded. */
-  unsigned int reference_count;
+  /** Number of references to this font object. When it reaches zero, font is unloaded. */
+  std::atomic<uint32_t> reference_count;
 
   /** Aspect ratio or scale. */
   float aspect[3];

source/blender/blenkernel/intern/blender.cc

@@ -44,6 +44,8 @@
 
 #include "BLF_api.hh"
 
+#include "SEQ_utils.hh"
+
 Global G;
 UserDef U;
 
@@ -76,6 +78,7 @@ void BKE_blender_free()
   BKE_callback_global_finalize();
 
   IMB_moviecache_destruct();
+  SEQ_fontmap_clear();
 #ifdef WITH_FFMPEG
   BKE_ffmpeg_exit();
 #endif

source/blender/sequencer/SEQ_utils.hh

@@ -56,6 +56,8 @@ void SEQ_set_scale_to_fit(const Sequence *seq,
  */
 void SEQ_ensure_unique_name(Sequence *seq, Scene *scene);
 
+void SEQ_fontmap_clear();
+
 namespace blender::seq {
 
 /**

source/blender/sequencer/intern/effects.cc

@@ -11,12 +11,14 @@
 #include <cmath>
 #include <cstdlib>
 #include <cstring>
+#include <mutex>
 
 #include "BLI_vector.hh"
 #include "DNA_vec_types.h"
 #include "MEM_guardedalloc.h"
 
 #include "BLI_array.hh"
+#include "BLI_map.hh"
 #include "BLI_math_rotation.h"
 #include "BLI_math_vector.hh"
 #include "BLI_math_vector_types.hh"
@@ -25,7 +27,6 @@
 #include "BLI_string.h"
 #include "BLI_string_utf8.h"
 #include "BLI_task.hh"
-#include "BLI_threads.h"
 #include "BLI_utildefines.h"
 
 #include "DNA_packedFile_types.h"
@@ -67,6 +68,110 @@ using namespace blender;
 
 static SeqEffectHandle get_sequence_effect_impl(int seq_type);
 
+/* -------------------------------------------------------------------- */
+/* Sequencer font access.
+ *
+ * Text strips can access and use fonts from a background thread
+ * (when depsgraph evaluation copies the scene, or when prefetch renders
+ * frames with text strips in a background thread).
+ *
+ * To not interfere with what might be happening on the main thread, all
+ * fonts used by the sequencer are made unique via #BLF_load_unique
+ * #BLF_load_mem_unique, and there's a mutex to guard against
+ * sequencer itself possibly using the fonts from several threads.
+ */
+
+struct SeqFontMap {
+  /* File path -> font ID mapping for file-based fonts. */
+  Map<std::string, int> path_to_file_font_id;
+  /* Datablock name -> font ID mapping for memory (datablock) fonts. */
+  Map<std::string, int> name_to_mem_font_id;
+
+  /* Font access mutex. Recursive since it is locked from
+   * text strip rendering, which can call into loading from within. */
+  std::recursive_mutex mutex;
+};
+
+static SeqFontMap g_font_map;
+
+void SEQ_fontmap_clear()
+{
+  for (const auto &item : g_font_map.path_to_file_font_id.items()) {
+    BLF_unload_id(item.value);
+  }
+  g_font_map.path_to_file_font_id.clear_and_shrink();
+  for (const auto &item : g_font_map.name_to_mem_font_id.items()) {
+    BLF_unload_id(item.value);
+  }
+  g_font_map.name_to_mem_font_id.clear_and_shrink();
+}
+
+static int seq_load_font_file(const std::string &path)
+{
+  std::lock_guard lock(g_font_map.mutex);
+  int fontid = g_font_map.path_to_file_font_id.add_or_modify(
+      path,
+      [&](int *fontid) {
+        /* New path: load font. */
+        *fontid = BLF_load_unique(path.c_str());
+        return *fontid;
+      },
+      [&](int *fontid) {
+        /* Path already in cache: add reference to already loaded font,
+         * or load a new one in case that
+         * font id was unloaded behind our backs. */
+        if (*fontid >= 0) {
+          if (BLF_is_loaded_id(*fontid)) {
+            BLF_addref_id(*fontid);
+          }
+          else {
+            *fontid = BLF_load_unique(path.c_str());
+          }
+        }
+        return *fontid;
+      });
+  return fontid;
+}
+
+static int seq_load_font_mem(const std::string &name, const unsigned char *data, int data_size)
+{
+  std::lock_guard lock(g_font_map.mutex);
+  int fontid = g_font_map.name_to_mem_font_id.add_or_modify(
+      name,
+      [&](int *fontid) {
+        /* New name: load font. */
+        *fontid = BLF_load_mem_unique(name.c_str(), data, data_size);
+        return *fontid;
+      },
+      [&](int *fontid) {
+        /* Name already in cache: add reference to already loaded font,
+         * or (if we're on the main thread) load a new one in case that
+         * font id was unloaded behind our backs. */
+        if (*fontid >= 0) {
+          if (BLF_is_loaded_id(*fontid)) {
+            BLF_addref_id(*fontid);
+          }
+          else {
+            *fontid = BLF_load_mem_unique(name.c_str(), data, data_size);
+          }
+        }
+        return *fontid;
+      });
+  return fontid;
+}
+
+static void seq_unload_font(int fontid)
+{
+  std::lock_guard lock(g_font_map.mutex);
+  bool unloaded = BLF_unload_id(fontid);
+  /* If that was the last usage of the font and it got unloaded: remove
+   * it from our maps. */
+  if (unloaded) {
+    g_font_map.path_to_file_font_id.remove_if([&](auto item) { return item.value == fontid; });
+    g_font_map.name_to_mem_font_id.remove_if([&](auto item) { return item.value == fontid; });
+  }
+}
+
 /* -------------------------------------------------------------------- */
 /** \name Internal Utilities
  * \{ */
@@ -2555,9 +2660,10 @@ void SEQ_effect_text_font_unload(TextVars *data, const bool do_id_user)
     data->text_font = nullptr;
   }
 
-  /* Unload the BLF font. */
+  /* Unload the font. */
   if (data->text_blf_id >= 0) {
-    BLF_unload_id(data->text_blf_id);
+    seq_unload_font(data->text_blf_id);
+    data->text_blf_id = -1;
   }
 }
 
@@ -2583,25 +2689,14 @@ void SEQ_effect_text_font_load(TextVars *data, const bool do_id_user)
     char name[MAX_ID_FULL_NAME];
     BKE_id_full_name_get(name, &vfont->id, 0);
 
-    data->text_blf_id = BLF_load_mem(name, static_cast<const uchar *>(pf->data), pf->size);
+    data->text_blf_id = seq_load_font_mem(name, static_cast<const uchar *>(pf->data), pf->size);
   }
   else {
     char filepath[FILE_MAX];
     STRNCPY(filepath, vfont->filepath);
-    if (BLI_thread_is_main()) {
-      /* FIXME: This is a band-aid fix.
-       * A proper solution has to be worked on by the sequencer team.
-       *
-       * This code can be called from non-main thread, e.g. when copying sequences as part of
-       * depsgraph evaluated copy of the evaluated scene. Just skip font loading in that case, BLF
-       * code is not thread-safe, and if this happens from threaded context, it almost certainly
-       * means that a previous attempt to load the font already failed, e.g. because font file-path
-       * is invalid. Proposer fix would likely be to not attempt to reload a failed-to-load font
-       * every time. */
-      BLI_path_abs(filepath, ID_BLEND_PATH_FROM_GLOBAL(&vfont->id));
 
-      data->text_blf_id = BLF_load(filepath);
-    }
+    BLI_path_abs(filepath, ID_BLEND_PATH_FROM_GLOBAL(&vfont->id));
+    data->text_blf_id = seq_load_font_file(filepath);
   }
 }
 
@@ -3132,6 +3227,11 @@ static int text_effect_font_init(const SeqRenderData *context, const Sequence *s
   TextVars *data = static_cast<TextVars *>(seq->effectdata);
   int font = blf_mono_font_render;
 
+  /* In case font got unloaded behind our backs: mark it as needing a load. */
+  if (data->text_blf_id >= 0 && !BLF_is_loaded_id(data->text_blf_id)) {
+    data->text_blf_id = SEQ_FONT_NOT_LOADED;
+  }
+
   if (data->text_blf_id == SEQ_FONT_NOT_LOADED) {
     data->text_blf_id = -1;
 
@@ -3349,6 +3449,9 @@ static ImBuf *do_text_effect(const SeqRenderData *context,
   const int font_flags = ((data->flag & SEQ_TEXT_BOLD) ? BLF_BOLD : 0) |
                          ((data->flag & SEQ_TEXT_ITALIC) ? BLF_ITALIC : 0);
 
+  /* Guard against parallel accesses to the fonts map. */
+  std::lock_guard lock(g_font_map.mutex);
+
   const int font = text_effect_font_init(context, seq, font_flags);
 
   TextVarsRuntime runtime;