[PATCH 2/2] gdiplus: Create FontFamily objects only once for the FontCollection.

[Dmitry Timoshkov]

.Net 4.7+ depends on this behaviour and expects to be able to do pointer
equality tests for FontFamily objects.

Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru>
---
 dlls/gdiplus/font.c       | 208 ++++++++++++++++----------------------
 dlls/gdiplus/tests/font.c |   6 --
 2 files changed, 89 insertions(+), 125 deletions(-)

diff --git a/dlls/gdiplus/font.c b/dlls/gdiplus/font.c
index eee272082f..a7719a9458 100644
--- a/dlls/gdiplus/font.c
+++ b/dlls/gdiplus/font.c
@@ -115,8 +115,6 @@ typedef struct
 #define MS_OS2_TAG MS_MAKE_TAG('O','S','/','2')
 #define MS_HHEA_TAG MS_MAKE_TAG('h','h','e','a')
 
-static GpStatus clone_font_family(const GpFontFamily *, GpFontFamily **);
-
 static GpFontCollection installedFontCollection = {0};
 
 /*******************************************************************************
@@ -183,13 +181,7 @@ GpStatus WINGDIPAPI GdipCreateFont(GDIPCONST GpFontFamily *fontFamily,
     (*font)->unit = unit;
     (*font)->emSize = emSize;
     (*font)->otm = otm;
-
-    stat = clone_font_family(fontFamily, &(*font)->family);
-    if (stat != Ok)
-    {
-        heap_free(*font);
-        return stat;
-    }
+    (*font)->family = (GpFontFamily *)fontFamily;
 
     TRACE("<-- %p\n", *font);
 
@@ -322,7 +314,8 @@ GpStatus WINGDIPAPI GdipGetFamily(GpFont *font, GpFontFamily **family)
     if (!(font && family))
         return InvalidParameter;
 
-    return GdipCloneFontFamily(font->family, family);
+    *family = font->family;
+    return Ok;
 }
 
 static REAL get_font_size(const GpFont *font)
@@ -518,8 +511,6 @@ GpStatus WINGDIPAPI GdipGetLogFontW(GpFont *font, GpGraphics *graphics, LOGFONTW
  */
 GpStatus WINGDIPAPI GdipCloneFont(GpFont *font, GpFont **cloneFont)
 {
-    GpStatus stat;
-
     TRACE("(%p, %p)\n", font, cloneFont);
 
     if(!font || !cloneFont)
@@ -529,10 +520,7 @@ GpStatus WINGDIPAPI GdipCloneFont(GpFont *font, GpFont **cloneFont)
     if(!*cloneFont)    return OutOfMemory;
 
     **cloneFont = *font;
-    stat = GdipCloneFontFamily(font->family, &(*cloneFont)->family);
-    if (stat != Ok) heap_free(*cloneFont);
-
-    return stat;
+    return Ok;
 }
 
 /*******************************************************************************
@@ -699,29 +687,6 @@ static BOOL get_font_metrics(HDC hdc, struct font_metrics *fm)
     return TRUE;
 }
 
-static GpStatus find_installed_font(const WCHAR *name, struct font_metrics *fm)
-{
-    LOGFONTW lf;
-    HDC hdc = CreateCompatibleDC(0);
-    GpStatus ret = FontFamilyNotFound;
-
-    if(!EnumFontFamiliesW(hdc, name, is_font_installed_proc, (LPARAM)&lf))
-    {
-        HFONT hfont, old_font;
-
-        lstrcpyW(fm->facename, lf.lfFaceName);
-
-        hfont = CreateFontIndirectW(&lf);
-        old_font = SelectObject(hdc, hfont);
-        ret = get_font_metrics(hdc, fm) ? Ok : NotTrueTypeFont;
-        SelectObject(hdc, old_font);
-        DeleteObject(hfont);
-    }
-
-    DeleteDC(hdc);
-    return ret;
-}
-
 /*******************************************************************************
  * GdipCreateFontFamilyFromName [GDIPLUS.@]
  *
@@ -743,48 +708,45 @@ static GpStatus find_installed_font(const WCHAR *name, struct font_metrics *fm)
  */
 
 GpStatus WINGDIPAPI GdipCreateFontFamilyFromName(GDIPCONST WCHAR *name,
-                                        GpFontCollection *fontCollection,
-                                        GpFontFamily **FontFamily)
+                                        GpFontCollection *collection,
+                                        GpFontFamily **family)
 {
-    GpStatus stat;
-    GpFontFamily* ffamily;
-    struct font_metrics fm;
+    HDC hdc;
+    LOGFONTW lf;
+    GpStatus status;
+    int i;
 
-    TRACE("%s, %p %p\n", debugstr_w(name), fontCollection, FontFamily);
+    TRACE("%s, %p %p\n", debugstr_w(name), collection, family);
 
-    if (!(name && FontFamily))
+    if (!name || !family)
         return InvalidParameter;
-    if (fontCollection)
-        FIXME("No support for FontCollections yet!\n");
-
-    stat = find_installed_font(name, &fm);
-    if (stat != Ok) return stat;
-
-    ffamily = heap_alloc_zero(sizeof (GpFontFamily));
-    if (!ffamily) return OutOfMemory;
-
-    lstrcpyW(ffamily->FamilyName, fm.facename);
-    ffamily->em_height = fm.em_height;
-    ffamily->ascent = fm.ascent;
-    ffamily->descent = fm.descent;
-    ffamily->line_spacing = fm.line_spacing;
-    ffamily->dpi = fm.dpi;
 
-    *FontFamily = ffamily;
-
-    TRACE("<-- %p\n", ffamily);
+    if (!collection)
+    {
+        status = GdipNewInstalledFontCollection(&collection);
+        if (status != Ok) return status;
+    }
 
-    return Ok;
-}
+    status = FontFamilyNotFound;
 
