[PATCH] dwrite: Implement IDWriteCollection1::GetFontSet.

[Daniel Lehman]

Signed-off-by: Daniel Lehman <dlehman25(a)gmail.com>
---
the factory refcount is incremented by both:
- create_fontset_builder
- IDWriteFontSetBuilder2_CreateFontSet

there is no leak as the factory is released at end
it's just the intermediate counts that mismatch with windows

---
 dlls/dwrite/font.c       | 37 +++++++++++++++++++++++++++++++++++--
 dlls/dwrite/tests/font.c | 35 ++++++++++++++++++++++++++++-------
 2 files changed, 63 insertions(+), 9 deletions(-)

diff --git a/dlls/dwrite/font.c b/dlls/dwrite/font.c
index b7071314bb2..a6394ec58bb 100644
--- a/dlls/dwrite/font.c
+++ b/dlls/dwrite/font.c
@@ -3117,9 +3117,42 @@ static HRESULT WINAPI dwritefontcollection_GetFontFromFontFace(IDWriteFontCollec
 
 static HRESULT WINAPI dwritefontcollection1_GetFontSet(IDWriteFontCollection3 *iface, IDWriteFontSet **fontset)
 {
-    FIXME("%p, %p.\n", iface, fontset);
+    struct dwrite_fontcollection *collection = impl_from_IDWriteFontCollection3(iface);
+    IDWriteFontSetBuilder2 *builder;
+    IDWriteFontFaceReference *ref;
+    IDWriteFontFamily2 *family;
+    UINT32 i, nfamilies;
+    UINT32 j, nfonts;
+    HRESULT hr;
 
-    return E_NOTIMPL;
+    *fontset = NULL;
+    if (FAILED(hr = create_fontset_builder(collection->factory, &builder)))
+        return hr;
+
+    nfamilies = IDWriteFontCollection3_GetFontFamilyCount(iface);
+    for (i = 0; i < nfamilies; i++)
+    {
+        if (FAILED(hr = IDWriteFontCollection3_GetFontFamily(iface, i, &family)))
+            break;
+
+        nfonts = IDWriteFontFamily2_GetFontCount(family);
+        for (j = 0; j < nfonts; j++)
+        {
+            if (FAILED(hr = IDWriteFontFamily2_GetFontFaceReference(family, j, &ref)))
+                break;
+            if (IDWriteFontFaceReference_GetSimulations(ref) == DWRITE_FONT_SIMULATIONS_NONE)
+                hr = IDWriteFontSetBuilder2_AddFontFaceReference(builder, ref);
+            IDWriteFontFaceReference_Release(ref);
+            if (FAILED(hr))
+                break;
+        }
+        IDWriteFontFamily2_Release(family);
+    }
+
+    if (SUCCEEDED(hr))
+        hr = IDWriteFontSetBuilder2_CreateFontSet(builder, fontset);
+    IDWriteFontSetBuilder2_Release(builder);
+    return hr;
 }
 
 static HRESULT WINAPI dwritefontcollection1_GetFontFamily(IDWriteFontCollection3 *iface, UINT32 index,
diff --git a/dlls/dwrite/tests/font.c b/dlls/dwrite/tests/font.c
index ba4a2793372..8adb91108d5 100644
--- a/dlls/dwrite/tests/font.c
+++ b/dlls/dwrite/tests/font.c
@@ -2460,15 +2460,20 @@ static void test_system_fontcollection(void)
     IDWriteFontCollection2 *collection2;
     IDWriteFontCollection3 *collection3;
     IDWriteFactory *factory, *factory2;
+    IDWriteFontFaceReference *fontref;
     IDWriteFontFileLoader *loader;
     IDWriteFontFamily *family;
     IDWriteFontFace *fontface;
     IDWriteFactory6 *factory6;
     IDWriteFontFile *file;
     IDWriteFont *font;
+    UINT32 nfamilies;
+    UINT32 nfonts;
+    UINT32 count2;
+    UINT32 count;
+    UINT32 i, j;
     HRESULT hr;
     ULONG ref;
-    UINT32 i;
     BOOL ret;
 
     factory = create_factory();
@@ -2597,11 +2602,8 @@ static void test_system_fontcollection(void)
         EXPECT_REF(collection1, 2);
         EXPECT_REF(factory, 2);
         hr = IDWriteFontCollection1_GetFontSet(collection1, &fontset);
-    todo_wine
         ok(hr == S_OK, "Failed to get fontset, hr %#x.\n", hr);
-    if (hr == S_OK) {
         EXPECT_REF(collection1, 2);
-        EXPECT_REF(factory, 2);
         EXPECT_REF(fontset, 1);
 
         hr = IDWriteFontCollection1_GetFontSet(collection1, &fontset2);
@@ -2613,18 +2615,37 @@ static void test_system_fontcollection(void)
         hr = IDWriteFactory_QueryInterface(factory, &IID_IDWriteFactory3, (void **)&factory3);
         ok(hr == S_OK, "Failed to get IDWriteFactory3 interface, hr %#x.\n", hr);
 
-        EXPECT_REF(factory, 3);
         hr = IDWriteFactory3_GetSystemFontSet(factory3, &fontset2);
         ok(hr == S_OK, "Failed to get system font set, hr %#x.\n", hr);
         ok(fontset != fontset2, "Expected new fontset instance.\n");
         EXPECT_REF(fontset2, 1);
-        EXPECT_REF(factory, 4);
+
+        count = IDWriteFontSet_GetFontCount(fontset);
+        count2 = 0;
+        nfamilies = IDWriteFontCollection1_GetFontFamilyCount(collection1);
+        for (i = 0; i < nfamilies; i++)
+        {
+            hr = IDWriteFontCollection1_GetFontFamily(collection1, i, &family1);
+            ok(hr == S_OK, "Failed to get font family, hr %#x.\n", hr);
+
+            nfonts = IDWriteFontFamily1_GetFontCount(family1);
+            for (j = 0; j < nfonts; j++)
+            {
+                hr = IDWriteFontFamily1_GetFontFaceReference(family1, j, &fontref);
+                ok(hr == S_OK, "Failed to get font reference, hr %#x.\n", hr);
+
+                if (!IDWriteFontFaceReference_GetSimulations(fontref))
+                    count2++;
+                IDWriteFontFaceReference_Release(fontref);
+            }
+            IDWriteFontFamily1_Release(family1);
+        }
+        ok(count == count2, "Expected %#x, got %#x\n", count, count2);
 
         IDWriteFontSet_Release(fontset2);
         IDWriteFontSet_Release(fontset);
 
         IDWriteFactory3_Release(factory3);
-    }
         IDWriteFontCollection1_Release(collection1);
     }
     else
-- 
2.25.1

[Nikolay Sivov]

I'm not sure if that's the right direction. It's possible to create a
collection from existing set, and then get set back. That could mean
that set is a level below.

Something to try is to check if collections created from same set, but
with different models, will return same set in GetFontSet().

Related to that, it's worth checking if returned set contains duplicates
in a sense of same file refrences, and if it does, what kind of
duplicates. It could be that it lists each {file, face_index} tuple
once, or does actually include simulations/variations as separate
entries. And collection model could affect that.

[Daniel Lehman]

> Something to try is to check if collections created from same set, but
> with different models, will return same set in GetFontSet().

it's mostly the same, mostly...

the keys and number of fonts are the same in all font sets - the
original FontSet and FontSet created from FontCollections of each
model.  on my Windows 10, the System FontSet has 232 fonts

but the order of fonts in FontSet can vary by the FontCollection model
and both have a different order from the original FontSet.  not sure
how important that is, though

for example, the difference in ordering between each model is with the
Sitka font, which has 6 families (Small, Text, Subheading Heading,
Display, Banner) and 4 faces each (Regular, Bold, Italic, Bold
Italic):
- TYPOGRAPHIC
    - grouped by weight/stretch/style, incrementing index:
        - [Small, Text, Subheading Heading, Display, Banner] Regular
(SITKA.TTC index 0-5)
        - [Small, Text, Subheading Heading, Display, Banner] Italic
(SITKAI.TTC index 0-5)
        - ...
- WEIGHT_STRETCH_STYLE
    - grouped by index, different weight/stretch/style
        - Small [Regular, Italic, Bold, Bold Italic] (SITKA[IBZ].TTC index 0)
        - Text [Regular, Italic, Bold, Bold Italic] (SITKA[IBZ].TTC index 1)
        - ...

the size of the FontCollection varies, with MODEL_WEIGHT_STRETCH_STYLE
having more fonts.  on my Windows 10:
- TYPOGRAPHIC
    - 89 families
    - just one 'Sitka' family
    - same number of fonts as FontSet (232 on my box)
        - 0 Simulations
- WEIGHT_STRETCH_STYLE
    - 94 families
    - 6 families for Sitka: Small, Text, Subheading Heading, Display, Banner
    - more fonts (509 vs 232)
        - each FontFamily has 1 or more Simulations

IDWriteFactory3::CreateFontCollectionFromFontSet uses MODEL_WEIGHT_STRETCH_STYLE
> Related to that, it's worth checking if returned set contains duplicates
> in a sense of same file refrences, and if it does, what kind of duplicates.

FontSets contain duplicate keys that can differ on index (Cambria.ttc,
Sitka.ttc) or weight/stretch/style (Bahnshrift.ttf)

thanks
daniel