[PATCH 3/8] gdi32: Cleanup face and family creation and refcounting.
Rémi Bernon
rbernon at codeweavers.com
Wed Sep 9 05:21:59 CDT 2020
Signed-off-by: Rémi Bernon <rbernon at codeweavers.com>
---
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);
}
--
2.28.0
More information about the wine-devel
mailing list