-static GpStatus clone_font_family(const GpFontFamily *family, GpFontFamily **clone)
-{
-    *clone = heap_alloc_zero(sizeof(GpFontFamily));
-    if (!*clone) return OutOfMemory;
+    hdc = CreateCompatibleDC(0);
 
-    **clone = *family;
+    if (!EnumFontFamiliesW(hdc, name, is_font_installed_proc, (LPARAM)&lf))
+    {
+        for (i = 0; i < collection->count; i++)
+        {
+            if (!wcsicmp(lf.lfFaceName, collection->FontFamilies[i]->FamilyName))
+            {
+                *family = collection->FontFamilies[i];
+                TRACE("<-- %p\n", *family);
+                status = Ok;
+                break;
+            }
+        }
+    }
 
-    return Ok;
+    DeleteDC(hdc);
+    return status;
 }
 
 /*******************************************************************************
@@ -799,20 +761,14 @@ static GpStatus clone_font_family(const GpFontFamily *family, GpFontFamily **clo
  * RETURNS
  *  SUCCESS: Ok
  */
-GpStatus WINGDIPAPI GdipCloneFontFamily(GpFontFamily* FontFamily, GpFontFamily** clonedFontFamily)
+GpStatus WINGDIPAPI GdipCloneFontFamily(GpFontFamily *family, GpFontFamily **clone)
 {
-    GpStatus status;
-
-    if (!(FontFamily && clonedFontFamily)) return InvalidParameter;
-
-    TRACE("%p (%s), %p\n", FontFamily,
-            debugstr_w(FontFamily->FamilyName), clonedFontFamily);
-
-    status = clone_font_family(FontFamily, clonedFontFamily);
-    if (status != Ok) return status;
+    if (!family || !clone)
+        return InvalidParameter;
 
-    TRACE("<-- %p\n", *clonedFontFamily);
+    TRACE("%p (%s), %p\n", family, debugstr_w(family->FamilyName), clone);
 
+    *clone = family;
     return Ok;
 }
 
@@ -869,9 +825,6 @@ GpStatus WINGDIPAPI GdipDeleteFontFamily(GpFontFamily *FontFamily)
 {
     if (!FontFamily)
         return InvalidParameter;
-    TRACE("Deleting %p (%s)\n", FontFamily, debugstr_w(FontFamily->FamilyName));
-
-    heap_free (FontFamily);
 
     return Ok;
 }
@@ -1498,6 +1451,7 @@ struct add_font_param
     GpFontCollection *collection;
     BOOL is_system;
     GpStatus stat;
+    HDC hdc;
 };
 
 static INT CALLBACK add_font_proc(const LOGFONTW *lfw, const TEXTMETRICW *ntm, DWORD type, LPARAM lParam);
