(Font) Skip a font rebuild that would produce the font already loaded

The deferred free changed when a superseded font handle is released,
not how often one is created. Every layout pass still tears down and
rasterises its fonts unconditionally, and most layout passes are not
asking for a different font at all:

  ozone_set_layout   runs on thumbnail scale and padding changes, and
                     rebuilds all six fonts; each font also has its own
                     scale factor, so moving one moves all six
  materialui_layout  runs on a nav bar toggle and a landscape
                     optimisation change, rebuilding three
  gfx_widgets_layout runs on any video dimension change, but the size
                     it asks for derives from last_scale_factor alone,
                     so a resize at unchanged scale rebuilds six
  xmb_set_layout     the scale setting steps in hundredths while
                     font_size is an integer, so most steps of the
                     slider move nothing

None of that is cached today. font_driver.c's font_file_refs caches
the file bytes per path, which saves the read, but a rebuild still
rasterises the atlas and re-uploads the texture, and that is the
expensive part.

font_data_t already remembers both the size it was built at and the
path it was built from, so the question is answerable without new
state. Add font_driver_matches() and have the four layout paths return
early when it says yes.

It is conservative in one direction only. It answers no for a NULL
handle, for any size or path difference, and for a handle that is no
longer live — the list walk means a caller that freed a font and left
the pointer behind gets a no rather than a use-after-free read. So a
context reset, which frees and clears its fonts, always rebuilds. A
wrong no costs only the work that would have been done anyway; a wrong
yes would keep a font that should have been replaced, which is why the
test covers the no cases far more heavily than the yes case.

Two ordering details this depends on. The size compared is the one
that would actually be built, so the callers' own clamps (9 for ozone
and widgets) run first. And the early return precedes the approximate
glyph metrics those functions write before building, which are
overwritten by measured values afterwards — returning after them would
leave the estimate in place of a measurement.

The wide-glyph sample follows the menu language and is not refreshed
by font_driver_sync_impl(), so a change there must not be skipped past
even when the face is unchanged. XMB recomputes the derived widths
against the loaded font, which is all that is needed. Ozone and
MaterialUI compute those widths inside the build, so they take the
conservative route and fall through to a rebuild; a language change is
rare enough that the simpler code is worth more than the saving.

XMB's hand-rolled guard in xmb_set_layout() collapses into this.
Moving it into xmb_rebuild_fonts() puts it next to the path resolution
it needs, and last_font_path is now recorded before the test rather
than after: a menu font setting can change to a path that resolves to
the file already loaded, and leaving it stale would have xmb_render()
detect a change every frame and call straight back in.

Covered in samples/gfx/font_lifecycle, under ASan+UBSan+LSan and TSan:
match and every flavour of mismatch, a face re-resolved in place by a
language switch no longer matching the path it was built from, and a
freed handle answering no both after a direct free and after the
deferred queue drains.

gfx/font_driver.c

@@ -1346,6 +1346,38 @@ void font_driver_free(font_data_t *font)
    }
 }
 
