[PATCH 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?)

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

[Tim Clem]

From: Tim Clem

---
 dlls/dwrite/font.c | 71 +++++++++++++++++++++++++++++++++++++---------
 1 file changed, 57 insertions(+), 14 deletions(-)

diff --git a/dlls/dwrite/font.c b/dlls/dwrite/font.c
index 65bf7021938..70dd78478bc 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
@@ -485,6 +490,7 @@ struct dwrite_fontset
     LONG refcount;
     IDWriteFactory7 *factory;

+    BOOL owns_entries;  /* False if this is borrowing entries from a collection */
     struct dwrite_fontset_entry **entries;
     unsigned int count;
 };
@@ -501,7 +507,7 @@ struct dwrite_fontset_builder
 };

static HRESULT fontset_create_from_font_data(IDWriteFactory7 *factory, struct dwrite_font_data **fonts,
-        unsigned int count, IDWriteFontSet1 **ret);
+        unsigned int count, BOOL owns_entries, IDWriteFontSet1 **ret);

static void dwrite_grab_font_table(void *context, UINT32 table, const BYTE **data, UINT32 *size, void **data_context)
 {
@@ -2740,7 +2746,7 @@ static HRESULT WINAPI dwritefontlist2_GetFontSet(IDWriteFontList2 *iface, IDWrit
     TRACE("%p, %p.\n", iface, fontset);

return fontset_create_from_font_data(fontlist->family->collection->factory, fontlist->fonts,
-            fontlist->font_count, fontset);
+            fontlist->font_count, TRUE, fontset);
 }

static const IDWriteFontList2Vtbl dwritefontlistvtbl =
@@ -3074,7 +3080,7 @@ static HRESULT WINAPI dwritefontfamily2_GetFontSet(IDWriteFontFamily2 *iface, ID
     TRACE("%p, %p.\n", iface, fontset);

return fontset_create_from_font_data(family->collection->factory, family->data->fonts,
-            family->data->count, fontset);
+            family->data->count, TRUE, fontset);
 }

static const IDWriteFontFamily2Vtbl fontfamilyvtbl =
@@ -3158,7 +3164,7 @@ static HRESULT WINAPI dwritefontfamilylist2_GetFontSet(IDWriteFontList2 *iface,
     TRACE("%p, %p.\n", iface, fontset);

return fontset_create_from_font_data(family->collection->factory, family->data->fonts,
-            family->data->count, fontset);
+            family->data->count, TRUE, fontset);
 }

static const IDWriteFontList2Vtbl fontfamilylistvtbl =
@@ -3263,6 +3269,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 +3285,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 +3413,11 @@ 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, BOOL owns_entries);
+
+static struct dwrite_fontset_entry * addref_fontset_entry(struct dwrite_fontset_entry *entry);
+
 static HRESULT fontset_create_from_font_collection(struct dwrite_fontcollection *collection, IDWriteFontSet1 **fontset)
 {
     struct dwrite_font_data **fonts;
@@ -3410,6 +3426,23 @@ 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 *object;
+        if (!(object = calloc(1, sizeof(*object))))
+            return E_OUTOFMEMORY;
+
+        init_fontset(object, collection->factory, collection->fontset_entries, collection->entry_count, FALSE);
+
+        for (i = 0; i < collection->entry_count; ++i)
+            addref_fontset_entry(collection->fontset_entries[i]);
+
+        *fontset = (IDWriteFontSet1 *)&object->IDWriteFontSet3_iface;
+
+        return S_OK;
+    }
+
     for (i = 0; i < collection->count; ++i)
         count += collection->family_data[i]->count;

@@ -3420,7 +3453,18 @@ static HRESULT fontset_create_from_font_collection(struct dwrite_fontcollection
         for (j = 0; j < collection->family_data[i]->count; ++j)
             fonts[k++] = collection->family_data[i]->fonts[j];

-    hr = fontset_create_from_font_data(collection->factory, fonts, count, fontset);
+    hr = fontset_create_from_font_data(collection->factory, fonts, count, FALSE, fontset);
+
+    /* Cache the entries from the font set. */
+    if (SUCCEEDED(hr))
+    {
+        struct dwrite_fontset *object = impl_from_IDWriteFontSet3((IDWriteFontSet3 *)*fontset);
+        collection->fontset_entries = object->entries;
+        collection->entry_count = object->count;
+        /* Add references for the collection itself. */
+        for (i = 0; i < collection->entry_count; ++i)
+            addref_fontset_entry(collection->fontset_entries[i]);
+    }

free(fonts);

@@ -7753,9 +7797,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);
@@ -7769,7 +7810,8 @@ static ULONG WINAPI dwritefontset_Release(IDWriteFontSet3 *iface)
         IDWriteFactory7_Release(set->factory);
         for (i = 0; i < set->count; ++i)
             release_fontset_entry(set->entries[i]);
-        free(set->entries);
+        if (set->owns_entries)
+            free(set->entries);
         free(set);
     }

@@ -7947,7 +7989,7 @@ static HRESULT WINAPI dwritefontset_GetMatchingFonts(IDWriteFontSet3 *iface, DWR
         entries = NULL;
     }

-    init_fontset(object, set->factory, entries, matched_count);
+    init_fontset(object, set->factory, entries, matched_count, TRUE);

*filtered_set = (IDWriteFontSet *)&object->IDWriteFontSet3_iface;

@@ -8154,7 +8196,7 @@ static HRESULT fontset_create_entry(IDWriteFontFile *file, DWRITE_FONT_FACE_TYPE
 }

static void init_fontset(struct dwrite_fontset *object, IDWriteFactory7 *factory,
-        struct dwrite_fontset_entry **entries, unsigned int count)
+        struct dwrite_fontset_entry **entries, unsigned int count, BOOL owns_entries)
 {
     object->IDWriteFontSet3_iface.lpVtbl = &fontsetvtbl;
     object->refcount = 1;
@@ -8162,10 +8204,11 @@ static void init_fontset(struct dwrite_fontset *object, IDWriteFactory7 *factory
     IDWriteFactory7_AddRef(object->factory);
     object->entries = entries;
     object->count = count;
+    object->owns_entries = owns_entries;
 }

static HRESULT fontset_create_from_font_data(IDWriteFactory7 *factory, struct dwrite_font_data **fonts,
-        unsigned int count, IDWriteFontSet1 **ret)
+        unsigned int count, BOOL owns_entries, IDWriteFontSet1 **ret)
 {
     struct dwrite_fontset_entry **entries = NULL;
     struct dwrite_fontset *object;
@@ -8186,7 +8229,7 @@ static HRESULT fontset_create_from_font_data(IDWriteFactory7 *factory, struct dw
                     fonts[i]->simulations, &entries[i]);
         }
     }
-    init_fontset(object, factory, entries, count);
+    init_fontset(object, factory, entries, count, owns_entries);

*ret = (IDWriteFontSet1 *)&object->IDWriteFontSet3_iface;

@@ -8210,7 +8253,7 @@ static HRESULT fontset_builder_create_fontset(IDWriteFactory7 *factory, struct d
         for (i = 0; i < count; ++i)
             entries[i] = addref_fontset_entry(src_entries[i]);
     }
-    init_fontset(object, factory, entries, count);
+    init_fontset(object, factory, entries, count, TRUE);

*ret = (IDWriteFontSet *)&object->IDWriteFontSet3_iface;

[Nikolay Sivov (＠nsivov)]

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.