@@ -1528,10 +1482,9 @@ GpStatus WINGDIPAPI GdipPrivateAddMemoryFont(GpFontCollection* fontCollection,
     else
     {
         struct add_font_param param;
-        HDC hdc;
         LOGFONTW lfw;
 
-        hdc = CreateCompatibleDC(0);
+        param.hdc = CreateCompatibleDC(0);
 
         /* Truncate name if necessary, GDI32 can't deal with long names */
         if(lstrlenW(name) > LF_FACESIZE - 1)
@@ -1543,10 +1496,10 @@ GpStatus WINGDIPAPI GdipPrivateAddMemoryFont(GpFontCollection* fontCollection,
 
         param.collection = fontCollection;
         param.is_system = FALSE;
-        if (!EnumFontFamiliesExW(hdc, &lfw, add_font_proc, (LPARAM)¶m, 0))
+        if (!EnumFontFamiliesExW(param.hdc, &lfw, add_font_proc, (LPARAM)¶m, 0))
             ret = param.stat;
 
-        DeleteDC(hdc);
+        DeleteDC(param.hdc);
     }
     heap_free(name);
     return ret;
@@ -1575,7 +1528,6 @@ GpStatus WINGDIPAPI GdipGetFontCollectionFamilyList(
         GpFontFamily* gpfamilies[], INT* numFound)
 {
     INT i;
-    GpStatus stat=Ok;
 
     TRACE("%p, %d, %p, %p\n", fontCollection, numSought, gpfamilies, numFound);
 
@@ -1584,31 +1536,24 @@ GpStatus WINGDIPAPI GdipGetFontCollectionFamilyList(
 
     memset(gpfamilies, 0, sizeof(*gpfamilies) * numSought);
 
-    for (i = 0; i < numSought && i < fontCollection->count && stat == Ok; i++)
+    for (i = 0; i < numSought && i < fontCollection->count; i++)
     {
-        stat = GdipCloneFontFamily(fontCollection->FontFamilies[i], &gpfamilies[i]);
+        gpfamilies[i] = fontCollection->FontFamilies[i];
     }
 
-    if (stat == Ok)
-        *numFound = i;
-    else
-    {
-        int numToFree=i;
-        for (i=0; i<numToFree; i++)
-        {
-            GdipDeleteFontFamily(gpfamilies[i]);
-            gpfamilies[i] = NULL;
-        }
-    }
+    *numFound = i;
 
-    return stat;
+    return Ok;
 }
 
 void free_installed_fonts(void)
 {
-    while (installedFontCollection.count)
-        GdipDeleteFontFamily(installedFontCollection.FontFamilies[--installedFontCollection.count]);
+    INT i;
+
+    for (i = 0; i < installedFontCollection.count; i++)
+        heap_free(installedFontCollection.FontFamilies[i]);
     heap_free(installedFontCollection.FontFamilies);
+
     installedFontCollection.FontFamilies = NULL;
     installedFontCollection.allocated = 0;
 }
@@ -1618,8 +1563,9 @@ static INT CALLBACK add_font_proc(const LOGFONTW *lfw, const TEXTMETRICW *ntm,
 {
     struct add_font_param *param = (struct add_font_param *)lParam;
     GpFontCollection *fonts = param->collection;
-    GpFontFamily* family;
-    GpStatus stat;
+    GpFontFamily *family;
+    HFONT hfont, old_hfont;
+    struct font_metrics fm;
     int i;
 
     param->stat = Ok;
@@ -1651,25 +1597,50 @@ static INT CALLBACK add_font_proc(const LOGFONTW *lfw, const TEXTMETRICW *ntm,
         fonts->allocated = new_alloc_count;
     }
 
-    if ((stat = GdipCreateFontFamilyFromName(lfw->lfFaceName, NULL, &family)) != Ok)
+    family = heap_alloc(sizeof(*family));
+    if (!family)
     {
-        WARN("Failed to create font family for %s, status %d.\n", debugstr_w(lfw->lfFaceName), stat);
         if (param->is_system)
             return 1;
-        param->stat = stat;
+
+        param->stat = OutOfMemory;
         return 0;
     }
 
     /* skip duplicates */
     for (i=0; i<fonts->count; i++)
     {
-        if (wcsicmp(family->FamilyName, fonts->FontFamilies[i]->FamilyName) == 0)
+        if (wcsicmp(lfw->lfFaceName, fonts->FontFamilies[i]->FamilyName) == 0)
         {
-            GdipDeleteFontFamily(family);
+            heap_free(family);
             return 1;
         }
     }
 
+    hfont = CreateFontIndirectW(lfw);
+    old_hfont = SelectObject(param->hdc, hfont);
+
+    if (!get_font_metrics(param->hdc, &fm))
+    {
+        SelectObject(param->hdc, old_hfont);
+        DeleteObject(hfont);
+
+        heap_free(family);
+        param->stat = OutOfMemory;
+        return 0;
+    }
+
+    SelectObject(param->hdc, old_hfont);
+    DeleteObject(hfont);
+
+    family->em_height = fm.em_height;
+    family->ascent = fm.ascent;
+    family->descent = fm.descent;
+    family->line_spacing = fm.line_spacing;
+    family->dpi = fm.dpi;
+
+    lstrcpyW(family->FamilyName, lfw->lfFaceName);
+
     fonts->FontFamilies[fonts->count++] = family;
 
     return 1;
@@ -1686,10 +1657,9 @@ GpStatus WINGDIPAPI GdipNewInstalledFontCollection(
     if (installedFontCollection.count == 0)
     {
         struct add_font_param param;
-        HDC hdc;
         LOGFONTW lfw;
 
-        hdc = CreateCompatibleDC(0);
+        param.hdc = CreateCompatibleDC(0);
 
         lfw.lfCharSet = DEFAULT_CHARSET;
         lfw.lfFaceName[0] = 0;
@@ -1697,14 +1667,14 @@ GpStatus WINGDIPAPI GdipNewInstalledFontCollection(
 
         param.collection = &installedFontCollection;
         param.is_system = TRUE;
-        if (!EnumFontFamiliesExW(hdc, &lfw, add_font_proc, (LPARAM)¶m, 0))
+        if (!EnumFontFamiliesExW(param.hdc, &lfw, add_font_proc, (LPARAM)¶m, 0))
         {
             free_installed_fonts();
-            DeleteDC(hdc);
+            DeleteDC(param.hdc);
             return param.stat;
         }
 
-        DeleteDC(hdc);
+        DeleteDC(param.hdc);
     }
 
     *fontCollection = &installedFontCollection;
diff --git a/dlls/gdiplus/tests/font.c b/dlls/gdiplus/tests/font.c
index 36079fa7ad..9abb80faaf 100644
--- a/dlls/gdiplus/tests/font.c
+++ b/dlls/gdiplus/tests/font.c
@@ -141,7 +141,6 @@ static void test_createfont(void)
     expect(Ok, stat);
     stat = GdipGetFamilyName(fontfamily2, familyname, 0);
     expect(Ok, stat);
-todo_wine
     ok (fontfamily == fontfamily2, "Unexpected family instance.\n");
     ok (lstrcmpiW(Tahoma, familyname) == 0, "Expected Tahoma, got %s\n",
             wine_dbgstr_w(familyname));
@@ -345,7 +344,6 @@ static void test_fontfamily (void)
     ZeroMemory (itsName, sizeof(itsName));
     stat = GdipCloneFontFamily(family, &clonedFontFamily);
     expect (Ok, stat);
-todo_wine
     ok (family == clonedFontFamily, "Unexpected family instance.\n");
     GdipDeleteFontFamily(family);
     stat = GdipGetFamilyName(clonedFontFamily, itsName, LANG_NEUTRAL);
@@ -1238,7 +1236,6 @@ static void test_GdipGetFontCollectionFamilyList(void)
     status = GdipGetFontCollectionFamilyList(collection, 1, &family2, &found);
     ok(status == Ok, "Failed to get family list, status %d.\n", status);
     ok(found == 1, "Unexpected list count %d.\n", found);
-todo_wine
     ok(family2 == family, "Unexpected family instance.\n");
 
     status = GdipDeleteFontFamily(family);
@@ -1338,7 +1335,6 @@ static void test_CloneFont(void)
     expect(Ok, status);
 
     ret = is_family_in_collection(collection, family);
-todo_wine
     ok(ret, "family is not in collection\n");
 
     status = GdipCreateFont(family, 30.0f, FontStyleRegular, UnitPixel, &font);
@@ -1358,7 +1354,6 @@ todo_wine
 
     status = GdipGetFamily(font, &family2);
     expect(Ok, status);
-todo_wine
     ok(family == family2, "got %p\n", family2);
 
     status = GdipCloneFont(font, &font2);
@@ -1378,7 +1373,6 @@ todo_wine
 
     status = GdipGetFamily(font2, &family2);
     expect(Ok, status);
-todo_wine
     ok(family == family2, "got %p\n", family2);
 
     GdipDeleteFont(font2);
-- 
2.20.1

[Vincent Povirk (they/them)]

Signed-off-by: Vincent Povirk <vincent(a)codeweavers.com>