+bool font_driver_matches(const font_data_t *font,
+      const char *path, float size)
+{
+   const font_data_t *live;
+
+   if (!font)
+      return false;
+
+   /* Confirm the handle is still one of ours before reading through
+    * it. A caller that freed a font without clearing its pointer
+    * would otherwise turn this into a use-after-free, and "no" is the
+    * answer it needs in that case anyway. The list holds one entry
+    * per live font, so this is a walk of a handful of pointers. */
+   for (live = font_live; live; live = live->next)
+      if (live == font)
+         break;
+   if (!live)
+      return false;
+
+   if (font->size != size)
+      return false;
+
+   /* A font whose face the renderer chose carries no path, and can
+    * only match a request that likewise names none. */
+   if (!font->path)
+      return (!path || !*path);
+   if (!path || !*path)
+      return false;
+
+   return string_is_equal(font->path, path);
+}
+
 void font_driver_free_deferred(font_data_t *font)
 {
    unsigned i;

gfx/font_driver.h

@@ -179,6 +179,22 @@ void font_driver_render_msg(void *data,
 
 int font_driver_get_message_width(void *font_data, const char *msg, size_t len, float scale);
 
+/* Would rebuilding this font at this path and size produce the font
+ * that is already there? Lets a caller skip a rebuild that would
+ * rasterise an identical atlas and re-upload an identical texture,
+ * which is most of what a layout pass asks for: the things that
+ * trigger one — padding, thumbnail scale, a nav bar toggle — mostly
+ * do not move any font size.
+ *
+ * False for a NULL handle, and for one that is no longer live, so a
+ * context reset that cleared or released its fonts always rebuilds.
+ * Conservative in both directions: it answers no whenever it cannot
+ * be sure, and a wrong no only costs the work that would have been
+ * done anyway. Callers must pass the same size they would pass to the
+ * font builder, after any clamping of their own. */
+bool font_driver_matches(const font_data_t *font,
+      const char *path, float size);
+
 void font_driver_free(font_data_t *font);
 
 /* Release a font whose replacement has already been built, once the

gfx/gfx_widgets.c

@@ -939,10 +939,26 @@ static void gfx_widgets_font_init(
 {
    float scaled_size             = font_size * p_dispwidget->last_scale_factor;
 
-   /* Limit minimum font size to keep it readable */
+   /* Limit minimum font size to keep it readable.
+    * Before the match test below, so it compares the size that would
+    * actually be built. */
    if (scaled_size < 9)
       scaled_size = 9;
 
+   /* Nothing to do if this is already the font that was asked for.
+    * gfx_widgets_layout() runs on any video dimension change, but the
+    * size here derives from last_scale_factor alone, so a resize at
+    * an unchanged scale asks for the six fonts already loaded.
+    *
+    * usage_count is still cleared: it counts draws against the font
+    * since the last layout pass, and the callers below expect a
+    * layout to have reset it whether or not a rebuild happened. */
+   if (font_driver_matches(font_data->font, font_path, scaled_size))
+   {
+      font_data->usage_count     = 0;
+      return;
+   }
+
    /* Get approximate glyph width */
    font_data->glyph_width        = scaled_size * (3.0f / 4.0f);
 

menu/drivers/materialui.c

@@ -9454,9 +9454,6 @@ static void materialui_init_font(gfx_display_t *p_disp,
    font_data_t *old_font     = font_data->font;
    fontpath[0]               = '\0';
 
-   /* We assume the average glyph aspect ratio is close to 3:4 */
-   font_data->glyph_width    = (int)((font_size * (3.0f / 4.0f)) + 0.5f);
-
    {
       const char *lang_font = font_driver_language_font_file();
 
@@ -9473,6 +9470,23 @@ static void materialui_init_font(gfx_display_t *p_disp,
          strlcpy(fontpath, default_fontpath, sizeof(fontpath));
    }
 
+   /* Nothing to do if this is already the font that was asked for.
+    * materialui_layout() also runs on a nav bar toggle and a
+    * landscape-optimisation change, neither of which moves a font
+    * size.
+    *
+    * Resolved above rather than below, so the path being compared is
+    * the one that would be built. The wide-glyph sample follows the
+    * menu language and is not refreshed by font_driver_sync_impl(),
+    * so a change there falls through even when the face is
+    * unchanged. */
+   if (     font_data->wideglyph_str == wideglyph_str
+         && font_driver_matches(font_data->font, fontpath, (float)font_size))
+      return;
+
+   /* We assume the average glyph aspect ratio is close to 3:4 */
+   font_data->glyph_width    = (int)((font_size * (3.0f / 4.0f)) + 0.5f);
+
    /* Built before the old one is released, and the old one retired
     * rather than freed: materialui_layout() reaches here from
     * materialui_render(), which runs before the video driver's frame

menu/drivers/ozone.c

@@ -10002,12 +10002,28 @@ static bool ozone_init_font(
    int glyph_width               = 0;
    gfx_display_t *p_disp         = disp_get_ptr();
    const char *wideglyph_str     = msg_hash_get_wideglyph_str();
-   font_data_t *old_font         = font_data->font;
+   font_data_t *old_font;
 
-   /* Enforce minimum readable font size for small screens */
+   /* Enforce minimum readable font size for small screens.
+    * Before the match test below, so it compares the size that would
+    * actually be built. */
    if (font_size < 9)
       font_size = 9;
 
+   /* Nothing to do if this is already the font that was asked for.
+    * ozone_set_layout() runs on padding and thumbnail-scale changes
+    * too, and each of the six fonts has its own scale factor, so most
+    * calls here are asking for a font that is already loaded.
+    *
+    * The wide-glyph sample follows the menu language and is not
+    * refreshed by font_driver_sync_impl(), so a change there has to
+    * fall through even when the face is unchanged. */
+   if (     font_data->wideglyph_str == wideglyph_str
+         && font_driver_matches(font_data->font, font_path, font_size))
+      return true;
+
+   old_font                      = font_data->font;
+
    /* Cache approximate dimensions */
    font_data->line_height        = (int)(font_size + 0.5f);
    font_data->glyph_width        = (int)((font_size * (3.0f / 4.0f)) + 0.5f);

menu/drivers/xmb.c

@@ -7756,7 +7756,7 @@ static void xmb_context_reset_textures(
  * resolution lives here rather than at the call sites so that a
  * context reset and an in-place rebuild cannot disagree on what is
  * loaded. */
-static void xmb_rebuild_fonts(xmb_handle_t *xmb, bool is_threaded,
+static bool xmb_rebuild_fonts(xmb_handle_t *xmb, bool is_threaded,
       bool defer_free)
 {
    char fontpath[PATH_MAX_LENGTH];
@@ -7790,9 +7790,21 @@ static void xmb_rebuild_fonts(xmb_handle_t *xmb, bool is_threaded,
    else
       strlcpy(fontpath, default_fontpath, sizeof(fontpath));
 
+   /* Recorded before the match test, not after it: the setting can
+    * change to a path that resolves to the file already loaded, and
+    * leaving it stale would have xmb_render() detect a change every
+    * frame and call back in here forever. */
    strlcpy(xmb->last_font_path, menu_font ? menu_font : "",
          sizeof(xmb->last_font_path));
 
+   /* Both already the fonts that were asked for. Most steps of the
+    * scale slider land here, since the setting moves in hundredths
+    * and font_size is an integer, and a title-margin change never
+    * moves either size. */
+   if (     font_driver_matches(xmb->font,  fontpath, xmb->font_size)
+         && font_driver_matches(xmb->font2, fontpath, xmb->font2_size))
+      return false;
+
    /* Build before releasing, so the old atlas stays valid for any
     * frame still in flight and there is no window with no font at
     * all. */
@@ -7819,6 +7831,8 @@ static void xmb_rebuild_fonts(xmb_handle_t *xmb, bool is_threaded,
       if (old_font2)
          font_driver_free(old_font2);
    }
+
+   return true;
 }
 
 /* Apply a scale factor / layout change in place.
@@ -7832,34 +7846,28 @@ static void xmb_rebuild_fonts(xmb_handle_t *xmb, bool is_threaded,
  * xmb_render(), where they are exactly what is unsafe. */
 static void xmb_set_layout(xmb_handle_t *xmb, bool is_threaded)
 {
-   settings_t *settings   = config_get_ptr();
-   const char *menu_font  = settings->paths.path_menu_xmb_font;
-   float old_font_size    = xmb->font_size;
-   float old_font2_size   = xmb->font2_size;
-   bool font_path_changed = !string_is_equal(xmb->last_font_path,
-         menu_font ? menu_font : "");
+   const char *wideglyph_str = msg_hash_get_wideglyph_str();
+   bool wideglyph_changed    = (xmb->wideglyph_str != wideglyph_str);
+   bool rebuilt;
 
+   /* The geometry is cheap and always applied. Rasterising the two
+    * atlases is not, so xmb_rebuild_fonts() decides for itself
+    * whether anything it would build differs from what is loaded. */
    xmb_layout(xmb);
+   rebuilt = xmb_rebuild_fonts(xmb, is_threaded, true);
 
-   /* The geometry above is cheap and always applied. Rasterising two
-    * atlases is not, and the setting steps in hundredths, so most
-    * steps of the slider do not move the font size at all — and a
-    * title-margin change never does. Rebuild only when it matters. */
-   if (     font_path_changed
-         || !xmb->font
-         || !xmb->font2
-         || (xmb->font_size  != old_font_size)
-         || (xmb->font2_size != old_font2_size))
+   /* The wide-glyph sample follows the menu language and can move
+    * without the face doing so, in which case the widths derived from
+    * it still need recomputing against the font already loaded. */
+   if (rebuilt || wideglyph_changed)
    {
-      xmb_rebuild_fonts(xmb, is_threaded, true);
-
-      xmb->wideglyph_str = msg_hash_get_wideglyph_str();
+      xmb->wideglyph_str = wideglyph_str;
       xmb_compute_wideglyph(xmb);
-
-      /* Title metrics are measured against the font that just
-       * changed. */
-      xmb_set_title(xmb);
    }
+
+   /* Title metrics are measured against the font. */
+   if (rebuilt)
+      xmb_set_title(xmb);
 }
 
 static void xmb_context_reset_internal(xmb_handle_t *xmb,

samples/gfx/font_lifecycle/font_lifecycle_test.c

@@ -451,6 +451,63 @@ int main(void)
       CHECK(live_renderer_state == 0, "flush releases everything queued");
    }
 
+   /* 11b. font_driver_matches(): the predicate the layout paths use to
+    *      skip a rebuild that would produce the font already loaded.
+    *      A wrong yes keeps a font that should have been replaced, so
+    *      the cases that must answer no matter more than the one that
+    *      must answer yes. */
+   {
+      font_data_t *m = mk("/tmp/san/match.ttf", 16.0f);
+      font_data_t *stale;
+
+      CHECK(m != NULL, "match font created");
+      CHECK(font_driver_matches(m, "/tmp/san/match.ttf", 16.0f),
+            "same path and size matches");
+      CHECK(!font_driver_matches(m, "/tmp/san/match.ttf", 16.5f),
+            "a different size does not match");
+      CHECK(!font_driver_matches(m, "/tmp/san/other.ttf", 16.0f),
+            "a different path does not match");
+      CHECK(!font_driver_matches(m, NULL, 16.0f),
+            "a pathless request does not match a font with a path");
+      CHECK(!font_driver_matches(m, "", 16.0f),
+            "an empty path does not match a font with a path");
+      CHECK(!font_driver_matches(NULL, "/tmp/san/match.ttf", 16.0f),
+            "NULL never matches");
+
+      /* A language switch re-resolves the face in place. The
+       * predicate must follow the font, not the request that built
+       * it, or the caller would skip past a face change. */
+      font_driver_set_language_font(m, "/assets/pkg", "/tmp/san/match.ttf");
+      test_language = TEST_LANG_KOREAN;
+      font_driver_reload_fonts();
+      CHECK(!font_driver_matches(m, "/tmp/san/match.ttf", 16.0f),
+            "a re-resolved face no longer matches the old path");
+      CHECK(font_driver_matches(m, m->path, 16.0f),
+            "and does match the face it now carries");
+      test_language = 0;
+      font_driver_reload_fonts();
+
+      /* The case the liveness walk exists for: a caller that frees a
+       * font and leaves the pointer behind. Reading through it would
+       * be a use-after-free, and the honest answer is no. */
+      stale = mk("/tmp/san/stale.ttf", 16.0f);
+      font_driver_free(stale);
+      CHECK(!font_driver_matches(stale, "/tmp/san/stale.ttf", 16.0f),
+            "a freed handle does not match");
+
+      /* Same again for one released through the deferred queue,
+       * before and after the window closes. */
+      stale = mk("/tmp/san/stale2.ttf", 16.0f);
+      font_driver_free_deferred(stale);
+      CHECK(font_driver_matches(stale, "/tmp/san/stale2.ttf", 16.0f),
+            "a retired handle still matches inside its window");
+      font_driver_free_pending(true);
+      CHECK(!font_driver_matches(stale, "/tmp/san/stale2.ttf", 16.0f),
+            "and stops matching once released");
+
+      font_driver_free(m);
+   }
+
    /* 12. retiring NULL is allowed, so callers need no guard. */
    font_driver_free_deferred(NULL);
    font_driver_free_pending(false);