[PATCH 1/8] gdi32: Implement AddFontMemResourceEx with temp files.

[Rémi Bernon]

This makes WineEngAddFontMemResourceEx, as well as the support for
loading font face from memory in freetype.c, obsolete.

Signed-off-by: Rémi Bernon 
---
 dlls/gdi32/font.c        | 68 +++++++++++++++++++++++++----------
 dlls/gdi32/freetype.c    | 78 ++++++++--------------------------------
 dlls/gdi32/gdi_private.h |  3 +-
 3 files changed, 65 insertions(+), 84 deletions(-)

diff --git a/dlls/gdi32/font.c b/dlls/gdi32/font.c
index e099bec5e81..e176752c2ce 100644
--- a/dlls/gdi32/font.c
+++ b/dlls/gdi32/font.c
@@ -3749,7 +3749,7 @@ fail:
  */
 INT WINAPI AddFontResourceExW( LPCWSTR str, DWORD fl, PVOID pdv )
 {
-    int ret = WineEngAddFontResourceEx(str, fl, pdv);
+    int ret = WineEngAddFontResourceEx( str, fl, pdv, FALSE );
     WCHAR *filename;
     BOOL hidden;

@@ -3771,7 +3771,7 @@ INT WINAPI AddFontResourceExW( LPCWSTR str, DWORD fl, PVOID pdv )
         else if ((filename = get_scalable_filename( str, &hidden )) != NULL)
         {
             if (hidden) fl |= FR_PRIVATE | FR_NOT_ENUM;
-            ret = WineEngAddFontResourceEx( filename, fl, pdv );
+            ret = WineEngAddFontResourceEx( filename, fl, pdv, FALSE );
             HeapFree( GetProcessHeap(), 0, filename );
         }
     }
