[PATCH v2 0/1] MR8906: Draft: dwrite: Cache fontset entries when one is created from a collection.

[Tim Clem (＠tclem)]

Motivation is Ubisoft Connect, which calls IDWriteFontCollection::GetFontSet many times on the system font collection, and then GetMatchingFonts on those sets. Without a change like this, each set created by GetFontSet has a separate cache of `dwrite_fontset_entry`s, so the GetMatchingFonts calls rescan every font on the system (via dwritefontset_GetMatchingFonts -> fontset_entry_is_matching -> fontset_entry_get_property, which does not find cached properties and thus makes a new file stream).

Perhaps it makes sense to not have the `owns_entries` flag in all the dwrite_fontset initializers, and just set it as needed in IDWriteFontCollection::GetFontSet? (And perhaps to invert it to `unowned_entries` or something, so the calloc initialization sets the most common value?)

--
  v2: dwrite: Reuse entries when a font set is created from a collection.

https://gitlab.winehq.org/wine/wine/-/merge_requests/8906

[PATCH v2 1/1] dwrite: Reuse entries when a font set is created from a collection.

[Tim Clem]

From: Tim Clem

---
 dlls/dwrite/font.c | 68 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 65 insertions(+), 3 deletions(-)

diff --git a/dlls/dwrite/font.c b/dlls/dwrite/font.c
index 65bf7021938..040502a07ed 100644
--- a/dlls/dwrite/font.c
+++ b/dlls/dwrite/font.c
@@ -346,6 +346,11 @@ struct dwrite_fontcollection
     struct dwrite_fontfamily_data **family_data;
     size_t size;
     size_t count;
+
+    /* Populated if GetFontSet is called on this collection. The
+       collection owns a reference to each of these entries. */
+    struct dwrite_fontset_entry **fontset_entries;
+    unsigned int entry_count;
 };

struct dwrite_fontfamily
@@ -3263,6 +3268,8 @@ static ULONG WINAPI dwritefontcollection_AddRef(IDWriteFontCollection3 *iface)
     return refcount;
 }

+static void release_fontset_entry(struct dwrite_fontset_entry *entry);
+
 static ULONG WINAPI dwritefontcollection_Release(IDWriteFontCollection3 *iface)
 {
     struct dwrite_fontcollection *collection = impl_from_IDWriteFontCollection3(iface);
@@ -3277,6 +3284,9 @@ static ULONG WINAPI dwritefontcollection_Release(IDWriteFontCollection3 *iface)
         for (i = 0; i < collection->count; ++i)
             release_fontfamily_data(collection->family_data[i]);
         free(collection->family_data);
+        for (i = 0; i < collection->entry_count; ++i)
+            release_fontset_entry(collection->fontset_entries[i]);
+        free(collection->fontset_entries);
         free(collection);
     }

@@ -3402,6 +3412,26 @@ static HRESULT WINAPI dwritefontcollection_GetFontFromFontFace(IDWriteFontCollec
     return hr;
 }

+static void init_fontset(struct dwrite_fontset *object, IDWriteFactory7 *factory,
+        struct dwrite_fontset_entry **entries, unsigned int count);
+
+static struct dwrite_fontset_entry * addref_fontset_entry(struct dwrite_fontset_entry *entry);
+
+/* Shallow copies an array of font set entries, adding a reference to each. */
+static HRESULT duplicate_fontset_entries(struct dwrite_fontset_entry **entries, unsigned int count,
+        struct dwrite_fontset_entry ***copy)
+{
+    unsigned int i;
+
+    if (!(*copy = malloc(count * sizeof(*entries))))
+        return E_OUTOFMEMORY;
+
+    for (i = 0; i < count; i++)
+        (*copy)[i] = addref_fontset_entry(entries[i]);
+
+    return S_OK;
+}
+
 static HRESULT fontset_create_from_font_collection(struct dwrite_fontcollection *collection, IDWriteFontSet1 **fontset)
 {
     struct dwrite_font_data **fonts;
@@ -3410,6 +3440,28 @@ static HRESULT fontset_create_from_font_collection(struct dwrite_fontcollection

*fontset = NULL;

+    /* Reuse cached entries if we've already made a set from this collection. */
+    if (collection->fontset_entries)
+    {
+        struct dwrite_fontset_entry **entries;
+        struct dwrite_fontset *object;
+        if (!(object = calloc(1, sizeof(*object))))
+            return E_OUTOFMEMORY;
+
+        hr = duplicate_fontset_entries(collection->fontset_entries, collection->entry_count, &entries);
+        if (FAILED(hr))
+        {
+            free(object);
+            return hr;
+        }
+
+        init_fontset(object, collection->factory, entries, collection->entry_count);
+
+        *fontset = (IDWriteFontSet1 *)&object->IDWriteFontSet3_iface;
+
+        return S_OK;
+    }
+
     for (i = 0; i < collection->count; ++i)
         count += collection->family_data[i]->count;

@@ -3422,6 +3474,19 @@ static HRESULT fontset_create_from_font_collection(struct dwrite_fontcollection

hr = fontset_create_from_font_data(collection->factory, fonts, count, fontset);

+    /* Cache the entries from the font set. */
+    if (SUCCEEDED(hr))
+    {
+        HRESULT copy_hr;
+        struct dwrite_fontset *object = impl_from_IDWriteFontSet3((IDWriteFontSet3 *)*fontset);
+
+        copy_hr = duplicate_fontset_entries(object->entries, object->count, &collection->fontset_entries);
+        if (SUCCEEDED(copy_hr))
+            collection->entry_count = object->count;
+        else
+            ERR("failed to cache font set entries on collection %p, 0x%08lx\n", collection, copy_hr);
+    }
+
     free(fonts);

return hr;
@@ -7753,9 +7818,6 @@ static IDWriteLocalizedStrings * fontset_entry_get_property(struct dwrite_fontse
     return value;
 }

-static void init_fontset(struct dwrite_fontset *object, IDWriteFactory7 *factory,
-       struct dwrite_fontset_entry **entries, unsigned int count);
-
 static ULONG WINAPI dwritefontset_Release(IDWriteFontSet3 *iface)
 {
     struct dwrite_fontset *set = impl_from_IDWriteFontSet3(iface);

[Tim Clem (＠tclem)]

On Thu Sep  4 20:04:15 2025 +0000, Nikolay Sivov wrote:
> We could make it work without such extra marker. One way would be to
> always allocate entries array of pointers for a new font set object, and
> copy existing arrays to it, addrefing entries. Another way is to add
> another layer with `"struct dwrite_fontset_entries { refcount; struct
> dwrite_fontset_entry **entries; entry_count };"` This way you'd create
> such structure once and can referencing it easily without copies.
> There are two different cases:
> - collection -> get_fontset. That gets you a complete set every time,
>
> but different instance of dwrite outer object implementing IDWriteFontSet;
> - fontset -> getmatchingfonts. That produces a subset, so you would
>
> create a new "struct dwrite_fontset_entries" if matching count does not
> equal full set count. If it's a smaller subset you'll have to go trough
> a loop to addref entries.
> But in collection -> GetFontSet() case you only need to allocate "struct
> fontset", set a pointer to "struct dwrite_fontset_entries *", and addref
> just fontset_entries, no need to loop.
> Let me know what you think, or if something is unclear.

Ah thanks, that makes sense. There's a new version up that takes the approach of giving each font set its own array that's a copy of the collection's (and caching a copy on the collection the first time a font set is made). That seemed far simpler to me than another level of indirection with its own refcount, though the lack of copying with that approach is appealing. Let me know what you think and if you'd prefer the other direction.

[Nikolay Sivov (＠nsivov)]

On Thu Sep  4 20:04:37 2025 +0000, Tim Clem wrote:
> Ah thanks, that makes sense. There's a new version up that takes the
> approach of giving each font set its own array that's a copy of the
> collection's (and caching a copy on the collection the first time a font
> set is made). That seemed far simpler to me than another level of
> indirection with its own refcount, though the lack of copying with that
> approach is appealing. Let me know what you think and if you'd prefer
> the other direction.

I think there is a better way. I'm going to rework existing parts a bit, so that it fits better.

[Nikolay Sivov (＠nsivov)]

@tclem this is what I had in mind !8961. With the exception of EUDC collection, which is easy to fix, every collection is now created from a font set. This makes it easy to reuse entries for following GetFontSet() calls. Could you take a look and ideally try to use it as a base, and test with it. I can certainly finish it too.

[Nikolay Sivov (＠nsivov)]

On Fri Sep 12 09:42:44 2025 +0000, Nikolay Sivov wrote:
> @tclem this is what I had in mind !8961. With the exception of EUDC
> collection, which is easy to fix, every collection is now created from a
> font set. This makes it easy to reuse entries for following GetFontSet()
> calls. Could you take a look and ideally try to use it as a base, and
> test with it. I can certainly finish it too.

According to test times, !8961 doubles execution time for font and layout tests, that recreate collections all the time. I'm going to look at that, it's a blocker.