@@ -3799,8 +3799,13 @@ BOOL WINAPI RemoveFontResourceW( LPCWSTR str )
  */
 HANDLE WINAPI AddFontMemResourceEx( PVOID pbFont, DWORD cbFont, PVOID pdv, DWORD *pcFonts)
 {
-    HANDLE ret;
-    DWORD num_fonts;
+    static const WCHAR tmpfmt_w[] = {'%', 's', '\', 'w', 'i', 'n', 'e', '-', 'f',
+                                     'o', 'n', 't',  '-', '%', '#', 'x', '-', '%',
+                                     '#', 'x', '.',  't', 'm', 'p', 0};
+    HANDLE file;
+    WCHAR tmppath_w[MAX_PATH], filename_w[MAX_PATH];
+    DWORD written;
+    LUID luid;

if (!pbFont || !cbFont || !pcFonts)
     {
@@ -3808,22 +3813,38 @@ HANDLE WINAPI AddFontMemResourceEx( PVOID pbFont, DWORD cbFont, PVOID pdv, DWORD
         return NULL;
     }

-    ret = WineEngAddFontMemResourceEx(pbFont, cbFont, pdv, &num_fonts);
-    if (ret)
+    __TRY { *pcFonts = 0; }
+    __EXCEPT_PAGE_FAULT
     {
-        __TRY
-        {
-            *pcFonts = num_fonts;
-        }
-        __EXCEPT_PAGE_FAULT
-        {
-            WARN("page fault while writing to *pcFonts (%p)\n", pcFonts);
-            RemoveFontMemResourceEx(ret);
-            ret = 0;
-        }
-        __ENDTRY
+        WARN( "page fault while writing to *pcFonts (%p)\n", pcFonts );
+        return NULL;
     }
-    return ret;
+    __ENDTRY
+
+    GetTempPathW( MAX_PATH, tmppath_w );
+    if (!AllocateLocallyUniqueId( &luid )) return NULL;
+    sprintfW( filename_w, tmpfmt_w, tmppath_w, luid.LowPart, luid.HighPart );
+    TRACE( "writing font to temporary %s\n", debugstr_w(filename_w) );
+
+    file = CreateFileW( filename_w, GENERIC_WRITE, FILE_SHARE_DELETE, NULL, CREATE_ALWAYS,
+                        FILE_ATTRIBUTE_TEMPORARY | FILE_FLAG_DELETE_ON_CLOSE, NULL );
+    if (file == INVALID_HANDLE_VALUE) return NULL;
+
+    if (WriteFile( file, pbFont, cbFont, &written, NULL ) && written == cbFont &&
+        GetFinalPathNameByHandleW( file, filename_w, MAX_PATH, FILE_NAME_NORMALIZED ))
+        *pcFonts = WineEngAddFontResourceEx( filename_w, FR_PRIVATE, NULL, TRUE );
+
+    if (*pcFonts == 0)
+    {
+        CloseHandle( file );
+        return NULL;
+    }
+
+    /* FIXME: is the handle only for use in RemoveFontMemResourceEx or should it be a true handle?
+     * For now return something unique but quite random
+     */
+    TRACE( "Returning handle %lx\n", ((INT_PTR)file) ^ 0x87654321 );
+    return (HANDLE)(((INT_PTR)file) ^ 0x87654321);
 }

/***********************************************************************
@@ -3831,7 +3852,16 @@ HANDLE WINAPI AddFontMemResourceEx( PVOID pbFont, DWORD cbFont, PVOID pdv, DWORD
  */
 BOOL WINAPI RemoveFontMemResourceEx( HANDLE fh )
 {
-    FIXME("(%p) stub\n", fh);
+    HANDLE file = (HANDLE)(((INT_PTR)fh) ^ 0x87654321);
+    WCHAR filename_w[MAX_PATH];
+
+    TRACE( "(%p)\n", fh );
+    if (!fh) return TRUE;
+
+    if (GetFinalPathNameByHandleW( file, filename_w, MAX_PATH, FILE_NAME_NORMALIZED ))
+        WineEngRemoveFontResourceEx( filename_w, FR_PRIVATE, NULL );
+
+    CloseHandle( file );
     return TRUE;
 }

diff --git a/dlls/gdi32/freetype.c b/dlls/gdi32/freetype.c
index cf53f148ad1..bdd8d97b16d 100644
--- a/dlls/gdi32/freetype.c
+++ b/dlls/gdi32/freetype.c
@@ -295,6 +295,7 @@ typedef struct tagFace {
 #define ADDFONT_ADD_TO_CACHE  0x04
 #define ADDFONT_ADD_RESOURCE  0x08  /* added through AddFontResource */
 #define ADDFONT_VERTICAL_FONT 0x10
+#define ADDFONT_ADD_MEMRESOURCE 0x20 /* added through AddFontMemResource */
 #define ADDFONT_AA_FLAGS(flags) ((flags) << 16)

typedef struct tagFamily {
@@ -1044,8 +1045,7 @@ static Face *find_face_from_filename(const WCHAR *file_name, const WCHAR *face_n
         face_list = get_face_list_from_family(family);
         LIST_FOR_EACH_ENTRY(face, face_list, Face, entry)
         {
-            if (!face->file)
-                continue;
+            if (face->flags & ADDFONT_ADD_MEMRESOURCE) continue;
             file = strrchrW(face->file, '/');
             if(!file)
                 file = face->file;
@@ -2592,8 +2592,7 @@ static void populate_system_links(const WCHAR *name, const WCHAR *const *values)
             face_list = get_face_list_from_family(family);
             LIST_FOR_EACH_ENTRY(face, face_list, Face, entry)
             {
-                if (!face->file)
-                    continue;
+                if (face->flags & ADDFONT_ADD_MEMRESOURCE) continue;
                 file = strrchrW(face->file, '/');
                 if (!file)
                     file = face->file;
@@ -3327,7 +3326,7 @@ static void delete_external_font_keys(void)
  *    WineEngAddFontResourceEx
  *
  */
-INT WineEngAddFontResourceEx(LPCWSTR file, DWORD flags, PVOID pdv)
+INT WineEngAddFontResourceEx( LPCWSTR file, DWORD flags, PVOID pdv, BOOL mem_resource )
 {
     WCHAR path[MAX_PATH];
     INT ret = 0;
@@ -3337,6 +3336,7 @@ INT WineEngAddFontResourceEx(LPCWSTR file, DWORD flags, PVOID pdv)
     if (ft_handle)  /* do it only if we have freetype up and running */
     {
         DWORD addfont_flags = ADDFONT_ALLOW_BITMAP | ADDFONT_ADD_RESOURCE;
+        if (mem_resource) addfont_flags |= ADDFONT_ADD_MEMRESOURCE;

EnterCriticalSection( &freetype_cs );

@@ -3346,12 +3346,12 @@ INT WineEngAddFontResourceEx(LPCWSTR file, DWORD flags, PVOID pdv)
         if (!ret && !strchrW(file, '\')) {
             /* Try in %WINDIR%/fonts, needed for Fotobuch Designer */
             get_winfonts_dir_path( file, path );
-            ret = add_font_resource( path, ADDFONT_ALLOW_BITMAP | ADDFONT_ADD_RESOURCE );
+            ret = add_font_resource( path, addfont_flags & ~ADDFONT_ADD_TO_CACHE );
             /* Try in datadir/fonts (or builddir/fonts), needed for Magic the Gathering Online */
             if (!ret)
             {
                 get_data_dir_path( file, path );
-                ret = add_font_resource( path, ADDFONT_ALLOW_BITMAP | ADDFONT_ADD_RESOURCE );
+                ret = add_font_resource( path, addfont_flags & ~ADDFONT_ADD_TO_CACHE );
             }
         }

@@ -3360,42 +3360,6 @@ INT WineEngAddFontResourceEx(LPCWSTR file, DWORD flags, PVOID pdv)
     return ret;
 }

-/*************************************************************
- *    WineEngAddFontMemResourceEx
- *
- */
-HANDLE WineEngAddFontMemResourceEx(PVOID pbFont, DWORD cbFont, PVOID pdv, DWORD *pcFonts)
-{
-    GDI_CheckNotLock();
-
-    if (ft_handle)  /* do it only if we have freetype up and running */
-    {
-        PVOID pFontCopy = HeapAlloc(GetProcessHeap(), 0, cbFont);
-
-        TRACE("Copying %d bytes of data from %p to %p\n", cbFont, pbFont, pFontCopy);
-        memcpy(pFontCopy, pbFont, cbFont);
-
-        EnterCriticalSection( &freetype_cs );
-        *pcFonts = AddFontToList(NULL, pFontCopy, cbFont, ADDFONT_ALLOW_BITMAP | ADDFONT_ADD_RESOURCE);
-        LeaveCriticalSection( &freetype_cs );
-
-        if (*pcFonts == 0)
-        {
-            TRACE("AddFontToList failed\n");
-            HeapFree(GetProcessHeap(), 0, pFontCopy);
-            return 0;
-        }
-        /* FIXME: is the handle only for use in RemoveFontMemResourceEx or should it be a true handle?
-         * For now return something unique but quite random
-         */
-        TRACE("Returning handle %lx\n", ((INT_PTR)pFontCopy)^0x87654321);
-        return (HANDLE)(((INT_PTR)pFontCopy)^0x87654321);
-    }
-
-    *pcFonts = 0;
-    return 0;
-}
-
 /*************************************************************
  *    WineEngRemoveFontResourceEx
  *
@@ -5519,25 +5483,19 @@ static const VOID * get_GSUB_vert_feature(const GdiFont *font)
 static void fill_fileinfo_from_face( GdiFont *font, Face *face )
 {
     WIN32_FILE_ATTRIBUTE_DATA info;
-    int len;
+    SIZE_T path_size;

-    if (!face->file)
-    {
-        font->fileinfo = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*font->fileinfo));
-        font->fileinfo->size.QuadPart = face->font_data_size;
-        return;
-    }
-
-    len = strlenW(face->file);
-    font->fileinfo = HeapAlloc(GetProcessHeap(), 0, sizeof(*font->fileinfo) + len * sizeof(WCHAR));
+    path_size = strlenW( face->file ) * sizeof(WCHAR);
+    font->fileinfo = HeapAlloc( GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*font->fileinfo) + path_size );
     if (GetFileAttributesExW(face->file, GetFileExInfoStandard, &info))
     {
         font->fileinfo->writetime = info.ftLastWriteTime;
         font->fileinfo->size.QuadPart = (LONGLONG)info.nFileSizeHigh << 32 | info.nFileSizeLow;
         strcpyW(font->fileinfo->path, face->file);
     }
-    else
-        memset(font->fileinfo, 0, sizeof(*font->fileinfo) + len * sizeof(WCHAR));
+
+    /* clear the path if the face was supposed to be loaded from memory */
+    if (face->flags & ADDFONT_ADD_MEMRESOURCE) memset( font->fileinfo->path, 0, path_size );
 }

/*************************************************************
@@ -9292,9 +9250,9 @@ BOOL WineEngInit(void)
     return FALSE;
 }

-INT WineEngAddFontResourceEx(LPCWSTR file, DWORD flags, PVOID pdv)
+INT WineEngAddFontResourceEx( LPCWSTR file, DWORD flags, PVOID pdv, BOOL mem_resource )
 {
-    FIXME("(%s, %x, %p): stub\n", debugstr_w(file), flags, pdv);
+    FIXME( "(%s, %x, %p, %d): stub\n", debugstr_w(file), flags, pdv, mem_resource );
     return 1;
 }

@@ -9304,12 +9262,6 @@ INT WineEngRemoveFontResourceEx(LPCWSTR file, DWORD flags, PVOID pdv)
     return TRUE;
 }

-HANDLE WineEngAddFontMemResourceEx(PVOID pbFont, DWORD cbFont, PVOID pdv, DWORD *pcFonts)
-{
-    FIXME("(%p, %u, %p, %p): stub\n", pbFont, cbFont, pdv, pcFonts);
-    return NULL;
-}
-
 BOOL WineEngCreateScalableFontResource( DWORD hidden, LPCWSTR resource,
                                         LPCWSTR font_file, LPCWSTR font_path )
 {
diff --git a/dlls/gdi32/gdi_private.h b/dlls/gdi32/gdi_private.h
index 6b2502c4586..51398653bd4 100644
--- a/dlls/gdi32/gdi_private.h
+++ b/dlls/gdi32/gdi_private.h
@@ -302,8 +302,7 @@ struct char_width_info
     INT unk;   /* unknown */
 };

-extern INT WineEngAddFontResourceEx(LPCWSTR, DWORD, PVOID) DECLSPEC_HIDDEN;
-extern HANDLE WineEngAddFontMemResourceEx(PVOID, DWORD, PVOID, LPDWORD) DECLSPEC_HIDDEN;
+extern INT WineEngAddFontResourceEx( LPCWSTR, DWORD, PVOID, BOOL ) DECLSPEC_HIDDEN;
 extern BOOL WineEngCreateScalableFontResource(DWORD, LPCWSTR, LPCWSTR, LPCWSTR) DECLSPEC_HIDDEN;
 extern BOOL WineEngInit(void) DECLSPEC_HIDDEN;
 extern BOOL WineEngRemoveFontResourceEx(LPCWSTR, DWORD, PVOID) DECLSPEC_HIDDEN;

[PATCH 2/8] gdi32: Remove loading face from memory support code.

[Rémi Bernon]

Signed-off-by: Rémi Bernon 
---
 dlls/gdi32/freetype.c | 112 ++++++++++++++----------------------------
 1 file changed, 38 insertions(+), 74 deletions(-)

diff --git a/dlls/gdi32/freetype.c b/dlls/gdi32/freetype.c
index bdd8d97b16d..6a9fad5b5dc 100644
--- a/dlls/gdi32/freetype.c
+++ b/dlls/gdi32/freetype.c
@@ -274,8 +274,6 @@ typedef struct tagFace {
     WCHAR *file;
     dev_t dev;
     ino_t ino;
-    void *font_data_ptr;
-    DWORD font_data_size;
     FT_Long face_index;
     FONTSIGNATURE fs;
     DWORD ntmFlags;
@@ -1581,7 +1579,7 @@ static BOOL insert_face_in_family_list( Face *face, Family *family )
                    debugstr_w(face->full_name), debugstr_w(family->family_name),
                    cursor->font_version, face->font_version );

-            if (face->file && face->dev == cursor->dev && face->ino == cursor->ino)
+            if (face->dev == cursor->dev && face->ino == cursor->ino)
             {
                 cursor->refcount++;
                 TRACE("Font %s already in list, refcount now %d\n",
@@ -2101,8 +2099,7 @@ static inline void get_fontsig( FT_Face ft_face, FONTSIGNATURE *fs )
     }
 }

-static Face *create_face( FT_Face ft_face, FT_Long face_index, const char *file, void *font_data_ptr, DWORD font_data_size,
-                          DWORD flags )
+static Face *create_face( FT_Face ft_face, FT_Long face_index, const char *file, DWORD flags )
 {
     struct stat st;
     Face *face = HeapAlloc( GetProcessHeap(), 0, sizeof(*face) );
@@ -2114,22 +2111,11 @@ static Face *create_face( FT_Face ft_face, FT_Long face_index, const char *file,

face->dev = 0;
     face->ino = 0;
-    if (file)
+    face->file = towstr( CP_UNIXCP, file );
+    if (!stat( file, &st ))
     {
-        face->file = towstr( CP_UNIXCP, file );
-        face->font_data_ptr = NULL;
-        face->font_data_size = 0;
-        if (!stat( file, &st ))
-        {
-            face->dev = st.st_dev;
-            face->ino = st.st_ino;
-        }
-    }
-    else
-    {
-        face->file = NULL;
-        face->font_data_ptr = font_data_ptr;
-        face->font_data_size = font_data_size;
+        face->dev = st.st_dev;
+        face->ino = st.st_ino;
     }

face->face_index = face_index;
@@ -2161,13 +2147,12 @@ static Face *create_face( FT_Face ft_face, FT_Long face_index, const char *file,
     return face;
 }

-static void AddFaceToList(FT_Face ft_face, const char *file, void *font_data_ptr, DWORD font_data_size,
-                          FT_Long face_index, DWORD flags )
+static void AddFaceToList( FT_Face ft_face, const char *file, FT_Long face_index, DWORD flags )
 {
     Face *face;
     Family *family;

-    face = create_face( ft_face, face_index, file, font_data_ptr, font_data_size, flags );
+    face = create_face( ft_face, face_index, file, flags );
     family = get_family( ft_face, flags & ADDFONT_VERTICAL_FONT );

if (insert_face_in_family_list( face, family ))
@@ -2180,34 +2165,25 @@ static void AddFaceToList(FT_Face ft_face, const char *file, void *font_data_ptr
     release_family( family );
 }

-static FT_Face new_ft_face( const char *file, void *font_data_ptr, DWORD font_data_size,
-                            FT_Long face_index, BOOL allow_bitmap )
+static FT_Face new_ft_face( const char *file, FT_Long face_index, BOOL allow_bitmap )
 {
     FT_Error err;
     TT_OS2 *pOS2;
     FT_Face ft_face;

-    if (file)
-    {
-        TRACE("Loading font file %s index %ld\n", debugstr_a(file), face_index);
-        err = pFT_New_Face(library, file, face_index, &ft_face);
-    }
-    else
-    {
-        TRACE("Loading font from ptr %p size %d, index %ld\n", font_data_ptr, font_data_size, face_index);
-        err = pFT_New_Memory_Face(library, font_data_ptr, font_data_size, face_index, &ft_face);
-    }
+    TRACE( "Loading font file %s index %ld\n", debugstr_a(file), face_index );
+    err = pFT_New_Face( library, file, face_index, &ft_face );

if (err != 0)
     {
-        WARN("Unable to load font %s/%p err = %x\n", debugstr_a(file), font_data_ptr, err);
+        WARN( "Unable to load font %s err = %x\n", debugstr_a(file), err );
         return NULL;
     }

/* There are too many bugs in FreeType < 2.1.9 for bitmap font support */
     if (!FT_IS_SCALABLE( ft_face ) && FT_SimpleVersion < FT_VERSION_VALUE(2, 1, 9))
     {
-        WARN("FreeType version < 2.1.9, skipping bitmap font %s/%p\n", debugstr_a(file), font_data_ptr);
+        WARN( "FreeType version < 2.1.9, skipping bitmap font %s\n", debugstr_a(file) );
         goto fail;
     }

@@ -2215,7 +2191,7 @@ static FT_Face new_ft_face( const char *file, void *font_data_ptr, DWORD font_da
     {
         if (FT_IS_SCALABLE( ft_face ) || !allow_bitmap )
         {
-            WARN("Ignoring font %s/%p\n", debugstr_a(file), font_data_ptr);
+            WARN( "Ignoring font %s\n", debugstr_a(file) );
             goto fail;
         }
     }
@@ -2225,8 +2201,7 @@ static FT_Face new_ft_face( const char *file, void *font_data_ptr, DWORD font_da
             !pFT_Get_Sfnt_Table( ft_face, ft_sfnt_hhea ) ||
             !pFT_Get_Sfnt_Table( ft_face, ft_sfnt_head ))
         {
-            TRACE("Font %s/%p lacks either an OS2, HHEA or HEAD table.\n"
-                  "Skipping this font.\n", debugstr_a(file), font_data_ptr);
+            TRACE( "Skipping font %s lacking OS2, HHEA and HEAD tables.\n", debugstr_a(file) );
             goto fail;
         }

@@ -2246,7 +2221,7 @@ static FT_Face new_ft_face( const char *file, void *font_data_ptr, DWORD font_da

if (!ft_face->family_name || !ft_face->style_name)
     {
-        TRACE("Font %s/%p lacks either a family or style name\n", debugstr_a(file), font_data_ptr);
+        TRACE( "Font %s lacks either a family or style name\n", debugstr_a(file) );
         goto fail;
     }

@@ -2256,7 +2231,7 @@ fail:
     return NULL;
 }

-static INT AddFontToList(const char *file, void *font_data_ptr, DWORD font_data_size, DWORD flags)
+static INT AddFontToList( const char *file, DWORD flags )
 {
     FT_Face ft_face;
     FT_Long face_index = 0, num_faces;
@@ -2276,7 +2251,7 @@ static INT AddFontToList(const char *file, void *font_data_ptr, DWORD font_data_
             for(cursor = mac_list; *cursor; cursor++)
             {
                 had_one = TRUE;
-                AddFontToList(*cursor, NULL, 0, flags);
+                AddFontToList( *cursor, flags );
                 HeapFree(GetProcessHeap(), 0, *cursor);
             }
             HeapFree(GetProcessHeap(), 0, mac_list);
@@ -2289,7 +2264,7 @@ static INT AddFontToList(const char *file, void *font_data_ptr, DWORD font_data_
     do {
         FONTSIGNATURE fs;

-        ft_face = new_ft_face( file, font_data_ptr, font_data_size, face_index, flags & ADDFONT_ALLOW_BITMAP );
+        ft_face = new_ft_face( file, face_index, flags & ADDFONT_ALLOW_BITMAP );
         if (!ft_face) return 0;

if(ft_face->family_name[0] == '.') /* Ignore fonts with names beginning with a dot */
@@ -2299,14 +2274,13 @@ static INT AddFontToList(const char *file, void *font_data_ptr, DWORD font_data_
             return 0;
         }

-        AddFaceToList(ft_face, file, font_data_ptr, font_data_size, face_index, flags);
+        AddFaceToList( ft_face, file, face_index, flags );
         ++ret;

get_fontsig(ft_face, &fs);
         if (fs.fsCsb[0] & FS_DBCS_MASK)
         {
-            AddFaceToList(ft_face, file, font_data_ptr, font_data_size, face_index,
-                          flags | ADDFONT_VERTICAL_FONT);
+            AddFaceToList( ft_face, file, face_index, flags | ADDFONT_VERTICAL_FONT );
             ++ret;
         }

@@ -2323,7 +2297,7 @@ static int add_font_resource( const WCHAR *file, DWORD flags )

if (unixname)
     {
-        ret = AddFontToList( unixname, NULL, 0, flags );
+        ret = AddFontToList( unixname, flags );
         HeapFree( GetProcessHeap(), 0, unixname );
     }
     return ret;
@@ -2344,7 +2318,6 @@ static int remove_font_resource( const WCHAR *file, DWORD flags )
         family->refcount++;
         LIST_FOR_EACH_ENTRY_SAFE( face, face_next, &family->faces, Face, entry )
         {
-            if (!face->file) continue;
             if (LOWORD(face->flags) != LOWORD(flags)) continue;
             if (st.st_dev == face->dev && st.st_ino == face->ino)
             {
@@ -2811,7 +2784,7 @@ static BOOL ReadFontDir(const char *dirname, BOOL external_fonts)
         {
             DWORD addfont_flags = ADDFONT_ADD_TO_CACHE;
             if(external_fonts) addfont_flags |= ADDFONT_EXTERNAL_FONT;
-            AddFontToList(path, NULL, 0, addfont_flags);
+            AddFontToList( path, addfont_flags );
         }
     }
     closedir(dir);
@@ -2946,8 +2919,7 @@ static void load_fontconfig_fonts(void)
         if(len < 4) continue;
         ext = &file[ len - 3 ];
         if(_strnicmp(ext, "pfa", -1) && _strnicmp(ext, "pfb", -1))
-            AddFontToList(file, NULL, 0,
-                          ADDFONT_EXTERNAL_FONT | ADDFONT_ADD_TO_CACHE | ADDFONT_AA_FLAGS(aa_flags) );
+            AddFontToList( file, ADDFONT_EXTERNAL_FONT | ADDFONT_ADD_TO_CACHE | ADDFONT_AA_FLAGS( aa_flags ) );
     }
     pFcFontSetDestroy(fontset);
     pFcPatternDestroy(pat);
@@ -2966,7 +2938,7 @@ static void load_mac_font_callback(const void *value, void *context)
     if (path && CFStringGetFileSystemRepresentation(pathStr, path, len))
     {
         TRACE("font file %s\n", path);
-        AddFontToList(path, NULL, 0, ADDFONT_EXTERNAL_FONT | ADDFONT_ADD_TO_CACHE);
+        AddFontToList( path, ADDFONT_EXTERNAL_FONT | ADDFONT_ADD_TO_CACHE );
     }
     HeapFree(GetProcessHeap(), 0, path);
 }
@@ -3472,7 +3444,7 @@ static void GetEnumStructs(Face *face, const WCHAR *family_name, LPENUMLOGFONTEX

static BOOL get_fontdir( const char *unix_name, struct fontdir *fd )
 {
-    FT_Face ft_face = new_ft_face( unix_name, NULL, 0, 0, FALSE );
+    FT_Face ft_face = new_ft_face( unix_name, 0, FALSE );
     Face *face;
     WCHAR *family_name;
     ENUMLOGFONTEXW elf;
@@ -3480,7 +3452,7 @@ static BOOL get_fontdir( const char *unix_name, struct fontdir *fd )
     DWORD type;

if (!ft_face) return FALSE;
-    face = create_face( ft_face, 0, unix_name, NULL, 0, 0 );
+    face = create_face( ft_face, 0, unix_name, 0 );
     family_name = ft_face_get_family_name( ft_face, GetSystemDefaultLCID() );
     pFT_Done_Face( ft_face );

@@ -4553,28 +4525,21 @@ static FT_Face OpenFontFace(GdiFont *font, Face *face, LONG width, LONG height)
     FT_Error err;
     FT_Face ft_face;
     void *data_ptr;
+    char *filename;
     DWORD data_size;

-    TRACE("%s/%p, %ld, %d x %d\n", debugstr_w(face->file), face->font_data_ptr, face->face_index, width, height);
+    TRACE( "%s, %d x %d\n", debugstr_w(face->file), width, height );

-    if (face->file)
-    {
-        char *filename = strWtoA( CP_UNIXCP, face->file );
-        font->mapping = map_font_file( filename );
-        HeapFree( GetProcessHeap(), 0, filename );
-        if (!font->mapping)
-        {
-            WARN("failed to map %s\n", debugstr_w(face->file));
-            return 0;
-        }
-        data_ptr = font->mapping->data;
-        data_size = font->mapping->size;
-    }
-    else
+    filename = strWtoA( CP_UNIXCP, face->file );
+    font->mapping = map_font_file( filename );
+    HeapFree( GetProcessHeap(), 0, filename );
+    if (!font->mapping)
     {
-        data_ptr = face->font_data_ptr;
-        data_size = face->font_data_size;
+        WARN( "failed to map %s\n", debugstr_w(face->file) );
+        return 0;
     }
+    data_ptr = font->mapping->data;
+    data_size = font->mapping->size;

err = pFT_New_Memory_Face(library, data_ptr, data_size, face->face_index, &ft_face);
     if(err) {
@@ -5844,8 +5809,7 @@ found_face:
     else
         ret->charset = get_nearest_charset( family->family_name, face, &ret->codepage );

-    TRACE( "Chosen: %s (%s/%p:%ld)\n", debugstr_w(face->full_name), debugstr_w(face->file),
-           face->font_data_ptr, face->face_index );
+    TRACE( "Chosen: %s (%s:%ld)\n", debugstr_w(face->full_name), debugstr_w(face->file), face->face_index );

ret->aveWidth = height ? lf.lfWidth : 0;

[PATCH 3/8] gdi32: Cleanup face and family creation and refcounting.

[Rémi Bernon]

Signed-off-by: Rémi Bernon 
---

The interlocked inc/dec aren't really required, and not really needed
as we hold a global mutex.

I also believe the face refcount isn't really useful for system fonts,
as we are never removing them or releasing their memory anyway.

This is mostly to be sure everything goes through the helper functions
and gets traced.

dlls/gdi32/freetype.c | 217 ++++++++++++++++++++++++------------------
 1 file changed, 124 insertions(+), 93 deletions(-)

diff --git a/dlls/gdi32/freetype.c b/dlls/gdi32/freetype.c
index 6a9fad5b5dc..f3f66e223e4 100644
--- a/dlls/gdi32/freetype.c
+++ b/dlls/gdi32/freetype.c
@@ -268,7 +268,7 @@ struct enum_data

typedef struct tagFace {
     struct list entry;
-    unsigned int refcount;
+    LONG ref;
     WCHAR *style_name;
     WCHAR *full_name;
     WCHAR *file;
@@ -298,7 +298,7 @@ typedef struct tagFace {

typedef struct tagFamily {
     struct list entry;
-    unsigned int refcount;
+    LONG ref;
     WCHAR family_name[LF_FACESIZE];
     WCHAR english_name[LF_FACESIZE];
     struct list faces;
@@ -619,6 +619,14 @@ static const WCHAR font_mutex_nameW[] = {'_','_','W','I','N','E','_','F','O','N'
 static const WCHAR szDefaultFallbackLink[] = {'M','i','c','r','o','s','o','f','t',' ','S','a','n','s',' ','S','e','r','i','f',0};
 static BOOL use_default_fallback = FALSE;

+static Family *family_create( const WCHAR *family_name, const WCHAR *english_name );
+static ULONG family_addref( Family *family );
+static ULONG family_release( Family *family );
+
+static Face *face_create( void );
+static ULONG face_addref( Face *face );
+static ULONG face_release( Face *face );
+
 static BOOL map_font_family(const WCHAR *orig, const WCHAR *repl);
 static BOOL get_glyph_index_linked(GdiFont *font, UINT c, GdiFont **linked_font, FT_UInt *glyph, BOOL *vert);
 static BOOL get_outline_text_metrics(GdiFont *font);
@@ -1050,7 +1058,7 @@ static Face *find_face_from_filename(const WCHAR *file_name, const WCHAR *face_n
             else
                 file++;
             if(strcmpiW(file, file_name)) continue;
-            face->refcount++;
+            face_addref( face );
             return face;
    }
     }
@@ -1525,28 +1533,87 @@ static inline BOOL faces_equal( const Face *f1, const Face *f2 )
     return !memcmp( &f1->fs, &f2->fs, sizeof(f1->fs) );
 }

-static void release_family( Family *family )
+static Family *family_create( const WCHAR *family_name, const WCHAR *english_name )
+{
+    Family *family;
+
+    if (!(family = HeapAlloc( GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*family) ))) return NULL;
+    family->ref = 1;
+
+    TRACE( "family %p, family_name %s, english_name %s\n", family, debugstr_w(family_name),
+           debugstr_w(english_name) );
+
+    lstrcpynW( family->family_name, family_name, LF_FACESIZE );
+    if (english_name) lstrcpynW( family->english_name, english_name, LF_FACESIZE );
+
+    list_init( &family->entry );
+    list_init( &family->faces );
+
+    family->replacement = &family->faces;
+    list_add_tail( &font_list, &family->entry );
+
+    return family;
+}
+
+static ULONG family_addref( Family *family )
 {
-    if (--family->refcount) return;
+    ULONG ref = InterlockedIncrement( &family->ref );
+    TRACE( "family %p, ref %d\n", family, ref );
+    return ref;
+}
+
+static ULONG family_release( Family *family )
+{
+    ULONG ref = InterlockedDecrement( &family->ref );
+    TRACE( "family %p, ref %d\n", family, ref );
+    if (ref) return ref;
+
     assert( list_empty( &family->faces ));
     list_remove( &family->entry );
     HeapFree( GetProcessHeap(), 0, family );
+    return ref;
 }

-static void release_face( Face *face )
+static Face *face_create( void )
 {
-    if (--face->refcount) return;
+    Face *face;
+
+    if (!(face = HeapAlloc( GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*face) ))) return NULL;
+    face->ref = 1;
+
+    TRACE( "face %p\n", face );
+
+    list_init( &face->entry );
+
+    return face;
+}
+
+static ULONG face_addref( Face *face )
+{
+    ULONG ref = InterlockedIncrement( &face->ref );
+    TRACE( "face %p, ref %d\n", face, ref );
+    return ref;
+}
+
+static ULONG face_release( Face *face )
+{
+    ULONG ref = InterlockedDecrement( &face->ref );
+    TRACE( "face %p, ref %d\n", face, ref );
+    if (ref) return ref;
+
     if (face->family)
     {
         if (face->flags & ADDFONT_ADD_TO_CACHE) remove_face_from_cache( face );
         list_remove( &face->entry );
-        release_family( face->family );
+        family_release( face->family );
     }
+
     HeapFree( GetProcessHeap(), 0, face->file );
     HeapFree( GetProcessHeap(), 0, face->style_name );
     HeapFree( GetProcessHeap(), 0, face->full_name );
     HeapFree( GetProcessHeap(), 0, face->cached_enum_data );
     HeapFree( GetProcessHeap(), 0, face );
+    return ref;
 }

static inline int style_order(const Face *face)
@@ -1581,9 +1648,8 @@ static BOOL insert_face_in_family_list( Face *face, Family *family )

if (face->dev == cursor->dev && face->ino == cursor->ino)
             {
-                cursor->refcount++;
-                TRACE("Font %s already in list, refcount now %d\n",
-                      debugstr_w(face->file), cursor->refcount);
+                TRACE( "Font %s already in list\n", debugstr_w(face->file) );
+                face_addref( cursor );
                 return FALSE;
             }
             if (face->font_version <= cursor->font_version)
@@ -1598,9 +1664,9 @@ static BOOL insert_face_in_family_list( Face *face, Family *family )
                       debugstr_w(cursor->file), debugstr_w(face->file));
                 list_add_before( &cursor->entry, &face->entry );
                 face->family = family;
-                family->refcount++;
-                face->refcount++;
-                release_face( cursor );
+                family_addref( family );
+                face_addref( face );
+                face_release( cursor );
                 return TRUE;
             }
         }
@@ -1612,29 +1678,11 @@ static BOOL insert_face_in_family_list( Face *face, Family *family )
            debugstr_w(family->family_name), debugstr_w(face->file) );
     list_add_before( &cursor->entry, &face->entry );
     face->family = family;
-    family->refcount++;
-    face->refcount++;
+    family_addref( family );
+    face_addref( face );
     return TRUE;
 }

-/****************************************************************
- * NB This function stores the ptrs to the strings to save copying.
- * Don't free them after calling.
- */
-static Family *create_family( WCHAR *family_name, WCHAR *english_name )
-{
-    Family * const family = HeapAlloc( GetProcessHeap(), 0, sizeof(*family) );
-    family->refcount = 1;
-    lstrcpynW( family->family_name, family_name, LF_FACESIZE );
-    if (english_name) lstrcpynW( family->english_name, english_name, LF_FACESIZE );
-    else family->english_name[0] = 0;
-    list_init( &family->faces );
-    family->replacement = &family->faces;
-    list_add_tail( &font_list, &family->entry );
-
-    return family;
-}
-
 static LONG reg_load_dword(HKEY hkey, const WCHAR *value, DWORD *data)
 {
     DWORD type, size = sizeof(DWORD);
@@ -1673,18 +1721,14 @@ static void load_face(HKEY hkey_face, WCHAR *face_name, Family *family, void *bu
 {
     DWORD needed, strike_index = 0;
     HKEY hkey_strike;
+    Face *face;

/* If we have a File Name key then this is a real font, not just the parent
        key of a bunch of non-scalable strikes */
     needed = buffer_size;
-    if (RegQueryValueExW(hkey_face, face_file_name_value, NULL, NULL, buffer, &needed) == ERROR_SUCCESS)
+    if (RegQueryValueExW( hkey_face, face_file_name_value, NULL, NULL, buffer, &needed ) == ERROR_SUCCESS &&
+        (face = face_create()))
     {
-        Face *face;
-        face = HeapAlloc(GetProcessHeap(), 0, sizeof(*face));
-        face->cached_enum_data = NULL;
-        face->family = NULL;
-
-        face->refcount = 1;
         face->file = strdupW( buffer );
         face->style_name = strdupW( face_name );

@@ -1693,7 +1737,7 @@ static void load_face(HKEY hkey_face, WCHAR *face_name, Family *family, void *bu
         {
             ERR( "couldn't find full name for %s %s in cache\n", debugstr_w(family->family_name),
                  debugstr_w(face->style_name) );
-            release_face( face );
+            face_release( face );
             return;
         }
         face->full_name = strdupW( buffer );
@@ -1707,10 +1751,7 @@ static void load_face(HKEY hkey_face, WCHAR *face_name, Family *family, void *bu
         RegQueryValueExW(hkey_face, face_font_sig_value, NULL, NULL, (BYTE*)&face->fs, &needed);

if(reg_load_ftshort(hkey_face, face_height_value, &face->size.height) != ERROR_SUCCESS)
-        {
             face->scalable = TRUE;
-            memset(&face->size, 0, sizeof(face->size));
-        }
         else
         {
             face->scalable = FALSE;
@@ -1733,7 +1774,7 @@ static void load_face(HKEY hkey_face, WCHAR *face_name, Family *family, void *bu
         if (insert_face_in_family_list(face, family))
             TRACE( "Added face %s to family %s\n", debugstr_w(face->full_name), debugstr_w(family->family_name) );

-        release_face( face );
+        face_release( face );
     }

/* load bitmap strikes */
@@ -1802,7 +1843,7 @@ static void load_font_list_from_cache(HKEY hkey_font_cache)
         if (!RegQueryValueExW(hkey_family, english_name_value, NULL, NULL, (BYTE *)buffer, &size))
             english_name = strdupW( buffer );

-        family = create_family( family_name, english_name );
+        family = family_create( family_name, english_name );

if (english_name)
         {
@@ -1833,7 +1874,7 @@ static void load_font_list_from_cache(HKEY hkey_font_cache)
         HeapFree( GetProcessHeap(), 0, english_name );

RegCloseKey(hkey_family);
-        release_family( family );
+        family_release( family );
         size = sizeof(buffer);
     }

@@ -1962,8 +2003,8 @@ static Family *get_family( FT_Face ft_face, BOOL vertical )
         english_name = get_vertical_name( english_name );
     }

-    if ((family = find_family_from_name( family_name ))) family->refcount++;
-    else if ((family = create_family( family_name, english_name )) && english_name)
+    if ((family = find_family_from_name( family_name ))) family_addref( family );
+    else if ((family = family_create( family_name, english_name )) && english_name)
     {
         FontSubst *subst = HeapAlloc( GetProcessHeap(), 0, sizeof(*subst) );
         subst->from.name = strdupW( english_name );
@@ -2102,9 +2143,10 @@ static inline void get_fontsig( FT_Face ft_face, FONTSIGNATURE *fs )
 static Face *create_face( FT_Face ft_face, FT_Long face_index, const char *file, DWORD flags )
 {
     struct stat st;
-    Face *face = HeapAlloc( GetProcessHeap(), 0, sizeof(*face) );
+    Face *face;
+
+    if (!(face = face_create())) return NULL;

-    face->refcount = 1;
     face->style_name = ft_face_get_style_name( ft_face, GetSystemDefaultLangID() );
     face->full_name = ft_face_get_full_name( ft_face, GetSystemDefaultLangID() );
     if (flags & ADDFONT_VERTICAL_FONT) face->full_name = get_vertical_name( face->full_name );
@@ -2123,21 +2165,10 @@ static Face *create_face( FT_Face ft_face, FT_Long face_index, const char *file,
     face->ntmFlags = get_ntm_flags( ft_face );
     face->font_version = get_font_version( ft_face );

-    if (FT_IS_SCALABLE( ft_face ))
-    {
-        memset( &face->size, 0, sizeof(face->size) );
-        face->scalable = TRUE;
-    }
-    else
-    {
-        get_bitmap_size( ft_face, &face->size );
-        face->scalable = FALSE;
-    }
+    if (!(face->scalable = FT_IS_SCALABLE( ft_face ))) get_bitmap_size( ft_face, &face->size );

if (!HIWORD( flags )) flags |= ADDFONT_AA_FLAGS( default_aa_flags );
     face->flags  = flags;
-    face->family = NULL;
-    face->cached_enum_data = NULL;

TRACE("fsCsb = %08x %08x/%08x %08x %08x %08x\n",
           face->fs.fsCsb[0], face->fs.fsCsb[1],
@@ -2161,8 +2192,9 @@ static void AddFaceToList( FT_Face ft_face, const char *file, FT_Long face_index
             add_face_to_cache( face );
         TRACE( "Added face %s to family %s\n", debugstr_w(face->full_name), debugstr_w(family->family_name) );
     }
-    release_face( face );
-    release_family( family );
+
+    face_release( face );
+    family_release( family );
 }

static FT_Face new_ft_face( const char *file, FT_Long face_index, BOOL allow_bitmap )
@@ -2315,18 +2347,18 @@ static int remove_font_resource( const WCHAR *file, DWORD flags )
     if (stat( unixname, &st ) == -1) goto done;
     LIST_FOR_EACH_ENTRY_SAFE( family, family_next, &font_list, Family, entry )
     {
-        family->refcount++;
+        family_addref( family );
         LIST_FOR_EACH_ENTRY_SAFE( face, face_next, &family->faces, Face, entry )
         {
             if (LOWORD(face->flags) != LOWORD(flags)) continue;
             if (st.st_dev == face->dev && st.st_ino == face->ino)
             {
-                TRACE( "removing matching face %s refcount %d\n", debugstr_w(face->file), face->refcount );
-                release_face( face );
+                TRACE( "removing matching face %s\n", debugstr_w(face->file) );
+                face_release( face );
                 count++;
             }
    }
-        release_family( family );
+        family_release( family );
     }
 done:
     HeapFree( GetProcessHeap(), 0, unixname );
@@ -2376,27 +2408,26 @@ static BOOL map_vertical_font_family(const WCHAR *orig, const WCHAR *repl, const

static BOOL map_font_family(const WCHAR *orig, const WCHAR *repl)
 {
-    Family *family = find_family_from_any_name(repl);
-    if (family != NULL)
-    {
-        Family *new_family = HeapAlloc(GetProcessHeap(), 0, sizeof(*new_family));
-        if (new_family != NULL)
-        {
-            TRACE("mapping %s to %s\n", debugstr_w(repl), debugstr_w(orig));
-            lstrcpynW( new_family->family_name, orig, LF_FACESIZE );
-            new_family->english_name[0] = 0;
-            list_init(&new_family->faces);
-            new_family->replacement = &family->faces;
-            list_add_tail(&font_list, &new_family->entry);
+    Family *family, *new_family;

-            if (repl[0] != '@')
-                map_vertical_font_family(orig, repl, family);
+    if (!(family = find_family_from_any_name( repl )))
+    {
+        TRACE( "%s is not available. Skip this replacement.\n", debugstr_w(repl) );
+        return FALSE;
+    }

-            return TRUE;
-        }
+    if (!(new_family = family_create( orig, NULL )))
+    {
+        WARN( "failed to allocate new family\n" );
+        return FALSE;
     }
-    TRACE("%s is not available. Skip this replacement.\n", debugstr_w(repl));
-    return FALSE;
+
+    TRACE( "mapping %s to %s\n", debugstr_w(repl), debugstr_w(orig) );
+
+    new_family->replacement = &family->faces;
+    if (repl[0] != '@') map_vertical_font_family( orig, repl, family );
+
+    return TRUE;
 }

/***********************************************************
@@ -2741,7 +2772,7 @@ skip_internal:
             new_child = HeapAlloc(GetProcessHeap(), 0, sizeof(*new_child));
             new_child->face = font_link_entry->face;
             new_child->font = NULL;
-            new_child->face->refcount++;
+            face_addref( new_child->face );
             system_font_link->fs.fsCsb[0] |= font_link_entry->face->fs.fsCsb[0];
             system_font_link->fs.fsCsb[1] |= font_link_entry->face->fs.fsCsb[1];
             list_add_tail(&system_font_link->links, &new_child->entry);
@@ -3457,7 +3488,7 @@ static BOOL get_fontdir( const char *unix_name, struct fontdir *fd )
     pFT_Done_Face( ft_face );

GetEnumStructs( face, family_name, &elf, &ntm, &type );
-    release_face( face );
+    face_release( face );
     HeapFree( GetProcessHeap(), 0, family_name );

if ((type & TRUETYPE_FONTTYPE) == 0) return FALSE;
@@ -4653,7 +4684,7 @@ static void free_font(GdiFont *font)
         list_remove(&child->entry);
         if(child->font)
             free_font(child->font);
-        release_face( child->face );
+        face_release( child->face );
         HeapFree(GetProcessHeap(), 0, child);
     }

@@ -5053,7 +5084,7 @@ static BOOL create_child_font_list(GdiFont *font)
             new_child = HeapAlloc(GetProcessHeap(), 0, sizeof(*new_child));
             new_child->face = font_link_entry->face;
             new_child->font = NULL;
-            new_child->face->refcount++;
+            face_addref( new_child->face );
             list_add_tail(&font->child_fonts, &new_child->entry);
             TRACE("font %s %ld\n", debugstr_w(new_child->face->file), new_child->face->face_index);
         }
@@ -5076,7 +5107,7 @@ static BOOL create_child_font_list(GdiFont *font)
                 new_child = HeapAlloc(GetProcessHeap(), 0, sizeof(*new_child));
                 new_child->face = font_link_entry->face;
                 new_child->font = NULL;
-                new_child->face->refcount++;
+                face_addref( new_child->face );
                 list_add_tail(&font->child_fonts, &new_child->entry);
                 TRACE("font %s %ld\n", debugstr_w(new_child->face->file), new_child->face->face_index);
             }

[PATCH 4/8] gdi32: Create face from unix_name, face_index and flags.

[Rémi Bernon]

And use unix_name + face_index as identifier instead of dev/ino.

Signed-off-by: Rémi Bernon 
---

This will make it possible to use FreeType cache later, using Face* as
FTC_FaceID, and load FT_Face from them when needed.

dlls/gdi32/freetype.c | 151 ++++++++++++++++++------------------------
 1 file changed, 64 insertions(+), 87 deletions(-)

diff --git a/dlls/gdi32/freetype.c b/dlls/gdi32/freetype.c
index f3f66e223e4..d0b076ce6fd 100644
--- a/dlls/gdi32/freetype.c
+++ b/dlls/gdi32/freetype.c
@@ -269,23 +269,27 @@ struct enum_data
 typedef struct tagFace {
     struct list entry;
     LONG ref;
+    const char *unix_name;
+    FT_Long face_index;
+    DWORD flags;
     WCHAR *style_name;
     WCHAR *full_name;
     WCHAR *file;
-    dev_t dev;
-    ino_t ino;
-    FT_Long face_index;
     FONTSIGNATURE fs;
     DWORD ntmFlags;
     FT_Fixed font_version;
     BOOL scalable;
     Bitmap_Size size;     /* set if face is a bitmap */
-    DWORD flags;          /* ADDFONT flags */
     struct tagFamily *family;
     /* Cached data for Enum */
     struct enum_data *cached_enum_data;
 } Face;

+static inline const char *debugstr_face( Face *face )
+{
+    return wine_dbg_sprintf( "%s:%ld", debugstr_a(face->unix_name), face->face_index );
+}
+
 #define FS_DBCS_MASK (FS_JISJAPAN|FS_CHINESESIMP|FS_WANSUNG|FS_CHINESETRAD|FS_JOHAB)

#define ADDFONT_EXTERNAL_FONT 0x01
@@ -585,7 +589,7 @@ static const WCHAR face_y_ppem_value[] = {'Y','p','p','e','m',0};
 static const WCHAR face_flags_value[] = {'F','l','a','g','s',0};
 static const WCHAR face_internal_leading_value[] = {'I','n','t','e','r','n','a','l',' ','L','e','a','d','i','n','g',0};
 static const WCHAR face_font_sig_value[] = {'F','o','n','t',' ','S','i','g','n','a','t','u','r','e',0};
-static const WCHAR face_file_name_value[] = {'F','i','l','e',' ','N','a','m','e','\0'};
+static const WCHAR face_unix_name_value[] = {'U','n','i','x',' ','N','a','m','e','\0'};
 static const WCHAR face_full_name_value[] = {'F','u','l','l',' ','N','a','m','e','\0'};

@@ -593,8 +597,7 @@ struct font_mapping
 {
     struct list entry;
     int         refcount;
-    dev_t       dev;
-    ino_t       ino;
+    char       *unix_name;
     void       *data;
     size_t      size;
 };
@@ -623,7 +626,7 @@ static Family *family_create( const WCHAR *family_name, const WCHAR *english_nam
 static ULONG family_addref( Family *family );
 static ULONG family_release( Family *family );

-static Face *face_create( void );
+static Face *face_create( const char *unix_name, DWORD face_index, DWORD flags );
 static ULONG face_addref( Face *face );
 static ULONG face_release( Face *face );

@@ -1177,14 +1180,6 @@ static WCHAR *towstr(UINT cp, const char *str)
     return wstr;
 }

-static char *strWtoA(UINT cp, const WCHAR *str)
-{
-    int len = WideCharToMultiByte( cp, 0, str, -1, NULL, 0, NULL, NULL );
-    char *ret = HeapAlloc( GetProcessHeap(), 0, len );
-    WideCharToMultiByte( cp, 0, str, -1, ret, len, NULL, NULL );
-    return ret;
-}
-
 static void split_subst_info(NameCs *nc, LPSTR str)
 {
     CHAR *p = strrchr(str, ',');
@@ -1574,17 +1569,26 @@ static ULONG family_release( Family *family )
     return ref;
 }

-static Face *face_create( void )
+static Face *face_create( const char *unix_name, DWORD face_index, DWORD flags )
 {
     Face *face;

-    if (!(face = HeapAlloc( GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*face) ))) return NULL;
+    if (!(face = HeapAlloc( GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*face) + strlen( unix_name ) + 1 )))
+        return NULL;
     face->ref = 1;

-    TRACE( "face %p\n", face );
+    TRACE( "face %p, unix_name %s, face_index %u, flags %#x\n", face, unix_name, face_index, flags );

list_init( &face->entry );

+    face->unix_name = (char *)(face + 1);
+    face->face_index = face_index;
+    face->flags = flags;
+    strcpy( (char *)face->unix_name, unix_name );
+
+    face->file = towstr( CP_UNIXCP, face->unix_name );
+    if (!HIWORD( face->flags )) face->flags |= ADDFONT_AA_FLAGS( default_aa_flags );
+
     return face;
 }

@@ -1646,22 +1650,20 @@ static BOOL insert_face_in_family_list( Face *face, Family *family )
                    debugstr_w(face->full_name), debugstr_w(family->family_name),
                    cursor->font_version, face->font_version );

-            if (face->dev == cursor->dev && face->ino == cursor->ino)
+            if (face->face_index == cursor->face_index && !strcmp( face->unix_name, cursor->unix_name ))
             {
-                TRACE( "Font %s already in list\n", debugstr_w(face->file) );
+                TRACE( "face %s already in list\n", debugstr_face(face) );
                 face_addref( cursor );
                 return FALSE;
             }
             if (face->font_version <= cursor->font_version)
             {
-                TRACE("Original font %s is newer so skipping %s\n",
-                      debugstr_w(cursor->file), debugstr_w(face->file));
+                TRACE( "Original %s is newer so skipping %s\n", debugstr_face(cursor), debugstr_face(face) );
                 return FALSE;
             }
             else
             {
-                TRACE("Replacing original %s with %s\n",
-                      debugstr_w(cursor->file), debugstr_w(face->file));
+                TRACE( "Replacing original %s with %s\n", debugstr_face(cursor), debugstr_face(face) );
                 list_add_before( &cursor->entry, &face->entry );
                 face->family = family;
                 family_addref( family );
@@ -1675,7 +1677,7 @@ static BOOL insert_face_in_family_list( Face *face, Family *family )
     }

TRACE( "Adding face %s in family %s from %s\n", debugstr_w(face->full_name),
-           debugstr_w(family->family_name), debugstr_w(face->file) );
+           debugstr_w(family->family_name), debugstr_face(face) );
     list_add_before( &cursor->entry, &face->entry );
     face->family = family;
     family_addref( family );
@@ -1719,17 +1721,18 @@ static inline LONG reg_save_dword(HKEY hkey, const WCHAR *value, DWORD data)

static void load_face(HKEY hkey_face, WCHAR *face_name, Family *family, void *buffer, DWORD buffer_size)
 {
-    DWORD needed, strike_index = 0;
+    DWORD face_index, flags, needed, strike_index = 0;
     HKEY hkey_strike;
     Face *face;

/* If we have a File Name key then this is a real font, not just the parent
        key of a bunch of non-scalable strikes */
     needed = buffer_size;
-    if (RegQueryValueExW( hkey_face, face_file_name_value, NULL, NULL, buffer, &needed ) == ERROR_SUCCESS &&
-        (face = face_create()))
+    if (RegQueryValueExW( hkey_face, face_unix_name_value, NULL, NULL, buffer, &needed ) == ERROR_SUCCESS &&
+        reg_load_dword( hkey_face, face_index_value, &face_index ) == ERROR_SUCCESS &&
+        reg_load_dword( hkey_face, face_flags_value, &flags ) == ERROR_SUCCESS &&
+        (face = face_create( buffer, face_index, flags )))
     {
-        face->file = strdupW( buffer );
         face->style_name = strdupW( face_name );

needed = buffer_size;
@@ -1742,10 +1745,8 @@ static void load_face(HKEY hkey_face, WCHAR *face_name, Family *family, void *bu
         }
         face->full_name = strdupW( buffer );

-        reg_load_ftlong(hkey_face, face_index_value, &face->face_index);
         reg_load_dword(hkey_face, face_ntmflags_value, &face->ntmFlags);
         reg_load_ftlong(hkey_face, face_version_value, &face->font_version);
-        reg_load_dword(hkey_face, face_flags_value, &face->flags);

needed = sizeof(face->fs);
         RegQueryValueExW(hkey_face, face_font_sig_value, NULL, NULL, (BYTE*)&face->fs, &needed);
@@ -1924,15 +1925,16 @@ static void add_face_to_cache(Face *face)
     if(!face->scalable)
         HeapFree(GetProcessHeap(), 0, face_key_name);

-    RegSetValueExW(hkey_face, face_file_name_value, 0, REG_SZ, (BYTE *)face->file,
-                   (strlenW(face->file) + 1) * sizeof(WCHAR));
+    RegSetValueExW( hkey_face, face_unix_name_value, 0, REG_BINARY, (BYTE *)face->unix_name,
+                    strlen( face->unix_name ) + 1 );
+    reg_save_dword( hkey_face, face_index_value, face->face_index );
+    reg_save_dword( hkey_face, face_flags_value, face->flags );
+
     RegSetValueExW( hkey_face, face_full_name_value, 0, REG_SZ, (BYTE *)face->full_name,
                     (strlenW( face->full_name ) + 1) * sizeof(WCHAR) );

-    reg_save_dword(hkey_face, face_index_value, face->face_index);
     reg_save_dword(hkey_face, face_ntmflags_value, face->ntmFlags);
     reg_save_dword(hkey_face, face_version_value, face->font_version);
-    if (face->flags) reg_save_dword(hkey_face, face_flags_value, face->flags);

RegSetValueExW(hkey_face, face_font_sig_value, 0, REG_BINARY, (BYTE*)&face->fs, sizeof(face->fs));

@@ -2142,34 +2144,20 @@ static inline void get_fontsig( FT_Face ft_face, FONTSIGNATURE *fs )

static Face *create_face( FT_Face ft_face, FT_Long face_index, const char *file, DWORD flags )
 {
-    struct stat st;
     Face *face;

-    if (!(face = face_create())) return NULL;
+    if (!(face = face_create( file, face_index, flags ))) return NULL;

face->style_name = ft_face_get_style_name( ft_face, GetSystemDefaultLangID() );
     face->full_name = ft_face_get_full_name( ft_face, GetSystemDefaultLangID() );
-    if (flags & ADDFONT_VERTICAL_FONT) face->full_name = get_vertical_name( face->full_name );
-
-    face->dev = 0;
-    face->ino = 0;
-    face->file = towstr( CP_UNIXCP, file );
-    if (!stat( file, &st ))
-    {
-        face->dev = st.st_dev;
-        face->ino = st.st_ino;
-    }
+    if (face->flags & ADDFONT_VERTICAL_FONT) face->full_name = get_vertical_name( face->full_name );

-    face->face_index = face_index;
     get_fontsig( ft_face, &face->fs );
     face->ntmFlags = get_ntm_flags( ft_face );
     face->font_version = get_font_version( ft_face );

if (!(face->scalable = FT_IS_SCALABLE( ft_face ))) get_bitmap_size( ft_face, &face->size );

-    if (!HIWORD( flags )) flags |= ADDFONT_AA_FLAGS( default_aa_flags );
-    face->flags  = flags;
-
     TRACE("fsCsb = %08x %08x/%08x %08x %08x %08x\n",
           face->fs.fsCsb[0], face->fs.fsCsb[1],
           face->fs.fsUsb[0], face->fs.fsUsb[1],
@@ -2339,28 +2327,26 @@ static int remove_font_resource( const WCHAR *file, DWORD flags )
 {
     Family *family, *family_next;
     Face *face, *face_next;
-    struct stat st;
     int count = 0;
     char *unixname;

if (!(unixname = wine_get_unix_file_name( file ))) return 0;
-    if (stat( unixname, &st ) == -1) goto done;
     LIST_FOR_EACH_ENTRY_SAFE( family, family_next, &font_list, Family, entry )
     {
         family_addref( family );
         LIST_FOR_EACH_ENTRY_SAFE( face, face_next, &family->faces, Face, entry )
         {
             if (LOWORD(face->flags) != LOWORD(flags)) continue;
-            if (st.st_dev == face->dev && st.st_ino == face->ino)
+            if (!strcmp( unixname, face->unix_name ))
             {
-                TRACE( "removing matching face %s\n", debugstr_w(face->file) );
+                TRACE( "removing matching face %s\n", debugstr_face(face) );
                 face_release( face );
                 count++;
             }
    }
         family_release( family );
     }
-done:
+
     HeapFree( GetProcessHeap(), 0, unixname );
     return count;
 }
@@ -2618,8 +2604,7 @@ static void populate_system_links(const WCHAR *name, const WCHAR *const *values)
             child_font->font = NULL;
             font_link->fs.fsCsb[0] |= face->fs.fsCsb[0];
             font_link->fs.fsCsb[1] |= face->fs.fsCsb[1];
-            TRACE("Adding file %s index %ld\n", debugstr_w(child_font->face->file),
-                  child_font->face->face_index);
+            TRACE( "Adding %s\n", debugstr_face(child_font->face) );
             list_add_tail(&font_link->links, &child_font->entry);

TRACE("added internal SystemLink for %s to %s in %s\n", debugstr_w(name), debugstr_w(value),debugstr_w(file));
@@ -2699,8 +2684,7 @@ static void init_system_links(void)
                 child_font->font = NULL;
                 font_link->fs.fsCsb[0] |= face->fs.fsCsb[0];
                 font_link->fs.fsCsb[1] |= face->fs.fsCsb[1];
-                TRACE("Adding file %s index %ld\n",
-                      debugstr_w(child_font->face->file), child_font->face->face_index);
+                TRACE( "Adding %s\n", debugstr_face(child_font->face) );
                 list_add_tail(&font_link->links, &child_font->entry);
             }
             list_add_tail(&system_links, &font_link->entry);
@@ -2758,8 +2742,7 @@ skip_internal:
         child_font->font = NULL;
         system_font_link->fs.fsCsb[0] |= face->fs.fsCsb[0];
         system_font_link->fs.fsCsb[1] |= face->fs.fsCsb[1];
-        TRACE("Found Tahoma in %s index %ld\n",
-              debugstr_w(child_font->face->file), child_font->face->face_index);
+        TRACE( "Found Tahoma in %s\n", debugstr_face(child_font->face) );
         list_add_tail(&system_font_link->links, &child_font->entry);
     }
     font_link = find_font_link(Tahoma);
@@ -3209,7 +3192,6 @@ static void update_reg_entries(void)

LIST_FOR_EACH_ENTRY( family, &font_list, Family, entry ) {
         LIST_FOR_EACH_ENTRY( face, &family->faces, Face, entry ) {
-            char *buffer;
             if (!(face->flags & ADDFONT_EXTERNAL_FONT)) continue;

len = strlenW( face->full_name ) + 1;
@@ -3222,11 +3204,7 @@ static void update_reg_entries(void)
             if (face->scalable)
                 strcatW(valueW, TrueType);

-            buffer = strWtoA( CP_UNIXCP, face->file );
-            path = wine_get_dos_file_name( buffer );
-            HeapFree( GetProcessHeap(), 0, buffer );
-
-            if (path)
+            if ((path = wine_get_dos_file_name( face->unix_name )))
             {
                 if ((full_path = get_full_path_name(path)))
                 {
@@ -4509,14 +4487,14 @@ static struct font_mapping *map_font_file( const char *name )

LIST_FOR_EACH_ENTRY( mapping, &mappings_list, struct font_mapping, entry )
     {
-        if (mapping->dev == st.st_dev && mapping->ino == st.st_ino)
+        if (!strcmp( mapping->unix_name, name ))
         {
             mapping->refcount++;
             close( fd );
             return mapping;
         }
     }
-    if (!(mapping = HeapAlloc( GetProcessHeap(), 0, sizeof(*mapping) )))
+    if (!(mapping = HeapAlloc( GetProcessHeap(), 0, sizeof(*mapping) + strlen( name ) + 1 )))
         goto error;

mapping->data = mmap( NULL, st.st_size, PROT_READ, MAP_PRIVATE, fd, 0 );
@@ -4528,8 +4506,8 @@ static struct font_mapping *map_font_file( const char *name )
         return NULL;
     }
     mapping->refcount = 1;
-    mapping->dev = st.st_dev;
-    mapping->ino = st.st_ino;
+    mapping->unix_name = (char *)(mapping + 1);
+    strcpy( mapping->unix_name, name );
     mapping->size = st.st_size;
     list_add_tail( &mappings_list, &mapping->entry );
     return mapping;
@@ -4556,17 +4534,14 @@ static FT_Face OpenFontFace(GdiFont *font, Face *face, LONG width, LONG height)
     FT_Error err;
     FT_Face ft_face;
     void *data_ptr;
-    char *filename;
     DWORD data_size;

-    TRACE( "%s, %d x %d\n", debugstr_w(face->file), width, height );
+    TRACE( "%s, %d x %d\n", debugstr_face(face), width, height );

-    filename = strWtoA( CP_UNIXCP, face->file );
-    font->mapping = map_font_file( filename );
-    HeapFree( GetProcessHeap(), 0, filename );
+    font->mapping = map_font_file( face->unix_name );
     if (!font->mapping)
     {
-        WARN( "failed to map %s\n", debugstr_w(face->file) );
+        WARN( "failed to map %s\n", debugstr_face(face) );
         return 0;
     }
     data_ptr = font->mapping->data;
@@ -4652,8 +4627,7 @@ static int get_nearest_charset(const WCHAR *family_name, Face *face, int *cp)
    }
     }

-    FIXME("returning DEFAULT_CHARSET face->fs.fsCsb[0] = %08x file = %s\n",
-	  face->fs.fsCsb[0], debugstr_w(face->file));
+    FIXME( "returning DEFAULT_CHARSET face->fs.fsCsb[0] = %08x %s\n", face->fs.fsCsb[0], debugstr_face(face) );
     *cp = acp;
     return DEFAULT_CHARSET;
 }
@@ -5086,7 +5060,7 @@ static BOOL create_child_font_list(GdiFont *font)
             new_child->font = NULL;
             face_addref( new_child->face );
             list_add_tail(&font->child_fonts, &new_child->entry);
-            TRACE("font %s %ld\n", debugstr_w(new_child->face->file), new_child->face->face_index);
+            TRACE( "face %s\n", debugstr_face(new_child->face) );
         }
         ret = TRUE;
     }
@@ -5109,7 +5083,7 @@ static BOOL create_child_font_list(GdiFont *font)
                 new_child->font = NULL;
                 face_addref( new_child->face );
                 list_add_tail(&font->child_fonts, &new_child->entry);
-                TRACE("font %s %ld\n", debugstr_w(new_child->face->file), new_child->face->face_index);
+                TRACE( "face %s\n", debugstr_face(new_child->face) );
             }
             ret = TRUE;
         }
@@ -5480,14 +5454,17 @@ static void fill_fileinfo_from_face( GdiFont *font, Face *face )
 {
     WIN32_FILE_ATTRIBUTE_DATA info;
     SIZE_T path_size;
+    WCHAR *path;
+
+    if (!(path = wine_get_dos_file_name( face->unix_name ))) path_size = 0;
+    else path_size = strlenW( path ) * sizeof(WCHAR);

-    path_size = strlenW( face->file ) * sizeof(WCHAR);
     font->fileinfo = HeapAlloc( GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*font->fileinfo) + path_size );
-    if (GetFileAttributesExW(face->file, GetFileExInfoStandard, &info))
+    if (path && GetFileAttributesExW( path, GetFileExInfoStandard, &info ))
     {
         font->fileinfo->writetime = info.ftLastWriteTime;
         font->fileinfo->size.QuadPart = (LONGLONG)info.nFileSizeHigh << 32 | info.nFileSizeLow;
-        strcpyW(font->fileinfo->path, face->file);
+        strcpyW( font->fileinfo->path, path );
     }

/* clear the path if the face was supposed to be loaded from memory */
@@ -5840,7 +5817,7 @@ found_face:
     else
         ret->charset = get_nearest_charset( family->family_name, face, &ret->codepage );

-    TRACE( "Chosen: %s (%s:%ld)\n", debugstr_w(face->full_name), debugstr_w(face->file), face->face_index );
+    TRACE( "Chosen: %s from %s\n", debugstr_w(face->full_name), debugstr_face(face) );

ret->aveWidth = height ? lf.lfWidth : 0;

[PATCH 5/8] gdi32: Compute file name on face creation for future lookup.

[Rémi Bernon]

Signed-off-by: Rémi Bernon 
---

Just a small change while I'm at it to not store the full path twice,
we now have unix_name and we are only comparing the file basename here.

dlls/gdi32/freetype.c | 32 +++++++++-----------------------
 1 file changed, 9 insertions(+), 23 deletions(-)

diff --git a/dlls/gdi32/freetype.c b/dlls/gdi32/freetype.c
index d0b076ce6fd..c613171e237 100644
--- a/dlls/gdi32/freetype.c
+++ b/dlls/gdi32/freetype.c
@@ -274,7 +274,7 @@ typedef struct tagFace {
     DWORD flags;
     WCHAR *style_name;
     WCHAR *full_name;
-    WCHAR *file;
+    WCHAR *file_name;
     FONTSIGNATURE fs;
     DWORD ntmFlags;
     FT_Fixed font_version;
@@ -1043,7 +1043,6 @@ static Face *find_face_from_filename(const WCHAR *file_name, const WCHAR *face_n
 {
     Family *family;
     Face *face;
-    const WCHAR *file;

TRACE("looking for file %s name %s\n", debugstr_w(file_name), debugstr_w(face_name));

@@ -1055,12 +1054,7 @@ static Face *find_face_from_filename(const WCHAR *file_name, const WCHAR *face_n
         LIST_FOR_EACH_ENTRY(face, face_list, Face, entry)
         {
             if (face->flags & ADDFONT_ADD_MEMRESOURCE) continue;
-            file = strrchrW(face->file, '/');
-            if(!file)
-                file = face->file;
-            else
-                file++;
-            if(strcmpiW(file, file_name)) continue;
+            if (strcmpiW( face->file_name, file_name )) continue;
             face_addref( face );
             return face;
    }
@@ -1571,6 +1565,7 @@ static ULONG family_release( Family *family )

static Face *face_create( const char *unix_name, DWORD face_index, DWORD flags )
 {
+    const char *file_name;
     Face *face;

if (!(face = HeapAlloc( GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*face) + strlen( unix_name ) + 1 )))
@@ -1586,7 +1581,9 @@ static Face *face_create( const char *unix_name, DWORD face_index, DWORD flags )
     face->flags = flags;
     strcpy( (char *)face->unix_name, unix_name );

-    face->file = towstr( CP_UNIXCP, face->unix_name );
+    if ((file_name = strrchr( face->unix_name, '/' ))) file_name++;
+    else file_name = face->unix_name;
+    face->file_name = towstr( CP_UNIXCP, file_name );
     if (!HIWORD( face->flags )) face->flags |= ADDFONT_AA_FLAGS( default_aa_flags );

return face;
@@ -1612,7 +1609,7 @@ static ULONG face_release( Face *face )
         family_release( face->family );
     }

-    HeapFree( GetProcessHeap(), 0, face->file );
+    HeapFree( GetProcessHeap(), 0, face->file_name );
     HeapFree( GetProcessHeap(), 0, face->style_name );
     HeapFree( GetProcessHeap(), 0, face->full_name );
     HeapFree( GetProcessHeap(), 0, face->cached_enum_data );
@@ -2583,11 +2580,7 @@ static void populate_system_links(const WCHAR *name, const WCHAR *const *values)
             LIST_FOR_EACH_ENTRY(face, face_list, Face, entry)
             {
                 if (face->flags & ADDFONT_ADD_MEMRESOURCE) continue;
-                file = strrchrW(face->file, '/');
-                if (!file)
-                    file = face->file;
-                else
-                    file++;
+                file = face->file_name;
                 break;
             }
             if (!file)
@@ -3213,14 +3206,7 @@ static void update_reg_entries(void)
                 }
                 file = path;
             }
-            else if ((file = strrchrW(face->file, '/')))
-            {
-                file++;
-            }
-            else
-            {
-                file = face->file;
-            }
+            else file = face->file_name;

len = strlenW(file) + 1;
             RegSetValueExW(winnt_key, valueW, 0, REG_SZ, (BYTE*)file, len * sizeof(WCHAR));

[PATCH 6/8] gdi32: Cleanup ReadFontDir and return the number of faces added.

[Rémi Bernon]

Rename it to add_faces_from_dir / add_faces_from_unix_dir, as we will
later add fontconfig dir support.

Move add_faces_from_dir below fontconfig functions, so we can use them
later.

Signed-off-by: Rémi Bernon 
---
 dlls/gdi32/freetype.c | 81 +++++++++++++++++++++----------------------
 1 file changed, 39 insertions(+), 42 deletions(-)

diff --git a/dlls/gdi32/freetype.c b/dlls/gdi32/freetype.c
index c613171e237..e26526462ba 100644
--- a/dlls/gdi32/freetype.c
+++ b/dlls/gdi32/freetype.c
@@ -2757,55 +2757,41 @@ skip_internal:
     list_add_tail(&system_links, &system_font_link->entry);
 }

-static BOOL ReadFontDir(const char *dirname, BOOL external_fonts)
+static int add_faces_from_unix_dir( const char *unix_name, DWORD flags )
 {
-    DIR *dir;
     struct dirent *dent;
     char path[MAX_PATH];
+    DIR *dir;
+    int ret = 0;

-    TRACE("Loading fonts from %s\n", debugstr_a(dirname));
+    TRACE( "Loading fonts from %s\n", debugstr_a(unix_name) );

-    dir = opendir(dirname);
-    if(!dir) {
-        WARN("Can't open directory %s\n", debugstr_a(dirname));
-	return FALSE;
+    if (!(dir = opendir( unix_name )))
+    {
+        WARN( "Can't open directory %s\n", debugstr_a(unix_name) );
+        return 0;
     }
-    while((dent = readdir(dir)) != NULL) {
-	struct stat statbuf;
-
-        if(!strcmp(dent->d_name, ".") || !strcmp(dent->d_name, ".."))
-	    continue;

-	TRACE("Found %s in %s\n", debugstr_a(dent->d_name), debugstr_a(dirname));
+    while ((dent = readdir( dir )) != NULL)
+    {
+        struct stat statbuf;
+        if (!strcmp( dent->d_name, "." ) || !strcmp( dent->d_name, ".." )) continue;

-	sprintf(path, "%s/%s", dirname, dent->d_name);
+        TRACE( "Found %s in %s\n", debugstr_a(dent->d_name), debugstr_a(unix_name) );

-	if(stat(path, &statbuf) == -1)
-	{
-	    WARN("Can't stat %s\n", debugstr_a(path));
-	    continue;
-	}
-	if(S_ISDIR(statbuf.st_mode))
-	    ReadFontDir(path, external_fonts);
-	else
+        sprintf( path, "%s/%s", unix_name, dent->d_name );
+        if (stat( path, &statbuf ) == -1)
         {
-            DWORD addfont_flags = ADDFONT_ADD_TO_CACHE;
-            if(external_fonts) addfont_flags |= ADDFONT_EXTERNAL_FONT;
-            AddFontToList( path, addfont_flags );
+            WARN( "Can't stat %s\n", debugstr_a(path) );
+            continue;
         }
-    }
-    closedir(dir);
-    return TRUE;
-}

-static void read_font_dir( const WCHAR *dirname, BOOL external_fonts )
-{
-    char *unixname = wine_get_unix_file_name( dirname );
-    if (unixname)
-    {
-        ReadFontDir( unixname, external_fonts );
-        HeapFree( GetProcessHeap(), 0, unixname );
+        if (S_ISDIR( statbuf.st_mode )) ret += add_faces_from_unix_dir( path, flags );
+        else ret += AddFontToList( path, flags );
     }
+
+    closedir( dir );
+    return ret;
 }

#ifdef SONAME_LIBFONTCONFIG
@@ -3056,6 +3042,17 @@ static void load_mac_fonts(void)

#endif

+static int add_faces_from_dir( const WCHAR *dirname, DWORD flags )
+{
+    int ret;
+    char *unix_name = wine_get_unix_file_name( dirname );
+    if (!unix_name) return 0;
+
+    ret = add_faces_from_unix_dir( unix_name, flags );
+    HeapFree( GetProcessHeap(), 0, unix_name );
+    return ret;
+}
+
 static void get_font_dir( WCHAR *path )
 {
     static const WCHAR slashW[] = {'\',0};
@@ -4197,15 +4194,15 @@ static void init_font_list(void)
     /* load in the fonts from %WINDOWSDIR%\Fonts first of all */
     GetWindowsDirectoryW(path, ARRAY_SIZE(path));
     strcatW(path, fontsW);
-    read_font_dir( path, FALSE );
+    add_faces_from_dir( path, ADDFONT_ADD_TO_CACHE );

/* load the wine fonts */
     get_font_dir( path );
-    read_font_dir( path, TRUE );
+    add_faces_from_dir( path, ADDFONT_ADD_TO_CACHE | ADDFONT_EXTERNAL_FONT );

/* now look under HKLM\Software\Microsoft\Windows[ NT]\CurrentVersion\Fonts
        for any fonts not installed in %WINDOWSDIR%\Fonts.  They will have their
-       full path as the entry.  Also look for any .fon fonts, since ReadFontDir
+       full path as the entry.  Also look for any .fon fonts, since add_faces_from_unix_dir
        will skip these. */
     if(RegOpenKeyW(HKEY_LOCAL_MACHINE,
                    is_win9x() ? win9x_font_reg_key : winnt_font_reg_key,
@@ -4255,7 +4252,7 @@ static void init_font_list(void)
 #elif defined(HAVE_CARBON_CARBON_H)
     load_mac_fonts();
 #elif defined(__ANDROID__)
-    ReadFontDir("/system/fonts", TRUE);
+    add_faces_from_unix_dir( "/system/fonts", ADDFONT_ADD_TO_CACHE | ADDFONT_EXTERNAL_FONT );
 #endif

/* then look in any directories that we've specified in the config file */
@@ -4287,11 +4284,11 @@ static void init_font_list(void)
                     {
                         strcpy( unixname, home );
                         strcat( unixname, ptr + 1 );
-                        ReadFontDir( unixname, TRUE );
+                        add_faces_from_unix_dir( unixname, ADDFONT_ADD_TO_CACHE | ADDFONT_EXTERNAL_FONT );
                         HeapFree( GetProcessHeap(), 0, unixname );
                     }
                     else
-                        ReadFontDir( ptr, TRUE );
+                        add_faces_from_unix_dir( ptr, ADDFONT_ADD_TO_CACHE | ADDFONT_EXTERNAL_FONT );
                     ptr = next;
                 }
                 HeapFree( GetProcessHeap(), 0, valueA );

[PATCH 7/8] gdi32: Cleanup and rename AddFontToList similarly.

[Rémi Bernon]

There too we will use fontconfig when available to parse font files,
as it may be able to use its cache to improve loading time.

Signed-off-by: Rémi Bernon 
---
 dlls/gdi32/freetype.c | 91 +++++++++++++++++++++----------------------
 1 file changed, 45 insertions(+), 46 deletions(-)

diff --git a/dlls/gdi32/freetype.c b/dlls/gdi32/freetype.c
index e26526462ba..6d559a917c1 100644
--- a/dlls/gdi32/freetype.c
+++ b/dlls/gdi32/freetype.c
@@ -2248,75 +2248,63 @@ fail:
     return NULL;
 }

-static INT AddFontToList( const char *file, DWORD flags )
+static int add_faces_from_unix_file( const char *unix_name, DWORD flags )
 {
     FT_Face ft_face;
     FT_Long face_index = 0, num_faces;
-    INT ret = 0;
+    int ret = 0;

/* we always load external fonts from files - otherwise we would get a crash in update_reg_entries */
-    assert(file || !(flags & ADDFONT_EXTERNAL_FONT));
+    assert( unix_name || !(flags & ADDFONT_EXTERNAL_FONT) );

#ifdef HAVE_CARBON_CARBON_H
-    if(file)
+    if (unix_name)
     {
-        char **mac_list = expand_mac_font(file);
-        if(mac_list)
+        char **mac_list = expand_mac_font( unix_name );
+        if (mac_list)
         {
             BOOL had_one = FALSE;
             char **cursor;
-            for(cursor = mac_list; *cursor; cursor++)
+            for (cursor = mac_list; *cursor; cursor++)
             {
                 had_one = TRUE;
-                AddFontToList( *cursor, flags );
-                HeapFree(GetProcessHeap(), 0, *cursor);
+                add_faces_from_unix_file( *cursor, flags );
+                HeapFree( GetProcessHeap(), 0, *cursor );
             }
-            HeapFree(GetProcessHeap(), 0, mac_list);
-            if(had_one)
-                return 1;
+            HeapFree( GetProcessHeap(), 0, mac_list );
+            if (had_one) return 1;
         }
     }
 #endif /* HAVE_CARBON_CARBON_H */

-    do {
+    do
+    {
         FONTSIGNATURE fs;

-        ft_face = new_ft_face( file, face_index, flags & ADDFONT_ALLOW_BITMAP );
+        ft_face = new_ft_face( unix_name, face_index, flags & ADDFONT_ALLOW_BITMAP );
         if (!ft_face) return 0;

-        if(ft_face->family_name[0] == '.') /* Ignore fonts with names beginning with a dot */
+        if (ft_face->family_name[0] == '.') /* Ignore fonts with names beginning with a dot */
         {
-            TRACE("Ignoring %s since its family name begins with a dot\n", debugstr_a(file));
-            pFT_Done_Face(ft_face);
+            TRACE( "Ignoring %s since its family name begins with a dot\n", debugstr_a(unix_name) );
+            pFT_Done_Face( ft_face );
             return 0;
         }

-        AddFaceToList( ft_face, file, face_index, flags );
+        AddFaceToList( ft_face, unix_name, face_index, flags );
         ++ret;

-        get_fontsig(ft_face, &fs);
+        get_fontsig( ft_face, &fs );
         if (fs.fsCsb[0] & FS_DBCS_MASK)
         {
-            AddFaceToList( ft_face, file, face_index, flags | ADDFONT_VERTICAL_FONT );
+            AddFaceToList( ft_face, unix_name, face_index, flags | ADDFONT_VERTICAL_FONT );
             ++ret;
         }

-	num_faces = ft_face->num_faces;
-	pFT_Done_Face(ft_face);
-    } while(num_faces > ++face_index);
-    return ret;
-}
+        num_faces = ft_face->num_faces;
+        pFT_Done_Face( ft_face );
+    } while (num_faces > ++face_index);

-static int add_font_resource( const WCHAR *file, DWORD flags )
-{
-    int ret = 0;
-    char *unixname = wine_get_unix_file_name( file );
-
-    if (unixname)
-    {
-        ret = AddFontToList( unixname, flags );
-        HeapFree( GetProcessHeap(), 0, unixname );
-    }
     return ret;
 }

@@ -2787,7 +2775,7 @@ static int add_faces_from_unix_dir( const char *unix_name, DWORD flags )
         }

if (S_ISDIR( statbuf.st_mode )) ret += add_faces_from_unix_dir( path, flags );
-        else ret += AddFontToList( path, flags );
+        else ret += add_faces_from_unix_file( path, flags );
     }

closedir( dir );
@@ -2912,7 +2900,7 @@ static void load_fontconfig_fonts(void)
         if(len < 4) continue;
         ext = &file[ len - 3 ];
         if(_strnicmp(ext, "pfa", -1) && _strnicmp(ext, "pfb", -1))
-            AddFontToList( file, ADDFONT_EXTERNAL_FONT | ADDFONT_ADD_TO_CACHE | ADDFONT_AA_FLAGS( aa_flags ) );
+            add_faces_from_unix_file(file, ADDFONT_EXTERNAL_FONT | ADDFONT_ADD_TO_CACHE | ADDFONT_AA_FLAGS(aa_flags) );
     }
     pFcFontSetDestroy(fontset);
     pFcPatternDestroy(pat);
@@ -2931,7 +2919,7 @@ static void load_mac_font_callback(const void *value, void *context)
     if (path && CFStringGetFileSystemRepresentation(pathStr, path, len))
     {
         TRACE("font file %s\n", path);
-        AddFontToList( path, ADDFONT_EXTERNAL_FONT | ADDFONT_ADD_TO_CACHE );
+        add_faces_from_unix_file( path, ADDFONT_EXTERNAL_FONT | ADDFONT_ADD_TO_CACHE );
     }
     HeapFree(GetProcessHeap(), 0, path);
 }
@@ -3042,6 +3030,17 @@ static void load_mac_fonts(void)

#endif

+static int add_faces_from_file( const WCHAR *file, DWORD flags )
+{
+    int ret;
+    char *unix_name = wine_get_unix_file_name( file );
+    if (!unix_name) return 0;
+
+    ret = add_faces_from_unix_file( unix_name, flags );
+    HeapFree( GetProcessHeap(), 0, unix_name );
+    return ret;
+}
+
 static int add_faces_from_dir( const WCHAR *dirname, DWORD flags )
 {
     int ret;
@@ -3105,10 +3104,10 @@ static void load_system_fonts(void)
             if(RegQueryValueExW(hkey, *value, 0, &type, (void*)data, &dlen) == ERROR_SUCCESS &&
                type == REG_SZ) {
                 get_winfonts_dir_path( data, pathW );
-                if (!add_font_resource( pathW, ADDFONT_ALLOW_BITMAP | ADDFONT_ADD_TO_CACHE ))
+                if (!add_faces_from_file( pathW, ADDFONT_ALLOW_BITMAP | ADDFONT_ADD_TO_CACHE ))
                 {
                     get_data_dir_path( data, pathW );
-                    add_font_resource( pathW, ADDFONT_ALLOW_BITMAP | ADDFONT_ADD_TO_CACHE );
+                    add_faces_from_file( pathW, ADDFONT_ALLOW_BITMAP | ADDFONT_ADD_TO_CACHE );
                 }
             }
         }
@@ -3305,17 +3304,17 @@ INT WineEngAddFontResourceEx( LPCWSTR file, DWORD flags, PVOID pdv, BOOL mem_res
         EnterCriticalSection( &freetype_cs );

if (!(flags & FR_PRIVATE)) addfont_flags |= ADDFONT_ADD_TO_CACHE;
-        ret = add_font_resource( file, addfont_flags );
+        ret = add_faces_from_file( file, addfont_flags );

if (!ret && !strchrW(file, '\')) {
             /* Try in %WINDIR%/fonts, needed for Fotobuch Designer */
             get_winfonts_dir_path( file, path );
-            ret = add_font_resource( path, addfont_flags & ~ADDFONT_ADD_TO_CACHE );
+            ret = add_faces_from_file( path, addfont_flags & ~ADDFONT_ADD_TO_CACHE );
             /* Try in datadir/fonts (or builddir/fonts), needed for Magic the Gathering Online */
             if (!ret)
             {
                 get_data_dir_path( file, path );
-                ret = add_font_resource( path, addfont_flags & ~ADDFONT_ADD_TO_CACHE );
+                ret = add_faces_from_file( path, addfont_flags & ~ADDFONT_ADD_TO_CACHE );
             }
         }

@@ -4224,17 +4223,17 @@ static void init_font_list(void)
             {
                 if(data[0] && (data[1] == ':'))
                 {
-                    add_font_resource( data, ADDFONT_ALLOW_BITMAP | ADDFONT_ADD_TO_CACHE);
+                    add_faces_from_file( data, ADDFONT_ALLOW_BITMAP | ADDFONT_ADD_TO_CACHE );
                 }
                 else if(dlen / 2 >= 6 && !strcmpiW(data + dlen / 2 - 5, dot_fonW))
                 {
                     WCHAR pathW[MAX_PATH];

get_winfonts_dir_path( data, pathW );
-                    if (!add_font_resource( pathW, ADDFONT_ALLOW_BITMAP | ADDFONT_ADD_TO_CACHE ))
+                    if (!add_faces_from_file( pathW, ADDFONT_ALLOW_BITMAP | ADDFONT_ADD_TO_CACHE ))
                     {
                         get_data_dir_path( data, pathW );
-                        add_font_resource( pathW, ADDFONT_ALLOW_BITMAP | ADDFONT_ADD_TO_CACHE );
+                        add_faces_from_file( pathW, ADDFONT_ALLOW_BITMAP | ADDFONT_ADD_TO_CACHE );
                     }
                 }
                 /* reset dlen and vlen */

[PATCH 8/8] gdi32: Create face first then initialize with ft_face.

[Rémi Bernon]

We will use FreeType cache manager to load FT_Face from Face pointers.

Signed-off-by: Rémi Bernon 
---

When fontconfig is available, we will have a face_init_from_fc_pattern
equivalent, loading data from its caches for a huge time improvement.

dlls/gdi32/freetype.c | 72 +++++++++++++++++++++----------------------
 1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/dlls/gdi32/freetype.c b/dlls/gdi32/freetype.c
index 6d559a917c1..2381514f6b4 100644
--- a/dlls/gdi32/freetype.c
+++ b/dlls/gdi32/freetype.c
@@ -2139,11 +2139,11 @@ static inline void get_fontsig( FT_Face ft_face, FONTSIGNATURE *fs )
     }
 }

-static Face *create_face( FT_Face ft_face, FT_Long face_index, const char *file, DWORD flags )
+static BOOL face_init_from_ft_face( Face *face, FT_Face ft_face )
 {
-    Face *face;
+    Family *family;

-    if (!(face = face_create( file, face_index, flags ))) return NULL;
+    if (!(family = get_family( ft_face, face->flags & ADDFONT_VERTICAL_FONT ))) return FALSE;

face->style_name = ft_face_get_style_name( ft_face, GetSystemDefaultLangID() );
     face->full_name = ft_face_get_full_name( ft_face, GetSystemDefaultLangID() );
@@ -2160,26 +2160,14 @@ static Face *create_face( FT_Face ft_face, FT_Long face_index, const char *file,
           face->fs.fsUsb[0], face->fs.fsUsb[1],
           face->fs.fsUsb[2], face->fs.fsUsb[3]);

-    return face;
-}
-
-static void AddFaceToList( FT_Face ft_face, const char *file, FT_Long face_index, DWORD flags )
-{
-    Face *face;
-    Family *family;
-
-    face = create_face( ft_face, face_index, file, flags );
-    family = get_family( ft_face, flags & ADDFONT_VERTICAL_FONT );
-
     if (insert_face_in_family_list( face, family ))
     {
-        if (flags & ADDFONT_ADD_TO_CACHE)
-            add_face_to_cache( face );
+        if (face->flags & ADDFONT_ADD_TO_CACHE) add_face_to_cache( face );
         TRACE( "Added face %s to family %s\n", debugstr_w(face->full_name), debugstr_w(family->family_name) );
     }

-    face_release( face );
     family_release( family );
+    return TRUE;
 }

static FT_Face new_ft_face( const char *file, FT_Long face_index, BOOL allow_bitmap )
@@ -2250,7 +2238,8 @@ fail:

static int add_faces_from_unix_file( const char *unix_name, DWORD flags )
 {
-    FT_Face ft_face;
+    Face *face = NULL;
+    FT_Face ft_face = NULL;
     FT_Long face_index = 0, num_faces;
     int ret = 0;

@@ -2279,33 +2268,39 @@ static int add_faces_from_unix_file( const char *unix_name, DWORD flags )

do
     {
-        FONTSIGNATURE fs;
-
-        ft_face = new_ft_face( unix_name, face_index, flags & ADDFONT_ALLOW_BITMAP );
-        if (!ft_face) return 0;
+        if (!(face = face_create( unix_name, face_index, flags ))) goto failed;
+        if (!(ft_face = new_ft_face( unix_name, face_index, flags & ADDFONT_ALLOW_BITMAP )))
+            goto failed;

if (ft_face->family_name[0] == '.') /* Ignore fonts with names beginning with a dot */
         {
             TRACE( "Ignoring %s since its family name begins with a dot\n", debugstr_a(unix_name) );
-            pFT_Done_Face( ft_face );
-            return 0;
+            goto failed;
         }

-        AddFaceToList( ft_face, unix_name, face_index, flags );
+        if (!face_init_from_ft_face( face, ft_face )) goto failed;
         ++ret;

-        get_fontsig( ft_face, &fs );
-        if (fs.fsCsb[0] & FS_DBCS_MASK)
+        if (face->fs.fsCsb[0] & FS_DBCS_MASK)
         {
-            AddFaceToList( ft_face, unix_name, face_index, flags | ADDFONT_VERTICAL_FONT );
+            face_release( face );
+            if (!(face = face_create( unix_name, face_index, flags | ADDFONT_VERTICAL_FONT )))
+                goto failed;
+            if (!face_init_from_ft_face( face, ft_face )) goto failed;
             ++ret;
         }

num_faces = ft_face->num_faces;
         pFT_Done_Face( ft_face );
+        face_release( face );
     } while (num_faces > ++face_index);

return ret;
+
+failed:
+    if (ft_face) pFT_Done_Face( ft_face );
+    if (face) face_release( face );
+    return 0;
 }

static int remove_font_resource( const WCHAR *file, DWORD flags )
@@ -3435,23 +3430,21 @@ static void GetEnumStructs(Face *face, const WCHAR *family_name, LPENUMLOGFONTEX

static BOOL get_fontdir( const char *unix_name, struct fontdir *fd )
 {
-    FT_Face ft_face = new_ft_face( unix_name, 0, FALSE );
-    Face *face;
+    FT_Face ft_face = NULL;
+    Face *face = NULL;
     WCHAR *family_name;
     ENUMLOGFONTEXW elf;
     NEWTEXTMETRICEXW ntm;
     DWORD type;

-    if (!ft_face) return FALSE;
-    face = create_face( ft_face, 0, unix_name, 0 );
+    if (!(face = face_create( unix_name, 0, 0 ))) goto failed;
+    if (!(ft_face = new_ft_face( unix_name, 0, FALSE ))) goto failed;
+    if (!face_init_from_ft_face( face, ft_face )) goto failed;
     family_name = ft_face_get_family_name( ft_face, GetSystemDefaultLCID() );
-    pFT_Done_Face( ft_face );
-
     GetEnumStructs( face, family_name, &elf, &ntm, &type );
-    face_release( face );
     HeapFree( GetProcessHeap(), 0, family_name );

-    if ((type & TRUETYPE_FONTTYPE) == 0) return FALSE;
+    if ((type & TRUETYPE_FONTTYPE) == 0) goto failed;

memset( fd, 0, sizeof(*fd) );

@@ -3487,7 +3480,14 @@ static BOOL get_fontdir( const char *unix_name, struct fontdir *fd )
     fd->dfReserved        = 0;
     WideCharToMultiByte( CP_ACP, 0, elf.elfLogFont.lfFaceName, -1, fd->szFaceName, LF_FACESIZE, NULL, NULL );

+    pFT_Done_Face( ft_face );
+    face_release( face );
     return TRUE;
+
+failed:
+    if (ft_face) pFT_Done_Face( ft_face );
+    if (face) face_release( face );
+    return FALSE;
 }

#define NE_FFLAGS_LIBMODULE     0x8000

[PATCH 8/8] gdi32: Create face first then initialize with ft_face.

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=78402

Your paranoid android.

=== debiant (32 bit report) ===

gdi32:
font.c:5914: Test failed: font wine_test should not be enumerated
font.c:5932: Test failed: RemoveFontResourceEx() error 0
font.c:5935: Test failed: font wine_test should not be enumerated
font.c:5951: Test failed: RemoveFontResourceEx() error 0
font.c:5951: Test failed: RemoveFontResourceEx() error 0
font.c:5951: Test failed: RemoveFontResourceEx() error 0
font.c:5951: Test failed: RemoveFontResourceEx() error 0
font.c:5951: Test failed: RemoveFontResourceEx() error 0
font.c:5964: Test failed: font wine_test should not be enumerated
font.c:5977: Test failed: RemoveFontResourceEx() error 0
font.c:5980: Test failed: font wine_test should not be enumerated

=== debiant (32 bit Chinese:China report) ===

gdi32:
font.c:5914: Test failed: font wine_test should not be enumerated
font.c:5932: Test failed: RemoveFontResourceEx() error 0
font.c:5935: Test failed: font wine_test should not be enumerated
font.c:5951: Test failed: RemoveFontResourceEx() error 0
font.c:5951: Test failed: RemoveFontResourceEx() error 0
font.c:5951: Test failed: RemoveFontResourceEx() error 0
font.c:5951: Test failed: RemoveFontResourceEx() error 0
font.c:5951: Test failed: RemoveFontResourceEx() error 0
font.c:5964: Test failed: font wine_test should not be enumerated
font.c:5977: Test failed: RemoveFontResourceEx() error 0
font.c:5980: Test failed: font wine_test should not be enumerated

=== debiant (32 bit WoW report) ===

gdi32:
font.c:5914: Test failed: font wine_test should not be enumerated
font.c:5932: Test failed: RemoveFontResourceEx() error 0
font.c:5935: Test failed: font wine_test should not be enumerated
font.c:5951: Test failed: RemoveFontResourceEx() error 0
font.c:5951: Test failed: RemoveFontResourceEx() error 0
font.c:5951: Test failed: RemoveFontResourceEx() error 0
font.c:5951: Test failed: RemoveFontResourceEx() error 0
font.c:5951: Test failed: RemoveFontResourceEx() error 0
font.c:5964: Test failed: font wine_test should not be enumerated
font.c:5977: Test failed: RemoveFontResourceEx() error 0
font.c:5980: Test failed: font wine_test should not be enumerated

=== debiant (64 bit WoW report) ===

gdi32:
font.c:5914: Test failed: font wine_test should not be enumerated
font.c:5932: Test failed: RemoveFontResourceEx() error 0
font.c:5935: Test failed: font wine_test should not be enumerated
font.c:5951: Test failed: RemoveFontResourceEx() error 0
font.c:5951: Test failed: RemoveFontResourceEx() error 0
font.c:5951: Test failed: RemoveFontResourceEx() error 0
font.c:5951: Test failed: RemoveFontResourceEx() error 0
font.c:5951: Test failed: RemoveFontResourceEx() error 0
font.c:5964: Test failed: font wine_test should not be enumerated
font.c:5977: Test failed: RemoveFontResourceEx() error 0
font.c:5980: Test failed: font wine_test should not be enumerated

[PATCH 8/8] gdi32: Create face first then initialize with ft_face.

[Rémi Bernon]

On 2020-09-09 13:15, Marvin wrote:
> Hi,
>
> While running your changed tests, I think I found new failures.
> Being a bot and all I'm not very good at pattern recognition, so I might be
> wrong, but could you please double-check?
>
> Full results can be found at:
> https://testbot.winehq.org/JobDetails.pl?Key=78402
>
> Your paranoid android.
>
> === debiant (32 bit report) ===
>
> gdi32:
> font.c:5914: Test failed: font wine_test should not be enumerated
> font.c:5932: Test failed: RemoveFontResourceEx() error 0
> font.c:5935: Test failed: font wine_test should not be enumerated
> font.c:5951: Test failed: RemoveFontResourceEx() error 0
> font.c:5951: Test failed: RemoveFontResourceEx() error 0
> font.c:5951: Test failed: RemoveFontResourceEx() error 0
> font.c:5951: Test failed: RemoveFontResourceEx() error 0
> font.c:5951: Test failed: RemoveFontResourceEx() error 0
> font.c:5964: Test failed: font wine_test should not be enumerated
> font.c:5977: Test failed: RemoveFontResourceEx() error 0
> font.c:5980: Test failed: font wine_test should not be enumerated
>
> === debiant (32 bit Chinese:China report) ===
>
> gdi32:
> font.c:5914: Test failed: font wine_test should not be enumerated
> font.c:5932: Test failed: RemoveFontResourceEx() error 0
> font.c:5935: Test failed: font wine_test should not be enumerated
> font.c:5951: Test failed: RemoveFontResourceEx() error 0
> font.c:5951: Test failed: RemoveFontResourceEx() error 0
> font.c:5951: Test failed: RemoveFontResourceEx() error 0
> font.c:5951: Test failed: RemoveFontResourceEx() error 0
> font.c:5951: Test failed: RemoveFontResourceEx() error 0
> font.c:5964: Test failed: font wine_test should not be enumerated
> font.c:5977: Test failed: RemoveFontResourceEx() error 0
> font.c:5980: Test failed: font wine_test should not be enumerated
>
> === debiant (32 bit WoW report) ===
>
> gdi32:
> font.c:5914: Test failed: font wine_test should not be enumerated
> font.c:5932: Test failed: RemoveFontResourceEx() error 0
> font.c:5935: Test failed: font wine_test should not be enumerated
> font.c:5951: Test failed: RemoveFontResourceEx() error 0
> font.c:5951: Test failed: RemoveFontResourceEx() error 0
> font.c:5951: Test failed: RemoveFontResourceEx() error 0
> font.c:5951: Test failed: RemoveFontResourceEx() error 0
> font.c:5951: Test failed: RemoveFontResourceEx() error 0
> font.c:5964: Test failed: font wine_test should not be enumerated
> font.c:5977: Test failed: RemoveFontResourceEx() error 0
> font.c:5980: Test failed: font wine_test should not be enumerated
>
> === debiant (64 bit WoW report) ===
>
> gdi32:
> font.c:5914: Test failed: font wine_test should not be enumerated
> font.c:5932: Test failed: RemoveFontResourceEx() error 0
> font.c:5935: Test failed: font wine_test should not be enumerated
> font.c:5951: Test failed: RemoveFontResourceEx() error 0
> font.c:5951: Test failed: RemoveFontResourceEx() error 0
> font.c:5951: Test failed: RemoveFontResourceEx() error 0
> font.c:5951: Test failed: RemoveFontResourceEx() error 0
> font.c:5951: Test failed: RemoveFontResourceEx() error 0
> font.c:5964: Test failed: font wine_test should not be enumerated
> font.c:5977: Test failed: RemoveFontResourceEx() error 0
> font.c:5980: Test failed: font wine_test should not be enumerated

Hmm that last patch indeed causes regressions, sorry. Please just ignore 
it for now.

[PATCH v2 8/8] gdi32: Create face first then initialize with ft_face.

[Rémi Bernon]

We will use FreeType cache manager to load FT_Face from Face pointers.

Signed-off-by: Rémi Bernon 
---

Supersedes: 192328

v2: Fix the test regression, face_init_from_ft_face was also used in
    get_fontdir but only to retrieve some face information, and it
    shouldn't insert the face into its family there (or we need to
    properly remove it, which we don't).

Let's keep it simple: rename AddFaceToList to face_insert_in_family,
    and call it only when we should.

As Marvin isn't going to pick this up, I did it manually, and for a
    few locales:

* https://testbot.winehq.org/JobDetails.pl?Key=78403
    * https://testbot.winehq.org/JobDetails.pl?Key=78404
    * https://testbot.winehq.org/JobDetails.pl?Key=78406
    * https://testbot.winehq.org/JobDetails.pl?Key=78407

dlls/gdi32/freetype.c | 60 ++++++++++++++++++++++---------------------
 1 file changed, 31 insertions(+), 29 deletions(-)

diff --git a/dlls/gdi32/freetype.c b/dlls/gdi32/freetype.c
index 6d559a917c1..abbeb5036f2 100644
--- a/dlls/gdi32/freetype.c
+++ b/dlls/gdi32/freetype.c
@@ -2139,12 +2139,8 @@ static inline void get_fontsig( FT_Face ft_face, FONTSIGNATURE *fs )
     }
 }

-static Face *create_face( FT_Face ft_face, FT_Long face_index, const char *file, DWORD flags )
+static void face_init_from_ft_face( Face *face, FT_Face ft_face )
 {
-    Face *face;
-
-    if (!(face = face_create( file, face_index, flags ))) return NULL;
-
     face->style_name = ft_face_get_style_name( ft_face, GetSystemDefaultLangID() );
     face->full_name = ft_face_get_full_name( ft_face, GetSystemDefaultLangID() );
     if (face->flags & ADDFONT_VERTICAL_FONT) face->full_name = get_vertical_name( face->full_name );
@@ -2159,27 +2155,21 @@ static Face *create_face( FT_Face ft_face, FT_Long face_index, const char *file,
           face->fs.fsCsb[0], face->fs.fsCsb[1],
           face->fs.fsUsb[0], face->fs.fsUsb[1],
           face->fs.fsUsb[2], face->fs.fsUsb[3]);
-
-    return face;
 }

-static void AddFaceToList( FT_Face ft_face, const char *file, FT_Long face_index, DWORD flags )
+static BOOL face_insert_in_family( Face *face, FT_Face ft_face )
 {
-    Face *face;
     Family *family;
-
-    face = create_face( ft_face, face_index, file, flags );
-    family = get_family( ft_face, flags & ADDFONT_VERTICAL_FONT );
+    if (!(family = get_family( ft_face, face->flags & ADDFONT_VERTICAL_FONT ))) return FALSE;

if (insert_face_in_family_list( face, family ))
     {
-        if (flags & ADDFONT_ADD_TO_CACHE)
-            add_face_to_cache( face );
+        if (face->flags & ADDFONT_ADD_TO_CACHE) add_face_to_cache( face );
         TRACE( "Added face %s to family %s\n", debugstr_w(face->full_name), debugstr_w(family->family_name) );
     }

-    face_release( face );
     family_release( family );
+    return TRUE;
 }

static FT_Face new_ft_face( const char *file, FT_Long face_index, BOOL allow_bitmap )
@@ -2250,7 +2240,8 @@ fail:

static int add_faces_from_unix_file( const char *unix_name, DWORD flags )
 {
-    FT_Face ft_face;
+    Face *face = NULL;
+    FT_Face ft_face = NULL;
     FT_Long face_index = 0, num_faces;
     int ret = 0;

@@ -2279,33 +2270,39 @@ static int add_faces_from_unix_file( const char *unix_name, DWORD flags )

do
     {
-        FONTSIGNATURE fs;
-
-        ft_face = new_ft_face( unix_name, face_index, flags & ADDFONT_ALLOW_BITMAP );
-        if (!ft_face) return 0;
+        if (!(face = face_create( unix_name, face_index, flags ))) goto failed;
+        if (!(ft_face = new_ft_face( unix_name, face_index, flags & ADDFONT_ALLOW_BITMAP ))) goto failed;

if (ft_face->family_name[0] == '.') /* Ignore fonts with names beginning with a dot */
         {
             TRACE( "Ignoring %s since its family name begins with a dot\n", debugstr_a(unix_name) );
-            pFT_Done_Face( ft_face );
-            return 0;
+            goto failed;
         }

-        AddFaceToList( ft_face, unix_name, face_index, flags );
+        face_init_from_ft_face( face, ft_face );
+        if (!face_insert_in_family( face, ft_face )) goto failed;
         ++ret;

-        get_fontsig( ft_face, &fs );
-        if (fs.fsCsb[0] & FS_DBCS_MASK)
+        if (face->fs.fsCsb[0] & FS_DBCS_MASK)
         {
-            AddFaceToList( ft_face, unix_name, face_index, flags | ADDFONT_VERTICAL_FONT );
+            face_release( face );
+            if (!(face = face_create( unix_name, face_index, flags | ADDFONT_VERTICAL_FONT ))) goto failed;
+            face_init_from_ft_face( face, ft_face );
+            if (!face_insert_in_family( face, ft_face )) goto failed;
             ++ret;
         }

num_faces = ft_face->num_faces;
         pFT_Done_Face( ft_face );
+        face_release( face );
     } while (num_faces > ++face_index);

return ret;
+
+failed:
+    if (ft_face) pFT_Done_Face( ft_face );
+    if (face) face_release( face );
+    return 0;
 }

static int remove_font_resource( const WCHAR *file, DWORD flags )
@@ -3435,15 +3432,20 @@ static void GetEnumStructs(Face *face, const WCHAR *family_name, LPENUMLOGFONTEX

static BOOL get_fontdir( const char *unix_name, struct fontdir *fd )
 {
-    FT_Face ft_face = new_ft_face( unix_name, 0, FALSE );
+    FT_Face ft_face;
     Face *face;
     WCHAR *family_name;
     ENUMLOGFONTEXW elf;
     NEWTEXTMETRICEXW ntm;
     DWORD type;

-    if (!ft_face) return FALSE;
-    face = create_face( ft_face, 0, unix_name, 0 );
+    if (!(face = face_create( unix_name, 0, 0 ))) return FALSE;
+    if (!(ft_face = new_ft_face( unix_name, 0, FALSE )))
+    {
+        face_release( face );
+        return FALSE;
+    }
+    face_init_from_ft_face( face, ft_face );
     family_name = ft_face_get_family_name( ft_face, GetSystemDefaultLCID() );
     pFT_Done_Face( ft_face );

[Rémi Bernon]

Forgot to add this to the notes:

This is mostly some cleanup, the real meat will still come later. The 
idea is to ultimately use fontconfig to load font information, making it 
much faster.

However, it doesn't provide font signature data in the way we are using 
it, and it's mostly impossible to reconstruct it from the charset and 
langset it provides (possibly because fonts are advertising inaccurate 
charset in their signatures), so we will still need to load the FT_Face 
at some point.

To do that in an efficient way, we will use FreeType cache manager, and 
also use it for GdiFont logic too, as it could also generally benefit 
from it.

Cheers,

[Huw Davies]

On Wed, Sep 09, 2020 at 12:21:57PM +0200, Rémi Bernon wrote:
> This makes WineEngAddFontMemResourceEx, as well as the support for
> loading font face from memory in freetype.c, obsolete.

This doesn't seem ideal to say the least.

I'm also not convinced that using fontconfig to load the font info
is necessarily the way to go.  As you say, it's missing fontsig
information and isn't available on all platforms.

Did you look at a delayed initialisation approach?  That could
potentially use an out-of-process server.

Huw.

[Rémi Bernon]

On 2020-09-15 16:12, Huw Davies wrote:
> On Wed, Sep 09, 2020 at 12:21:57PM +0200, Rémi Bernon wrote:
> > This makes WineEngAddFontMemResourceEx, as well as the support for
> > loading font face from memory in freetype.c, obsolete.
>
> This doesn't seem ideal to say the least.
>
> I'm also not convinced that using fontconfig to load the font info
> is necessarily the way to go.  As you say, it's missing fontsig
> information and isn't available on all platforms.

Well, AFAICS loading fontsig isn't actually useful until we enumerate 
fonts, in which case it can be done lazily and (hopefully) unlikely to 
be triggered on all font faces.

Regarding fontconfig usage and availability, I think it's actually at 
the same time the cause of the performance problem, and its solution. 
It's not available on all platforms, but then we don't use it there 
anyway, and on other platforms there's an equivalent issue and solution 
with the corresponding platform font management API.

On Linux for instance, the vast majority of listed fonts actually come 
from fontconfig. However, instead of using its cached data, we take the 
hard way and re-parse everything with FreeType. This really feels wrong, 
and even if we could keep using that code path for the non-platform 
fonts, we should at least use fontconfig cached data to get the font 
face info.

On other systems where fontconfig isn't available, we probably have the 
same issues and the same solutions. On macos for instance, although I've 
got limited knowledge, we use Carbon to list fonts, but then take the 
hard way too instead of using dedicated functions to access font family 
and face names [1] (which hopefully is cached by the system too).

[1] https://developer.apple.com/documentation/coretext/ctfont-q6r

On Android, we just enumerate the system fonts the hard way anyway, but 
maybe there's some better there too, or fontconfig could perhaps be 
ported there.

In any case, as long as there's only a few platform fonts, the fallback 
code with registry cache is probably still good enough, and we have to 
have it anyway in case none of the above is supported. Making the 
non-platform font listing also go the specialized font code can make 
things even a little bit faster, but it's not really necessary.

And I don't like however the idea of re-implementing a font cache and 
make everything use it instead, especially since it still takes time to 
initialize when empty, fc-cache (or the platform font management API) 
has probably already done it, and then cache management is hard to get 
right, especially since we need it to be persistent to make it really 
useful.
> Did you look at a delayed initialisation approach?  That could
> potentially use an out-of-process server.

I'm not sure if it's possible to make it lazy and right at the same 
time. Don't we have to list system fonts and update the registry entries 
on startup, so any application listing the registry could get an up to 
date font list even if they don't use gdi32?

The staging patch does the lazy initialization, and although I haven't 
checked that specific example, I have a hunch that it's broken there. I 
may be wrong, or maybe not having the registry up to date on startup 
isn't a big issue?

Cheers,

[Nikolay Sivov]

On 9/15/20 6:34 PM, Rémi Bernon wrote:
> On 2020-09-15 16:12, Huw Davies wrote:
> > On Wed, Sep 09, 2020 at 12:21:57PM +0200, Rémi Bernon wrote:
> > > This makes WineEngAddFontMemResourceEx, as well as the support for
> > > loading font face from memory in freetype.c, obsolete.
> >
> > This doesn't seem ideal to say the least.
> >
> > I'm also not convinced that using fontconfig to load the font info
> > is necessarily the way to go.  As you say, it's missing fontsig
> > information and isn't available on all platforms.
>
> Well, AFAICS loading fontsig isn't actually useful until we enumerate
> fonts, in which case it can be done lazily and (hopefully) unlikely to
> be triggered on all font faces.
>
> Regarding fontconfig usage and availability, I think it's actually at
> the same time the cause of the performance problem, and its solution.
> It's not available on all platforms, but then we don't use it there
> anyway, and on other platforms there's an equivalent issue and
> solution with the corresponding platform font management API.
>
> On Linux for instance, the vast majority of listed fonts actually come
> from fontconfig. However, instead of using its cached data, we take
> the hard way and re-parse everything with FreeType. This really feels
> wrong, and even if we could keep using that code path for the
> non-platform fonts, we should at least use fontconfig cached data to
> get the font face info.

We don't really have to create freetype objects to extract a small
number of properties, we could read everything we need directly. You'll
need this path anyway for fonts that don't come from fontconfig.
> On other systems where fontconfig isn't available, we probably have
> the same issues and the same solutions. On macos for instance,
> although I've got limited knowledge, we use Carbon to list fonts, but
> then take the hard way too instead of using dedicated functions to
> access font family and face names [1] (which hopefully is cached by
> the system too).
>
> [1] https://developer.apple.com/documentation/coretext/ctfont-q6r
>
> On Android, we just enumerate the system fonts the hard way anyway,
> but maybe there's some better there too, or fontconfig could perhaps
> be ported there.
>
> In any case, as long as there's only a few platform fonts, the
> fallback code with registry cache is probably still good enough, and
> we have to have it anyway in case none of the above is supported.
> Making the non-platform font listing also go the specialized font code
> can make things even a little bit faster, but it's not really necessary.
>
> And I don't like however the idea of re-implementing a font cache and
> make everything use it instead, especially since it still takes time
> to initialize when empty, fc-cache (or the platform font management
> API) has probably already done it, and then cache management is hard
> to get right, especially since we need it to be persistent to make it
> really useful.

Is it possible to get everything we need from fc-cache? I don't believe
that's the case for directwrite for example. For instance for names
strings you'll have to prioritize by encoding/platform, and that most
likely differs between fontconfig and windows.

We should obviously continue to use fontconfig for initial font
discovery, and maybe for some fallback cases that we don't currently
have, I think fontconfig is capable of specifying preferred font per
language or script, which is again not exactly what we need.
> > Did you look at a delayed initialisation approach?  That could
> > potentially use an out-of-process server.
>
> I'm not sure if it's possible to make it lazy and right at the same
> time. Don't we have to list system fonts and update the registry
> entries on startup, so any application listing the registry could get
> an up to date font list even if they don't use gdi32?
>
> The staging patch does the lazy initialization, and although I haven't
> checked that specific example, I have a hunch that it's broken there.
> I may be wrong, or maybe not having the registry up to date on startup
> isn't a big issue?
>
> Cheers,

[Rémi Bernon]

On 2020-09-15 18:55, Nikolay Sivov wrote:
> On 9/15/20 6:34 PM, Rémi Bernon wrote:
> > On 2020-09-15 16:12, Huw Davies wrote:
> > > On Wed, Sep 09, 2020 at 12:21:57PM +0200, Rémi Bernon wrote:
> > > > This makes WineEngAddFontMemResourceEx, as well as the support for
> > > > loading font face from memory in freetype.c, obsolete.
> > >
> > > This doesn't seem ideal to say the least.
> > >
> > > I'm also not convinced that using fontconfig to load the font info
> > > is necessarily the way to go.  As you say, it's missing fontsig
> > > information and isn't available on all platforms.
> >
> > Well, AFAICS loading fontsig isn't actually useful until we enumerate
> > fonts, in which case it can be done lazily and (hopefully) unlikely to
> > be triggered on all font faces.
> >
> > Regarding fontconfig usage and availability, I think it's actually at
> > the same time the cause of the performance problem, and its solution.
> > It's not available on all platforms, but then we don't use it there
> > anyway, and on other platforms there's an equivalent issue and
> > solution with the corresponding platform font management API.
> >
> > On Linux for instance, the vast majority of listed fonts actually come
> > from fontconfig. However, instead of using its cached data, we take
> > the hard way and re-parse everything with FreeType. This really feels
> > wrong, and even if we could keep using that code path for the
> > non-platform fonts, we should at least use fontconfig cached data to
> > get the font face info.
>
> We don't really have to create freetype objects to extract a small
> number of properties, we could read everything we need directly. You'll
> need this path anyway for fonts that don't come from fontconfig.

I also thought about that too, but there again I'm not sure if it's 
worth re-implementing something that these library already provide.

Especially if we end up doing mostly as bad as they do already. Is it 
really possible to improve the parsing time compared to FreeType?  If 
fontconfig uses caches to store the font information, isn't it because 
it just takes a long time to parse font files (especially when there are 
many)?
> > On other systems where fontconfig isn't available, we probably have
> > the same issues and the same solutions. On macos for instance,
> > although I've got limited knowledge, we use Carbon to list fonts, but
> > then take the hard way too instead of using dedicated functions to
> > access font family and face names [1] (which hopefully is cached by
> > the system too).
> >
> > [1] https://developer.apple.com/documentation/coretext/ctfont-q6r
> >
> > On Android, we just enumerate the system fonts the hard way anyway,
> > but maybe there's some better there too, or fontconfig could perhaps
> > be ported there.
> >
> > In any case, as long as there's only a few platform fonts, the
> > fallback code with registry cache is probably still good enough, and
> > we have to have it anyway in case none of the above is supported.
> > Making the non-platform font listing also go the specialized font code
> > can make things even a little bit faster, but it's not really necessary.
> >
> > And I don't like however the idea of re-implementing a font cache and
> > make everything use it instead, especially since it still takes time
> > to initialize when empty, fc-cache (or the platform font management
> > API) has probably already done it, and then cache management is hard
> > to get right, especially since we need it to be persistent to make it
> > really useful.
>
> Is it possible to get everything we need from fc-cache? I don't believe
> that's the case for directwrite for example. For instance for names
> strings you'll have to prioritize by encoding/platform, and that most
> likely differs between fontconfig and windows.

It provides high-level names, but all declared font names are there, 
with the corresponding language, encoded in UTF-8. Mapping it to the 
Windows equivalent isn't exactly immediate but I think it's doable with 
a few heuristics.

Maybe it's not accurate enough then, but gdi32 match_name_table_language 
seemed a little fuzzy, so it didn't look like it would matter too much. 
For platform provided fonts I would expect (perhaps incorrectly) font 
names to be roughly normalized.

Then I also don't know what we exactly need.
> We should obviously continue to use fontconfig for initial font
> discovery, and maybe for some fallback cases that we don't currently
> have, I think fontconfig is capable of specifying preferred font per
> language or script, which is again not exactly what we need.
> > > Did you look at a delayed initialisation approach?  That could
> > > potentially use an out-of-process server.
> >
> > I'm not sure if it's possible to make it lazy and right at the same
> > time. Don't we have to list system fonts and update the registry
> > entries on startup, so any application listing the registry could get
> > an up to date font list even if they don't use gdi32?
> >
> > The staging patch does the lazy initialization, and although I haven't
> > checked that specific example, I have a hunch that it's broken there.
> > I may be wrong, or maybe not having the registry up to date on startup
> > isn't a big issue?
> >
> > Cheers,
>
> --

Rémi Bernon

[Rémi Bernon]

On 2020-09-15 19:58, Rémi Bernon wrote:
> On 2020-09-15 18:55, Nikolay Sivov wrote:
> > On 9/15/20 6:34 PM, Rémi Bernon wrote:
> > > On 2020-09-15 16:12, Huw Davies wrote:
> > > > On Wed, Sep 09, 2020 at 12:21:57PM +0200, Rémi Bernon wrote:
> > > > > This makes WineEngAddFontMemResourceEx, as well as the support for
> > > > > loading font face from memory in freetype.c, obsolete.
> > > >
> > > > This doesn't seem ideal to say the least.
> > > >
> > > > I'm also not convinced that using fontconfig to load the font info
> > > > is necessarily the way to go.  As you say, it's missing fontsig
> > > > information and isn't available on all platforms.
> > >
> > > Well, AFAICS loading fontsig isn't actually useful until we enumerate
> > > fonts, in which case it can be done lazily and (hopefully) unlikely to
> > > be triggered on all font faces.
> > >
> > > Regarding fontconfig usage and availability, I think it's actually at
> > > the same time the cause of the performance problem, and its solution.
> > > It's not available on all platforms, but then we don't use it there
> > > anyway, and on other platforms there's an equivalent issue and
> > > solution with the corresponding platform font management API.
> > >
> > > On Linux for instance, the vast majority of listed fonts actually come
> > > from fontconfig. However, instead of using its cached data, we take
> > > the hard way and re-parse everything with FreeType. This really feels
> > > wrong, and even if we could keep using that code path for the
> > > non-platform fonts, we should at least use fontconfig cached data to
> > > get the font face info.
> >
> > We don't really have to create freetype objects to extract a small
> > number of properties, we could read everything we need directly. You'll
> > need this path anyway for fonts that don't come from fontconfig.
>
> I also thought about that too, but there again I'm not sure if it's 
> worth re-implementing something that these library already provide.
>
> Especially if we end up doing mostly as bad as they do already. Is it 
> really possible to improve the parsing time compared to FreeType?  If 
> fontconfig uses caches to store the font information, isn't it because 
> it just takes a long time to parse font files (especially when there are 
> many)?

So I did a quick try using dwrite opentype parsing code, and the parsing 
itself is probably fast enough. However, opening and mapping the font 
files using the Win32 API takes a long time, and it ends up being almost 
as slow as the FreeType parsing.

I suppose I will go down this route then, using open/mmap, as it seems 
indeed that we will be more accurate with the real font name tables. It 
will also allow us to get font signatures and keep the logic unchanged.